Use ServiceLoaderUtil with AppHooks
Bug: 40901855 Change-Id: I346dd09c39182abc39bb696c938d03b5b0b4a769 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5866990 Auto-Submit: Andrew Grieve <agrieve@chromium.org> Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com> Reviewed-by: Henrique Nakashima <hnakashima@chromium.org> Commit-Queue: Andrew Grieve <agrieve@chromium.org> Cr-Commit-Position: refs/heads/main@{#1356084}
This commit is contained in:
Andrew Grieve
committed by
Chromium LUCI CQ
Chromium LUCI CQ
parent
476d836641
commit
21849f23dd
PRESUBMIT.py
build/android/docs
chrome/android
tools/android/build_speed
@ -2216,7 +2216,6 @@ _LONG_PATH_ERROR = (
|
||||
)
|
||||
|
||||
_JAVA_MULTIPLE_DEFINITION_EXCLUDED_PATHS = [
|
||||
r".*/AppHooksImpl\.java",
|
||||
r".*/BuildHooksAndroidImpl\.java",
|
||||
r".*/LicenseContentProvider\.java",
|
||||
r".*/PlatformServiceBridgeImpl.java",
|
||||
|
||||
@ -108,12 +108,9 @@ This step happens only when targets that have `jar_excluded_patterns` or
|
||||
further on in the build process.
|
||||
* E.g.: `R.class` files - a part of [Android Resources].
|
||||
* E.g.: `GEN_JNI.class` - a part of our [JNI] glue.
|
||||
* E.g.: `AppHooksImpl.class` - how `chrome_java` wires up different
|
||||
implementations for [non-public builds][apphooks].
|
||||
|
||||
[JNI]: /third_party/jni_zero/README.md
|
||||
[Android Resources]: life_of_a_resource.md
|
||||
[apphooks]: /chrome/android/java/src/org/chromium/chrome/browser/AppHooksImpl.java
|
||||
|
||||
### Step 6: Per-Library Dexing
|
||||
|
||||
|
||||
@ -74,8 +74,6 @@ if (android_64bit_target_cpu) {
|
||||
}
|
||||
|
||||
if (current_toolchain == default_toolchain) {
|
||||
app_hooks_impl = "java/src/org/chromium/chrome/browser/AppHooksImpl.java"
|
||||
|
||||
template("chrome_public_bundle") {
|
||||
_is_monochrome = defined(invoker.is_monochrome) && invoker.is_monochrome
|
||||
if (_is_monochrome) {
|
||||
@ -291,15 +289,8 @@ if (current_toolchain == default_toolchain) {
|
||||
deps = [ "//ui/android:ui_java_resources" ]
|
||||
}
|
||||
|
||||
android_library("app_hooks_java") {
|
||||
sources = [ app_hooks_impl ]
|
||||
deps = [ ":chrome_java" ]
|
||||
jacoco_never_instrument = true
|
||||
}
|
||||
|
||||
java_group("delegate_public_impl_java") {
|
||||
deps = [
|
||||
":app_hooks_java",
|
||||
"//chrome/android/modules/readaloud/public:provider_public_java",
|
||||
"//chrome/browser/accessibility/hierarchysnapshotter/android:delegate_public_impl_java",
|
||||
"//chrome/browser/android/metrics:delegate_public_impl_java",
|
||||
@ -793,7 +784,7 @@ if (current_toolchain == default_toolchain) {
|
||||
]
|
||||
|
||||
# From java_sources.gni.
|
||||
sources = chrome_java_sources + [ app_hooks_impl ]
|
||||
sources = chrome_java_sources
|
||||
|
||||
# Include sources from feed_java_sources.gni.
|
||||
sources += feed_java_sources
|
||||
@ -807,10 +798,6 @@ if (current_toolchain == default_toolchain) {
|
||||
|
||||
srcjar_deps += [ ":chrome_jni_headers" ]
|
||||
|
||||
# Add the actual implementation where necessary so that downstream targets
|
||||
# can provide their own implementations.
|
||||
jar_excluded_patterns = [ "*/AppHooksImpl.class" ]
|
||||
|
||||
annotation_processor_deps = [
|
||||
# Dagger must come first so that when enable_chrome_android_internal=true,
|
||||
# the bundled version of Guava is not shadowed by the one in android_deps.
|
||||
|
||||
@ -10,120 +10,29 @@ import android.view.View;
|
||||
import androidx.annotation.Nullable;
|
||||
|
||||
import org.chromium.base.ContextUtils;
|
||||
import org.chromium.base.ResettersForTesting;
|
||||
import org.chromium.chrome.browser.customtabs.CustomTabsConnection;
|
||||
import org.chromium.chrome.browser.init.ProcessInitializationHandler;
|
||||
import org.chromium.chrome.browser.instantapps.InstantAppsHandler;
|
||||
import org.chromium.chrome.browser.metrics.VariationsSession;
|
||||
import org.chromium.chrome.browser.notifications.chime.ChimeDelegate;
|
||||
import org.chromium.chrome.browser.omaha.RequestGenerator;
|
||||
import org.chromium.base.ServiceLoaderUtil;
|
||||
import org.chromium.chrome.browser.partnerbookmarks.PartnerBookmark;
|
||||
import org.chromium.chrome.browser.partnerbookmarks.PartnerBookmarksProviderIterator;
|
||||
import org.chromium.chrome.browser.password_manager.GooglePasswordManagerUIProvider;
|
||||
import org.chromium.chrome.browser.policy.PolicyAuditor;
|
||||
import org.chromium.chrome.browser.rlz.RevenueStats;
|
||||
import org.chromium.chrome.browser.sync.TrustedVaultClient;
|
||||
import org.chromium.chrome.browser.ui.signin.GoogleActivityController;
|
||||
import org.chromium.chrome.browser.usage_stats.DigitalWellbeingClient;
|
||||
import org.chromium.chrome.browser.webapps.GooglePlayWebApkInstallDelegate;
|
||||
import org.chromium.components.commerce.core.ShoppingService.PriceInsightsInfo;
|
||||
import org.chromium.components.policy.AppRestrictionsProvider;
|
||||
import org.chromium.components.policy.CombinedPolicyProvider;
|
||||
import org.chromium.components.signin.AccountManagerDelegate;
|
||||
import org.chromium.components.signin.SystemAccountManagerDelegate;
|
||||
import org.chromium.components.webapps.AppDetailsDelegate;
|
||||
|
||||
/**
|
||||
* Base class for defining methods where different behavior is required by downstream targets.
|
||||
* The correct version of {@link AppHooksImpl} will be determined at compile time via build rules.
|
||||
* See http://crbug/560466.
|
||||
*
|
||||
* Note that new functionality should not be added to AppHooks. Instead the delegate pattern in
|
||||
* go/apphooks-migration should be followed to solve this class of problems.
|
||||
* <p>Uses ServiceLoaderUtil to override when downstream is available.
|
||||
*
|
||||
* <p>Prefer to create a dedicate interface and directly use ServiceLoaderUtil for future hooks.
|
||||
*/
|
||||
public abstract class AppHooks {
|
||||
private static AppHooksImpl sInstanceForTesting;
|
||||
|
||||
/** Sets a mocked instance for testing. */
|
||||
public static void setInstanceForTesting(AppHooksImpl instance) {
|
||||
sInstanceForTesting = instance;
|
||||
ResettersForTesting.register(() -> sInstanceForTesting = null);
|
||||
}
|
||||
|
||||
public class AppHooks {
|
||||
public static AppHooks get() {
|
||||
if (sInstanceForTesting != null) return sInstanceForTesting;
|
||||
// R8 can better optimize if we return a new instance each time.
|
||||
return new AppHooksImpl();
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a new {@link AccountManagerDelegate}.
|
||||
* @return the created {@link AccountManagerDelegate}.
|
||||
*/
|
||||
public AccountManagerDelegate createAccountManagerDelegate() {
|
||||
return new SystemAccountManagerDelegate();
|
||||
}
|
||||
|
||||
/**
|
||||
* @return An instance of AppDetailsDelegate that can be queried about app information for the
|
||||
* App Banner feature. Will be null if one is unavailable.
|
||||
*/
|
||||
public AppDetailsDelegate createAppDetailsDelegate() {
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return An instance of {@link CustomTabsConnection}. Should not be called outside of {@link
|
||||
* CustomTabsConnection#getInstance()}.
|
||||
*/
|
||||
public CustomTabsConnection createCustomTabsConnection() {
|
||||
return new CustomTabsConnection();
|
||||
}
|
||||
|
||||
/**
|
||||
* @return An instance of GoogleActivityController.
|
||||
*/
|
||||
public GoogleActivityController createGoogleActivityController() {
|
||||
return new GoogleActivityController();
|
||||
}
|
||||
|
||||
public InstantAppsHandler createInstantAppsHandler() {
|
||||
return new InstantAppsHandler();
|
||||
}
|
||||
|
||||
/**
|
||||
* @return An instance of {@link GooglePasswordManagerUIProvider}. Will be null if one is not
|
||||
* available.
|
||||
*/
|
||||
public GooglePasswordManagerUIProvider createGooglePasswordManagerUIProvider() {
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return An instance of RequestGenerator to be used for Omaha XML creation. Will be null if
|
||||
* a generator is unavailable.
|
||||
*/
|
||||
public RequestGenerator createOmahaRequestGenerator() {
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return a new {@link ProcessInitializationHandler} instance.
|
||||
*/
|
||||
public ProcessInitializationHandler createProcessInitializationHandler() {
|
||||
return new ProcessInitializationHandler();
|
||||
}
|
||||
|
||||
/**
|
||||
* @return An instance of RevenueStats to be installed as a singleton.
|
||||
*/
|
||||
public RevenueStats createRevenueStatsInstance() {
|
||||
return new RevenueStats();
|
||||
}
|
||||
|
||||
/** Returns a new instance of VariationsSession. */
|
||||
public VariationsSession createVariationsSession() {
|
||||
return new VariationsSession();
|
||||
AppHooks ret = ServiceLoaderUtil.maybeCreate(AppHooks.class);
|
||||
if (ret == null) {
|
||||
ret = new AppHooks();
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
|
||||
/** Returns the singleton instance of GooglePlayWebApkInstallDelegate. */
|
||||
@ -131,14 +40,6 @@ public abstract class AppHooks {
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return An instance of PolicyAuditor that notifies the policy system of the user's activity.
|
||||
* Only applicable when the user has a policy active, that is tracking the activity.
|
||||
*/
|
||||
public PolicyAuditor getPolicyAuditor() {
|
||||
return null;
|
||||
}
|
||||
|
||||
public void registerPolicyProviders(CombinedPolicyProvider combinedProvider) {
|
||||
combinedProvider.registerProvider(
|
||||
new AppRestrictionsProvider(ContextUtils.getApplicationContext()));
|
||||
@ -152,32 +53,11 @@ public abstract class AppHooks {
|
||||
return PartnerBookmarksProviderIterator.createIfAvailable();
|
||||
}
|
||||
|
||||
/**
|
||||
* @return A new {@link DigitalWellbeingClient} instance.
|
||||
*/
|
||||
public DigitalWellbeingClient createDigitalWellbeingClient() {
|
||||
return new DigitalWellbeingClient();
|
||||
}
|
||||
|
||||
/** Returns a new {@link TrustedVaultClient.Backend} instance. */
|
||||
public TrustedVaultClient.Backend createSyncTrustedVaultClientBackend() {
|
||||
return new TrustedVaultClient.EmptyBackend();
|
||||
}
|
||||
|
||||
/** Returns the URL to the WebAPK creation/update server. */
|
||||
public String getWebApkServerUrl() {
|
||||
return "";
|
||||
}
|
||||
|
||||
/** Returns a Chime Delegate if the chime module is defined. */
|
||||
public ChimeDelegate getChimeDelegate() {
|
||||
return new ChimeDelegate();
|
||||
}
|
||||
|
||||
public String getDefaultQueryTilesServerUrl() {
|
||||
return "";
|
||||
}
|
||||
|
||||
public void registerProtoExtensions() {}
|
||||
|
||||
/** Returns the view of the line chart given the price insights info. */
|
||||
@ -185,5 +65,6 @@ public abstract class AppHooks {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Stop! Do not add new methods to AppHooks anymore. Follow go/apphooks-migration instead.
|
||||
// Stop! Prefer to create a dedicated interface and directly use ServiceLoaderUtil for future
|
||||
// hooks.
|
||||
}
|
||||
|
||||
@ -1,11 +0,0 @@
|
||||
// Copyright 2017 The Chromium Authors
|
||||
// Use of this source code is governed by a BSD-style license that can be
|
||||
// found in the LICENSE file.
|
||||
|
||||
package org.chromium.chrome.browser;
|
||||
|
||||
/**
|
||||
* Instantiatable version of {@link AppHooks}, don't add anything to this class!
|
||||
* Downstream targets may provide a different implementation. See http://crbug/560466.
|
||||
*/
|
||||
public class AppHooksImpl extends AppHooks {}
|
||||
@ -28,6 +28,7 @@ import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
|
||||
import org.chromium.chrome.browser.metrics.UmaUtils;
|
||||
import org.chromium.chrome.browser.metrics.VariationsSession;
|
||||
import org.chromium.chrome.browser.notifications.NotificationPlatformBridge;
|
||||
import org.chromium.chrome.browser.notifications.chime.ChimeDelegate;
|
||||
import org.chromium.chrome.browser.partnercustomizations.PartnerBrowserCustomizations;
|
||||
import org.chromium.chrome.browser.password_manager.PasswordManagerLifecycleHelper;
|
||||
import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
|
||||
@ -163,7 +164,7 @@ public class ChromeActivitySessionTracker {
|
||||
ChromeLocalizationUtils.recordUiLanguageStatus();
|
||||
mVariationsSession.start();
|
||||
mOmahaServiceStartDelayer.onForegroundSessionStart();
|
||||
AppHooks.get().getChimeDelegate().startSession();
|
||||
new ChimeDelegate().startSession();
|
||||
PasswordManagerLifecycleHelper.getInstance().onStartForegroundSession();
|
||||
|
||||
// Track the ratio of Chrome startups that are caused by notification clicks.
|
||||
|
||||
@ -27,6 +27,7 @@ import org.chromium.chrome.browser.dependency_injection.ModuleFactoryOverrides;
|
||||
import org.chromium.chrome.browser.flags.ChromeFeatureList;
|
||||
import org.chromium.chrome.browser.fonts.FontPreloader;
|
||||
import org.chromium.chrome.browser.night_mode.SystemNightModeMonitor;
|
||||
import org.chromium.chrome.browser.notifications.chime.ChimeDelegate;
|
||||
import org.chromium.chrome.browser.profiles.ProfileResolver;
|
||||
import org.chromium.chrome.browser.webauthn.CredManUiRecommenderImpl;
|
||||
import org.chromium.components.browser_ui.util.BrowserUiUtilsCachedFlags;
|
||||
@ -96,7 +97,7 @@ public class ChromeApplicationImpl extends SplitCompatApplication.Impl {
|
||||
ContextualNotificationPermissionRequesterImpl.initialize();
|
||||
PartitionResolverSupplier.setInstance(new ProfileResolver());
|
||||
|
||||
AppHooks.get().getChimeDelegate().initialize();
|
||||
new ChimeDelegate().initialize();
|
||||
|
||||
// Initialize the AccessibilityHierarchySnapshotter. Do not include in release builds.
|
||||
if (!BuildConfig.IS_CHROME_BRANDED) {
|
||||
|
||||
@ -2,7 +2,6 @@ include_rules = [
|
||||
"+chrome/android/java/src/org/chromium/chrome/browser/app/ChromeActivity.java",
|
||||
"+chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java",
|
||||
"+chrome/android/java/src/org/chromium/chrome/browser/privacy_sandbox/TrackingProtectionSnackbarController.java",
|
||||
"!clank/java/src/org/chromium/chrome/browser/AppHooksImpl.java",
|
||||
"+chrome/browser/android/lifecycle",
|
||||
"+components/image_fetcher",
|
||||
"+chrome/browser/ui/android/appmenu/java/src/org/chromium/chrome/browser/ui/appmenu/AppMenuDelegate.java",
|
||||
|
||||
@ -15,7 +15,6 @@ import android.content.Context;
|
||||
import android.content.pm.PackageManager;
|
||||
import android.os.Build;
|
||||
|
||||
import org.junit.After;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
@ -34,7 +33,6 @@ import org.chromium.base.UserDataHost;
|
||||
import org.chromium.base.task.test.ShadowPostTask;
|
||||
import org.chromium.base.test.BaseRobolectricTestRunner;
|
||||
import org.chromium.chrome.browser.ActivityTabProvider;
|
||||
import org.chromium.chrome.browser.AppHooks;
|
||||
import org.chromium.chrome.browser.fullscreen.FullscreenManager;
|
||||
import org.chromium.chrome.browser.infobar.InfoBarContainer;
|
||||
import org.chromium.chrome.browser.tab.Tab;
|
||||
@ -126,11 +124,6 @@ public class FullscreenVideoPictureInPictureControllerUnitTest {
|
||||
mActivity, mActivityTabProvider, mFullscreenManager);
|
||||
}
|
||||
|
||||
@After
|
||||
public void tearDown() {
|
||||
AppHooks.setInstanceForTesting(null);
|
||||
}
|
||||
|
||||
/** Set up the mocks to claim that there is / is not full screen video playback */
|
||||
private void setHasFullscreenVideo(boolean hasVideo) {
|
||||
when(mWebContents.hasActiveEffectivelyFullscreenVideo()).thenReturn(hasVideo);
|
||||
|
||||
@ -28,8 +28,6 @@ import org.chromium.base.supplier.Supplier;
|
||||
import org.chromium.base.test.BaseRobolectricTestRunner;
|
||||
import org.chromium.base.test.util.HistogramWatcher;
|
||||
import org.chromium.base.test.util.JniMocker;
|
||||
import org.chromium.chrome.browser.AppHooks;
|
||||
import org.chromium.chrome.browser.AppHooksImpl;
|
||||
import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
|
||||
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
|
||||
import org.chromium.chrome.browser.profiles.Profile;
|
||||
@ -77,7 +75,6 @@ public class ShareDelegateImplUnitTest {
|
||||
@Mock private WindowAndroid mWindowAndroid;
|
||||
@Mock private Activity mActivity;
|
||||
@Mock private LargeIconBridgeJni mLargeIconBridgeJni;
|
||||
@Mock private AppHooksImpl mAppHooks;
|
||||
@Mock private Tracker mTracker;
|
||||
|
||||
private ShareDelegateImpl mShareDelegate;
|
||||
@ -97,7 +94,6 @@ public class ShareDelegateImplUnitTest {
|
||||
@Before
|
||||
public void setup() {
|
||||
mJniMocker.mock(LargeIconBridgeJni.TEST_HOOKS, mLargeIconBridgeJni);
|
||||
AppHooks.setInstanceForTesting(mAppHooks);
|
||||
TrackerFactory.setTrackerForTests(mTracker);
|
||||
Mockito.doReturn(new WeakReference<>(mActivity)).when(mWindowAndroid).getActivity();
|
||||
createShareDelegate(false);
|
||||
|
||||
@ -127,18 +127,18 @@ class Benchmark:
|
||||
_BENCHMARKS = [
|
||||
Benchmark(
|
||||
name='chrome_java_nosig',
|
||||
from_string='sInstanceForTesting = instance;',
|
||||
to_string='sInstanceForTesting = instance;String test = "Test";',
|
||||
from_string='super.onCreate();',
|
||||
to_string='super.onCreate();String test = "Test";',
|
||||
change_file=
|
||||
'chrome/android/java/src/org/chromium/chrome/browser/AppHooks.java',
|
||||
'chrome/android/java/src/org/chromium/chrome/browser/ChromeApplicationImpl.java', # pylint: disable=line-too-long
|
||||
),
|
||||
Benchmark(
|
||||
name='chrome_java_sig',
|
||||
from_string='AppHooksImpl sInstanceForTesting;',
|
||||
from_string='private static final Object sLock = new Object();',
|
||||
to_string=
|
||||
'AppHooksImpl sInstanceForTesting;public void NewInterfaceMethod(){}',
|
||||
'private static final Object sLock = new Object();public void NewInterfaceMethod(){}',
|
||||
change_file=
|
||||
'chrome/android/java/src/org/chromium/chrome/browser/AppHooks.java',
|
||||
'chrome/android/java/src/org/chromium/chrome/browser/ChromeApplicationImpl.java', # pylint: disable=line-too-long
|
||||
),
|
||||
Benchmark(
|
||||
name='module_java_public_sig',
|
||||
|
||||
Reference in New Issue
Block a user