Commit 98edb471 authored by Jinsuk Kim's avatar Jinsuk Kim Committed by Commit Bot

Android: Use a single TabDelegateFactory per ChromeActivity

A single factory instance for TabDelegate objects is enough for a
ChromeActivity. This CL prevents multiple instantiations of the factory.
This fixes a bug that logs MobileNTPOpenedInNewTab more often
than expected due to numerous UMA reporter objects.

Bug: 1014382
Change-Id: I7c28f6cd500bc58a835c16d7e5dbb6f9e90d82b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2554274
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#830498}
parent 1bdd89d6
...@@ -305,6 +305,7 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent ...@@ -305,6 +305,7 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
new ObservableSupplierImpl<>(); new ObservableSupplierImpl<>();
private CallbackController mCallbackController = new CallbackController(); private CallbackController mCallbackController = new CallbackController();
private TabbedModeTabDelegateFactory mTabDelegateFactory;
private final IncognitoTabHost mIncognitoTabHost = new IncognitoTabHost() { private final IncognitoTabHost mIncognitoTabHost = new IncognitoTabHost() {
@Override @Override
...@@ -1651,15 +1652,19 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent ...@@ -1651,15 +1652,19 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
mBookmarkBridgeSupplier, getModalDialogManager()); mBookmarkBridgeSupplier, getModalDialogManager());
} }
@Override private TabDelegateFactory getTabDelegateFactory() {
protected Pair<ChromeTabCreator, ChromeTabCreator> createTabCreators() { if (mTabDelegateFactory == null) {
Supplier<TabDelegateFactory> tabDelegateFactorySupplier = () -> { mTabDelegateFactory = new TabbedModeTabDelegateFactory(this,
return new TabbedModeTabDelegateFactory(this, getAppBrowserControlsVisibilityDelegate(), getAppBrowserControlsVisibilityDelegate(), getShareDelegateSupplier(),
getShareDelegateSupplier(), mEphemeralTabCoordinatorSupplier, mEphemeralTabCoordinatorSupplier,
((TabbedRootUiCoordinator) mRootUiCoordinator)::onContextMenuCopyLink, ((TabbedRootUiCoordinator) mRootUiCoordinator)::onContextMenuCopyLink,
mRootUiCoordinator.getBottomSheetController()); mRootUiCoordinator.getBottomSheetController());
}; }
return mTabDelegateFactory;
}
@Override
protected Pair<ChromeTabCreator, ChromeTabCreator> createTabCreators() {
ChromeTabCreator.OverviewNTPCreator overviewNTPCreator = null; ChromeTabCreator.OverviewNTPCreator overviewNTPCreator = null;
if (StartSurfaceConfiguration.isStartSurfaceEnabled()) { if (StartSurfaceConfiguration.isStartSurfaceEnabled()) {
...@@ -1676,10 +1681,10 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent ...@@ -1676,10 +1681,10 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
}; };
} }
return Pair.create(new ChromeTabCreator(this, getWindowAndroid(), getStartupTabPreloader(), return Pair.create(new ChromeTabCreator(this, getWindowAndroid(), getStartupTabPreloader(),
tabDelegateFactorySupplier, false, overviewNTPCreator, this::getTabDelegateFactory, false, overviewNTPCreator,
AsyncTabParamsManagerSingleton.getInstance()), AsyncTabParamsManagerSingleton.getInstance()),
new ChromeTabCreator(this, getWindowAndroid(), getStartupTabPreloader(), new ChromeTabCreator(this, getWindowAndroid(), getStartupTabPreloader(),
tabDelegateFactorySupplier, true, overviewNTPCreator, this::getTabDelegateFactory, true, overviewNTPCreator,
AsyncTabParamsManagerSingleton.getInstance())); AsyncTabParamsManagerSingleton.getInstance()));
} }
...@@ -2158,6 +2163,7 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent ...@@ -2158,6 +2163,7 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
ChromeAccessibilityUtil.get().removeObserver(this); ChromeAccessibilityUtil.get().removeObserver(this);
ChromeAccessibilityUtil.get().removeObserver(mLayoutManager); ChromeAccessibilityUtil.get().removeObserver(mLayoutManager);
if (mTabDelegateFactory != null) mTabDelegateFactory.destroy();
super.onDestroyInternal(); super.onDestroyInternal();
} }
......
...@@ -85,4 +85,9 @@ public class TabbedModeTabDelegateFactory implements TabDelegateFactory { ...@@ -85,4 +85,9 @@ public class TabbedModeTabDelegateFactory implements TabDelegateFactory {
} }
return mNativePageFactory.createNativePage(url, candidatePage, tab); return mNativePageFactory.createNativePage(url, candidatePage, tab);
} }
/** Destroy and unhook objects at destruction. */
public void destroy() {
if (mNativePageFactory != null) mNativePageFactory.destroy();
}
} }
...@@ -229,4 +229,9 @@ public class NativePageFactory { ...@@ -229,4 +229,9 @@ public class NativePageFactory {
return new BrowserControlsMarginSupplier(mBrowserControlsStateProvider); return new BrowserControlsMarginSupplier(mBrowserControlsStateProvider);
} }
} }
/** Destroy and unhook objects at destruction. */
public void destroy() {
if (mNewTabPageUma != null) mNewTabPageUma.destroy();
}
} }
...@@ -156,6 +156,7 @@ public class NewTabPageUma { ...@@ -156,6 +156,7 @@ public class NewTabPageUma {
private final Supplier<Long> mLastInteractionTime; private final Supplier<Long> mLastInteractionTime;
private final boolean mActivityHadWarmStart; private final boolean mActivityHadWarmStart;
private final Supplier<Intent> mActivityIntent; private final Supplier<Intent> mActivityIntent;
private TabCreationRecorder mTabCreationRecorder;
/** /**
* Constructor. * Constructor.
...@@ -216,7 +217,8 @@ public class NewTabPageUma { ...@@ -216,7 +217,8 @@ public class NewTabPageUma {
* users navigate back to already opened NTPs. * users navigate back to already opened NTPs.
*/ */
public void monitorNTPCreation() { public void monitorNTPCreation() {
mTabModelSelector.addObserver(new TabCreationRecorder()); mTabCreationRecorder = new TabCreationRecorder();
mTabModelSelector.addObserver(mTabCreationRecorder);
} }
/** /**
...@@ -321,4 +323,9 @@ public class NewTabPageUma { ...@@ -321,4 +323,9 @@ public class NewTabPageUma {
} }
}); });
} }
/** Destroy and unhook objects at destruction. */
public void destroy() {
if (mTabCreationRecorder != null) mTabModelSelector.removeObserver(mTabCreationRecorder);
}
} }
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