Commit 6d63f17e authored by Clark DuVall's avatar Clark DuVall Committed by Chromium LUCI CQ

Add check enforcing providers are all defined in base module

Defining providers in the base module can improve startup performance
by giving the chrome split more time to preload. Now that all providers
have been moved to the base module, this CL adds a check to make sure
we don't accidentally add any in another module.

Bug: 1150600
Change-Id: Ifd49034cbf7f1fce1ad2dd7d562b520e0876e93b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2591725Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837460}
parent db544942
...@@ -400,16 +400,18 @@ def _GetManifestForModule(bundle_path, module_name): ...@@ -400,16 +400,18 @@ def _GetManifestForModule(bundle_path, module_name):
])) ]))
def _GetServiceNames(manifest): def _GetComponentNames(manifest, tag_name):
android_name = '{%s}name' % manifest_utils.ANDROID_NAMESPACE android_name = '{%s}name' % manifest_utils.ANDROID_NAMESPACE
return [s.attrib.get(android_name) for s in manifest.iter('service')] return [s.attrib.get(android_name) for s in manifest.iter(tag_name)]
def _MaybeCheckServicesPresentInBase(bundle_path, module_zips): def _MaybeCheckServicesAndProvidersPresentInBase(bundle_path, module_zips):
"""Checks bundles with isolated splits define all services in the base module. """Checks bundles with isolated splits define all services in the base module.
Due to b/169196314, service classes are not found if they are not present in Due to b/169196314, service classes are not found if they are not present in
the base module. the base module. Providers are also checked because they are loaded early in
startup, and keeping them in the base module gives more time for the chrome
split to load.
""" """
base_manifest = _GetManifestForModule(bundle_path, 'base') base_manifest = _GetManifestForModule(bundle_path, 'base')
isolated_splits = base_manifest.get('{%s}isolatedSplits' % isolated_splits = base_manifest.get('{%s}isolatedSplits' %
...@@ -419,14 +421,21 @@ def _MaybeCheckServicesPresentInBase(bundle_path, module_zips): ...@@ -419,14 +421,21 @@ def _MaybeCheckServicesPresentInBase(bundle_path, module_zips):
# Collect service names from all split manifests. # Collect service names from all split manifests.
base_zip = None base_zip = None
service_names = _GetServiceNames(base_manifest) service_names = _GetComponentNames(base_manifest, 'service')
provider_names = _GetComponentNames(base_manifest, 'provider')
for module_zip in module_zips: for module_zip in module_zips:
name = os.path.basename(module_zip)[:-len('.zip')] name = os.path.basename(module_zip)[:-len('.zip')]
if name == 'base': if name == 'base':
base_zip = module_zip base_zip = module_zip
else: else:
service_names.extend( service_names.extend(
_GetServiceNames(_GetManifestForModule(bundle_path, name))) _GetComponentNames(_GetManifestForModule(bundle_path, name),
'service'))
module_providers = _GetComponentNames(
_GetManifestForModule(bundle_path, name), 'provider')
if module_providers:
raise Exception("Providers should all be declared in the base manifest."
" '%s' module declared: %s" % (name, module_providers))
# Extract classes from the base module's dex. # Extract classes from the base module's dex.
classes = set() classes = set()
...@@ -458,6 +467,13 @@ def _MaybeCheckServicesPresentInBase(bundle_path, module_zips): ...@@ -458,6 +467,13 @@ def _MaybeCheckServicesPresentInBase(bundle_path, module_zips):
raise Exception("Service %s should be present in the base module's dex." raise Exception("Service %s should be present in the base module's dex."
" See b/169196314 for more details." % service_name) " See b/169196314 for more details." % service_name)
# Ensure all providers are present in base module.
for provider_name in provider_names:
if provider_name not in classes:
raise Exception(
"Provider %s should be present in the base module's dex." %
provider_name)
def main(args): def main(args):
args = build_utils.ExpandFileArgs(args) args = build_utils.ExpandFileArgs(args)
...@@ -516,7 +532,8 @@ def main(args): ...@@ -516,7 +532,8 @@ def main(args):
# isolated splits disabled and 2s for bundles with isolated splits # isolated splits disabled and 2s for bundles with isolated splits
# enabled. Consider making this run in parallel or move into a separate # enabled. Consider making this run in parallel or move into a separate
# step before enabling isolated splits by default. # step before enabling isolated splits by default.
_MaybeCheckServicesPresentInBase(tmp_unsigned_bundle, module_zips) _MaybeCheckServicesAndProvidersPresentInBase(tmp_unsigned_bundle,
module_zips)
if options.keystore_path: if options.keystore_path:
# NOTE: As stated by the public documentation, apksigner cannot be used # NOTE: As stated by the public documentation, apksigner cannot be used
......
...@@ -850,14 +850,12 @@ ...@@ -850,14 +850,12 @@
android:authorities="$PACKAGE.DownloadFileProvider" android:authorities="$PACKAGE.DownloadFileProvider"
android:exported="false" android:exported="false"
android:grantUriPermissions="true" android:grantUriPermissions="true"
android:initOrder="10000"
android:name="org.chromium.chrome.browser.download.DownloadFileProvider"> android:name="org.chromium.chrome.browser.download.DownloadFileProvider">
<meta-data android:name="android.support.FILE_PROVIDER_PATHS" android:resource="@xml/file_paths"/> <meta-data android:name="android.support.FILE_PROVIDER_PATHS" android:resource="@xml/file_paths"/>
</provider> # DIFF-ANCHOR: 97e158a1 </provider> # DIFF-ANCHOR: 97e158a1
<provider # DIFF-ANCHOR: 2215b9cd <provider # DIFF-ANCHOR: 2215b9cd
android:authorities="$PACKAGE.ChromeBrowserProvider;$PACKAGE.browser;$PACKAGE" android:authorities="$PACKAGE.ChromeBrowserProvider;$PACKAGE.browser;$PACKAGE"
android:exported="true" android:exported="true"
android:initOrder="10000"
android:name="org.chromium.chrome.browser.provider.ChromeBrowserProvider"> android:name="org.chromium.chrome.browser.provider.ChromeBrowserProvider">
<path-permission android:path="/bookmarks/search_suggest_query" android:readPermission="android.permission.GLOBAL_SEARCH"/> <path-permission android:path="/bookmarks/search_suggest_query" android:readPermission="android.permission.GLOBAL_SEARCH"/>
</provider> # DIFF-ANCHOR: 2215b9cd </provider> # DIFF-ANCHOR: 2215b9cd
...@@ -865,7 +863,6 @@ ...@@ -865,7 +863,6 @@
android:authorities="$PACKAGE.FileProvider" android:authorities="$PACKAGE.FileProvider"
android:exported="false" android:exported="false"
android:grantUriPermissions="true" android:grantUriPermissions="true"
android:initOrder="10000"
android:name="org.chromium.chrome.browser.util.ChromeFileProvider"> android:name="org.chromium.chrome.browser.util.ChromeFileProvider">
<meta-data android:name="android.support.FILE_PROVIDER_PATHS" android:resource="@xml/file_paths"/> <meta-data android:name="android.support.FILE_PROVIDER_PATHS" android:resource="@xml/file_paths"/>
</provider> # DIFF-ANCHOR: 6e306896 </provider> # DIFF-ANCHOR: 6e306896
......
...@@ -788,14 +788,12 @@ ...@@ -788,14 +788,12 @@
android:authorities="$PACKAGE.DownloadFileProvider" android:authorities="$PACKAGE.DownloadFileProvider"
android:exported="false" android:exported="false"
android:grantUriPermissions="true" android:grantUriPermissions="true"
android:initOrder="10000"
android:name="org.chromium.chrome.browser.download.DownloadFileProvider"> android:name="org.chromium.chrome.browser.download.DownloadFileProvider">
<meta-data android:name="android.support.FILE_PROVIDER_PATHS" android:resource="@xml/file_paths"/> <meta-data android:name="android.support.FILE_PROVIDER_PATHS" android:resource="@xml/file_paths"/>
</provider> # DIFF-ANCHOR: 97e158a1 </provider> # DIFF-ANCHOR: 97e158a1
<provider # DIFF-ANCHOR: 2215b9cd <provider # DIFF-ANCHOR: 2215b9cd
android:authorities="$PACKAGE.ChromeBrowserProvider;$PACKAGE.browser;$PACKAGE" android:authorities="$PACKAGE.ChromeBrowserProvider;$PACKAGE.browser;$PACKAGE"
android:exported="true" android:exported="true"
android:initOrder="10000"
android:name="org.chromium.chrome.browser.provider.ChromeBrowserProvider"> android:name="org.chromium.chrome.browser.provider.ChromeBrowserProvider">
<path-permission android:path="/bookmarks/search_suggest_query" android:readPermission="android.permission.GLOBAL_SEARCH"/> <path-permission android:path="/bookmarks/search_suggest_query" android:readPermission="android.permission.GLOBAL_SEARCH"/>
</provider> # DIFF-ANCHOR: 2215b9cd </provider> # DIFF-ANCHOR: 2215b9cd
...@@ -803,7 +801,6 @@ ...@@ -803,7 +801,6 @@
android:authorities="$PACKAGE.FileProvider" android:authorities="$PACKAGE.FileProvider"
android:exported="false" android:exported="false"
android:grantUriPermissions="true" android:grantUriPermissions="true"
android:initOrder="10000"
android:name="org.chromium.chrome.browser.util.ChromeFileProvider"> android:name="org.chromium.chrome.browser.util.ChromeFileProvider">
<meta-data android:name="android.support.FILE_PROVIDER_PATHS" android:resource="@xml/file_paths"/> <meta-data android:name="android.support.FILE_PROVIDER_PATHS" android:resource="@xml/file_paths"/>
</provider> # DIFF-ANCHOR: 6e306896 </provider> # DIFF-ANCHOR: 6e306896
......
...@@ -1222,13 +1222,11 @@ by a child template that "extends" this file. ...@@ -1222,13 +1222,11 @@ by a child template that "extends" this file.
"com.google.android.gms.cast.framework.OPTIONS_PROVIDER_CLASS_NAME" "com.google.android.gms.cast.framework.OPTIONS_PROVIDER_CLASS_NAME"
android:value="org.chromium.components.media_router.caf.CastOptionsProvider"/> android:value="org.chromium.components.media_router.caf.CastOptionsProvider"/>
<!-- These providers are declared in the base module and have the <!-- These providers are declared in the base module to give the chrome
initOrder attribute to ensure they are loaded first, which gives the split preloader more time to work. -->
chrome split preloader more time to work. -->
<provider android:name="org.chromium.chrome.browser.util.ChromeFileProvider" <provider android:name="org.chromium.chrome.browser.util.ChromeFileProvider"
android:authorities="{{ manifest_package }}.FileProvider" android:authorities="{{ manifest_package }}.FileProvider"
android:exported="false" android:exported="false"
android:initOrder="10000"
android:grantUriPermissions="true"> android:grantUriPermissions="true">
<meta-data android:name="android.support.FILE_PROVIDER_PATHS" <meta-data android:name="android.support.FILE_PROVIDER_PATHS"
android:resource="@xml/file_paths" /> android:resource="@xml/file_paths" />
...@@ -1237,7 +1235,6 @@ by a child template that "extends" this file. ...@@ -1237,7 +1235,6 @@ by a child template that "extends" this file.
<provider android:name="org.chromium.chrome.browser.download.DownloadFileProvider" <provider android:name="org.chromium.chrome.browser.download.DownloadFileProvider"
android:authorities="{{ manifest_package }}.DownloadFileProvider" android:authorities="{{ manifest_package }}.DownloadFileProvider"
android:exported="false" android:exported="false"
android:initOrder="10000"
android:grantUriPermissions="true"> android:grantUriPermissions="true">
<meta-data android:name="android.support.FILE_PROVIDER_PATHS" <meta-data android:name="android.support.FILE_PROVIDER_PATHS"
android:resource="@xml/file_paths" /> android:resource="@xml/file_paths" />
...@@ -1246,7 +1243,6 @@ by a child template that "extends" this file. ...@@ -1246,7 +1243,6 @@ by a child template that "extends" this file.
<!-- Provider for chrome data. --> <!-- Provider for chrome data. -->
<provider android:name="org.chromium.chrome.browser.provider.ChromeBrowserProvider" <provider android:name="org.chromium.chrome.browser.provider.ChromeBrowserProvider"
android:authorities="{{ manifest_package }}.ChromeBrowserProvider;{{ manifest_package }}.browser;{{ manifest_package }}" android:authorities="{{ manifest_package }}.ChromeBrowserProvider;{{ manifest_package }}.browser;{{ manifest_package }}"
android:initOrder="10000"
android:exported="true"> android:exported="true">
<path-permission android:path="/bookmarks/search_suggest_query" <path-permission android:path="/bookmarks/search_suggest_query"
android:readPermission="android.permission.GLOBAL_SEARCH" /> android:readPermission="android.permission.GLOBAL_SEARCH" />
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment