Commit 728d678a authored by Matt Jones's avatar Matt Jones Committed by Commit Bot

ReaderModeManager is now a tab helper

This patch converts the ReaderModeManager to be a tab helper, replacing
access of the TabModelSelector with a single tab.

Bug: 1069815, 1051184
Change-Id: I3f16a4b053aed006ed8b232393d12396ad7e24fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2151469Reviewed-by: default avatarTommy Nyquist <nyquist@chromium.org>
Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759816}
parent 575129fd
...@@ -6,6 +6,7 @@ include_rules = [ ...@@ -6,6 +6,7 @@ include_rules = [
"+chrome/browser/flags/android", "+chrome/browser/flags/android",
"+chrome/browser/profiles/android/java", "+chrome/browser/profiles/android/java",
"+chrome/browser/share/android", "+chrome/browser/share/android",
"+chrome/browser/tab",
"+chrome/browser/thumbnail/generator/android/java", "+chrome/browser/thumbnail/generator/android/java",
"+chrome/browser/ui/android/appmenu", "+chrome/browser/ui/android/appmenu",
"-chrome/browser/ui/android/appmenu/internal", "-chrome/browser/ui/android/appmenu/internal",
......
...@@ -73,7 +73,6 @@ import org.chromium.chrome.browser.dependency_injection.ChromeActivityComponent; ...@@ -73,7 +73,6 @@ import org.chromium.chrome.browser.dependency_injection.ChromeActivityComponent;
import org.chromium.chrome.browser.dependency_injection.ModuleFactoryOverrides; import org.chromium.chrome.browser.dependency_injection.ModuleFactoryOverrides;
import org.chromium.chrome.browser.device.DeviceClassManager; import org.chromium.chrome.browser.device.DeviceClassManager;
import org.chromium.chrome.browser.dom_distiller.DomDistillerUIUtils; import org.chromium.chrome.browser.dom_distiller.DomDistillerUIUtils;
import org.chromium.chrome.browser.dom_distiller.ReaderModeManager;
import org.chromium.chrome.browser.download.DownloadManagerService; import org.chromium.chrome.browser.download.DownloadManagerService;
import org.chromium.chrome.browser.download.DownloadUtils; import org.chromium.chrome.browser.download.DownloadUtils;
import org.chromium.chrome.browser.download.items.OfflineContentAggregatorNotificationBridgeUiFactory; import org.chromium.chrome.browser.download.items.OfflineContentAggregatorNotificationBridgeUiFactory;
...@@ -253,7 +252,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent> ...@@ -253,7 +252,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
new ObservableSupplierImpl<>(); new ObservableSupplierImpl<>();
private InsetObserverView mInsetObserverView; private InsetObserverView mInsetObserverView;
private ContextualSearchManager mContextualSearchManager; private ContextualSearchManager mContextualSearchManager;
protected ReaderModeManager mReaderModeManager;
private SnackbarManager mSnackbarManager; private SnackbarManager mSnackbarManager;
private UpdateNotificationController mUpdateNotificationController; private UpdateNotificationController mUpdateNotificationController;
...@@ -745,10 +743,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent> ...@@ -745,10 +743,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
mContextualSearchManager = new ContextualSearchManager(this, this); mContextualSearchManager = new ContextualSearchManager(this, this);
} }
if (ReaderModeManager.isEnabled(this)) {
mReaderModeManager = new ReaderModeManager(getTabModelSelector());
}
TraceEvent.end("ChromeActivity:CompositorInitialization"); TraceEvent.end("ChromeActivity:CompositorInitialization");
} }
...@@ -1201,11 +1195,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent> ...@@ -1201,11 +1195,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
@SuppressLint("NewApi") @SuppressLint("NewApi")
@Override @Override
protected final void onDestroy() { protected final void onDestroy() {
if (mReaderModeManager != null) {
mReaderModeManager.destroy();
mReaderModeManager = null;
}
if (mContextualSearchManager != null) { if (mContextualSearchManager != null) {
mContextualSearchManager.destroy(); mContextualSearchManager.destroy();
mContextualSearchManager = null; mContextualSearchManager = null;
...@@ -1641,14 +1630,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent> ...@@ -1641,14 +1630,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
return mContextualSearchManager; return mContextualSearchManager;
} }
/**
* @return The {@code ReaderModeManager} or {@code null} if none;
*/
@VisibleForTesting
public ReaderModeManager getReaderModeManager() {
return mReaderModeManager;
}
/** /**
* Create a full-screen manager to be used by this activity. * Create a full-screen manager to be used by this activity.
* Note: This may be called before native code is initialized. * Note: This may be called before native code is initialized.
......
...@@ -1999,7 +1999,7 @@ public class ChromeTabbedActivity ...@@ -1999,7 +1999,7 @@ public class ChromeTabbedActivity
return getTabCreator(isIncognito).createNewTab(loadUrlParams, launchType, null, intent); return getTabCreator(isIncognito).createNewTab(loadUrlParams, launchType, null, intent);
} else { } else {
// Check if the tab is being created from a Reader Mode navigation. // Check if the tab is being created from a Reader Mode navigation.
if (ReaderModeManager.isEnabled(this) if (ReaderModeManager.isEnabled()
&& ReaderModeManager.isReaderModeCreatedIntent(intent)) { && ReaderModeManager.isReaderModeCreatedIntent(intent)) {
Bundle extras = intent.getExtras(); Bundle extras = intent.getExtras();
int readerParentId = IntentUtils.safeGetInt( int readerParentId = IntentUtils.safeGetInt(
......
...@@ -20,17 +20,6 @@ import org.chromium.ui.base.WindowAndroid; ...@@ -20,17 +20,6 @@ import org.chromium.ui.base.WindowAndroid;
*/ */
@JNINamespace("dom_distiller::android") @JNINamespace("dom_distiller::android")
public final class DomDistillerUIUtils { public final class DomDistillerUIUtils {
// Static handle to Reader Mode's manager.
private static ReaderModeManager sManagerManager;
/**
* Set the delegate to the ReaderModeManager.
* @param manager The class managing Reader Mode.
*/
public static void setReaderModeManagerDelegate(ReaderModeManager manager) {
sManagerManager = manager;
}
/** /**
* A static method for native code to call to open the distiller UI settings. * A static method for native code to call to open the distiller UI settings.
* @param webContents The WebContents containing the distilled content. * @param webContents The WebContents containing the distilled content.
...@@ -38,6 +27,7 @@ public final class DomDistillerUIUtils { ...@@ -38,6 +27,7 @@ public final class DomDistillerUIUtils {
@CalledByNative @CalledByNative
public static void openSettings(WebContents webContents) { public static void openSettings(WebContents webContents) {
Activity activity = getActivityFromWebContents(webContents); Activity activity = getActivityFromWebContents(webContents);
if (webContents != null && activity != null) { if (webContents != null && activity != null) {
RecordUserAction.record("DomDistiller_DistilledPagePrefsOpened"); RecordUserAction.record("DomDistiller_DistilledPagePrefsOpened");
AlertDialog.Builder builder = AlertDialog.Builder builder =
...@@ -47,16 +37,6 @@ public final class DomDistillerUIUtils { ...@@ -47,16 +37,6 @@ public final class DomDistillerUIUtils {
} }
} }
/**
* Clear static references to objects.
* @param manager The manager requesting the destoy. This prevents different managers in
* document mode from accidentally clearing a reference it doesn't own.
*/
public static void destroy(ReaderModeManager manager) {
if (manager != sManagerManager) return;
sManagerManager = null;
}
/** /**
* @param webContents The WebContents to get the Activity from. * @param webContents The WebContents to get the Activity from.
* @return The Activity associated with the WebContents. * @return The Activity associated with the WebContents.
......
...@@ -16,12 +16,10 @@ import org.chromium.base.ApiCompatibilityUtils; ...@@ -16,12 +16,10 @@ import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.NativeMethods; import org.chromium.base.annotations.NativeMethods;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.compositor.bottombar.OverlayPanel; import org.chromium.chrome.browser.compositor.bottombar.OverlayPanel;
import org.chromium.chrome.browser.compositor.bottombar.OverlayPanel.StateChangeReason; import org.chromium.chrome.browser.compositor.bottombar.OverlayPanel.StateChangeReason;
import org.chromium.chrome.browser.dom_distiller.ReaderModeManager; import org.chromium.chrome.browser.dom_distiller.ReaderModeManager;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabUtils;
import org.chromium.chrome.browser.ui.messages.infobar.InfoBarCompactLayout; import org.chromium.chrome.browser.ui.messages.infobar.InfoBarCompactLayout;
import org.chromium.components.browser_ui.widget.text.AccessibleTextView; import org.chromium.components.browser_ui.widget.text.AccessibleTextView;
...@@ -104,6 +102,7 @@ public class ReaderModeInfoBar extends InfoBar { ...@@ -104,6 +102,7 @@ public class ReaderModeInfoBar extends InfoBar {
/** @return The tab that this infobar is showing for. */ /** @return The tab that this infobar is showing for. */
private Tab getTab() { private Tab getTab() {
if (getNativeInfoBarPtr() == 0) return null;
return ReaderModeInfoBarJni.get().getTab(getNativeInfoBarPtr(), ReaderModeInfoBar.this); return ReaderModeInfoBarJni.get().getTab(getNativeInfoBarPtr(), ReaderModeInfoBar.this);
} }
...@@ -111,14 +110,9 @@ public class ReaderModeInfoBar extends InfoBar { ...@@ -111,14 +110,9 @@ public class ReaderModeInfoBar extends InfoBar {
* @return The {@link ReaderModeManager} for this infobar. * @return The {@link ReaderModeManager} for this infobar.
*/ */
private ReaderModeManager getReaderModeManager() { private ReaderModeManager getReaderModeManager() {
if (getNativeInfoBarPtr() == 0) return null; Tab tab = getTab();
if (tab == null) return null;
// TODO(1069815): Once ReaderModeManager is a tab helper, replace this cast and pull from return tab.getUserDataHost().getUserData(ReaderModeManager.USER_DATA_KEY);
// tab directly and remove any use of the activity.
ChromeActivity activity = (ChromeActivity) TabUtils.getActivity(getTab());
if (activity == null) return null;
return activity.getReaderModeManager();
} }
/** /**
......
...@@ -4,6 +4,7 @@ include_rules = [ ...@@ -4,6 +4,7 @@ include_rules = [
"+chrome/android/java/src/org/chromium/chrome/browser/AppHooks.java", "+chrome/android/java/src/org/chromium/chrome/browser/AppHooks.java",
"+chrome/android/java/src/org/chromium/chrome/browser/TabHidingType.java", "+chrome/android/java/src/org/chromium/chrome/browser/TabHidingType.java",
"+chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/ephemeraltab/EphemeralTabCoordinator.java", "+chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/ephemeraltab/EphemeralTabCoordinator.java",
"+chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java",
"+chrome/android/java/src/org/chromium/chrome/browser/incognito/IncognitoUtils.java", "+chrome/android/java/src/org/chromium/chrome/browser/incognito/IncognitoUtils.java",
"+chrome/android/java/src/org/chromium/chrome/browser/paint_preview/PaintPreviewTabHelper.java", "+chrome/android/java/src/org/chromium/chrome/browser/paint_preview/PaintPreviewTabHelper.java",
"+chrome/android/java/src/org/chromium/chrome/browser/previews/Previews.java", "+chrome/android/java/src/org/chromium/chrome/browser/previews/Previews.java",
......
...@@ -8,6 +8,7 @@ import org.chromium.chrome.browser.SwipeRefreshHandler; ...@@ -8,6 +8,7 @@ import org.chromium.chrome.browser.SwipeRefreshHandler;
import org.chromium.chrome.browser.complex_tasks.TaskTabHelper; import org.chromium.chrome.browser.complex_tasks.TaskTabHelper;
import org.chromium.chrome.browser.contextualsearch.ContextualSearchTabHelper; import org.chromium.chrome.browser.contextualsearch.ContextualSearchTabHelper;
import org.chromium.chrome.browser.crypto.CipherFactory; import org.chromium.chrome.browser.crypto.CipherFactory;
import org.chromium.chrome.browser.dom_distiller.ReaderModeManager;
import org.chromium.chrome.browser.dom_distiller.TabDistillabilityProvider; import org.chromium.chrome.browser.dom_distiller.TabDistillabilityProvider;
import org.chromium.chrome.browser.infobar.InfoBarContainer; import org.chromium.chrome.browser.infobar.InfoBarContainer;
import org.chromium.chrome.browser.media.ui.MediaSessionTabHelper; import org.chromium.chrome.browser.media.ui.MediaSessionTabHelper;
...@@ -34,6 +35,7 @@ public final class TabHelpers { ...@@ -34,6 +35,7 @@ public final class TabHelpers {
TaskTabHelper.createForTab(tab, parentTab); TaskTabHelper.createForTab(tab, parentTab);
TabBrowserControlsConstraintsHelper.createForTab(tab); TabBrowserControlsConstraintsHelper.createForTab(tab);
PaintPreviewTabHelper.createForTab(tab); PaintPreviewTabHelper.createForTab(tab);
if (ReaderModeManager.isEnabled()) ReaderModeManager.createForTab(tab);
// TODO(jinsukkim): Do this by having something observe new tab creation. // TODO(jinsukkim): Do this by having something observe new tab creation.
if (tab.isIncognito()) CipherFactory.getInstance().triggerKeyGeneration(); if (tab.isIncognito()) CipherFactory.getInstance().triggerKeyGeneration();
......
...@@ -91,7 +91,9 @@ public class ReaderModeTest { ...@@ -91,7 +91,9 @@ public class ReaderModeTest {
assertThat(innerHtml).doesNotContain("article-header"); assertThat(innerHtml).doesNotContain("article-header");
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
mActivityTestRule.getActivity().getReaderModeManager().activateReaderMode(originalTab); originalTab.getUserDataHost()
.getUserData(ReaderModeManager.USER_DATA_KEY)
.activateReaderMode(originalTab);
}); });
CustomTabActivity customTabActivity = waitForCustomTabActivity(); CustomTabActivity customTabActivity = waitForCustomTabActivity();
CriteriaHelper.pollUiThread( CriteriaHelper.pollUiThread(
...@@ -118,7 +120,9 @@ public class ReaderModeTest { ...@@ -118,7 +120,9 @@ public class ReaderModeTest {
assertThat(innerHtml).doesNotContain("article-header"); assertThat(innerHtml).doesNotContain("article-header");
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
mActivityTestRule.getActivity().getReaderModeManager().activateReaderMode(originalTab); originalTab.getUserDataHost()
.getUserData(ReaderModeManager.USER_DATA_KEY)
.activateReaderMode(originalTab);
}); });
CustomTabActivity customTabActivity = waitForCustomTabActivity(); CustomTabActivity customTabActivity = waitForCustomTabActivity();
CriteriaHelper.pollUiThread( CriteriaHelper.pollUiThread(
...@@ -141,7 +145,9 @@ public class ReaderModeTest { ...@@ -141,7 +145,9 @@ public class ReaderModeTest {
assertThat(innerHtml).doesNotContain("article-header"); assertThat(innerHtml).doesNotContain("article-header");
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
mActivityTestRule.getActivity().getReaderModeManager().activateReaderMode(tab); tab.getUserDataHost()
.getUserData(ReaderModeManager.USER_DATA_KEY)
.activateReaderMode(tab);
}); });
waitForDistillation(TITLE, mActivityTestRule.getActivity().getActivityTab()); waitForDistillation(TITLE, mActivityTestRule.getActivity().getActivityTab());
......
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