chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java - Issue 2878543003: Hook up Reader Mode InfoBar

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java

Issue 2878543003: Hook up Reader Mode InfoBar (Closed)

Patch Set: fix disabled test Created 3 years, 7 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/readermode/ReaderModePanel.java ('K') | « chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabLayoutManager.java ('k') | chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManagerDelegate.java » ('j') | chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManagerDelegate.java » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java b/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java

index 141a6837ebf1b95787131aa8bc9debf7e4d6be8c..d229033a4f3cbb6b829336753631112ef8d09230 100644

--- a/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java

+++ b/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java

@@ -8,18 +8,12 @@ import android.content.Context;

import android.text.TextUtils;

import org.chromium.base.CommandLine;

-import org.chromium.base.SysUtils;

-import org.chromium.base.VisibleForTesting;

import org.chromium.base.library_loader.LibraryLoader;

import org.chromium.base.metrics.RecordHistogram;

import org.chromium.chrome.browser.ChromeActivity;

import org.chromium.chrome.browser.ChromeSwitches;

-import org.chromium.chrome.browser.compositor.bottombar.OverlayPanel.PanelState;

import org.chromium.chrome.browser.compositor.bottombar.OverlayPanel.StateChangeReason;

-import org.chromium.chrome.browser.compositor.bottombar.readermode.ReaderModePanel;

-import org.chromium.chrome.browser.infobar.InfoBar;

-import org.chromium.chrome.browser.infobar.InfoBarContainer;

-import org.chromium.chrome.browser.infobar.InfoBarContainer.InfoBarContainerObserver;

+import org.chromium.chrome.browser.infobar.ReaderModeInfoBar;

import org.chromium.chrome.browser.rappor.RapporServiceBridge;

import org.chromium.chrome.browser.tab.Tab;

import org.chromium.chrome.browser.tabmodel.TabCreatorManager;

@@ -27,14 +21,14 @@ import org.chromium.chrome.browser.tabmodel.TabModel;

import org.chromium.chrome.browser.tabmodel.TabModelSelector;

import org.chromium.chrome.browser.tabmodel.TabModelSelectorTabObserver;

import org.chromium.chrome.browser.util.AccessibilityUtil;

-import org.chromium.chrome.browser.widget.findinpage.FindToolbarObserver;

import org.chromium.components.dom_distiller.content.DistillablePageUtils;

import org.chromium.components.dom_distiller.core.DomDistillerUrlUtils;

import org.chromium.content_public.browser.LoadUrlParams;

+import org.chromium.content_public.browser.NavigationController;

+import org.chromium.content_public.browser.NavigationEntry;

import org.chromium.content_public.browser.WebContents;

import org.chromium.content_public.browser.WebContentsObserver;

import org.chromium.ui.UiUtils;

-import org.chromium.ui.base.DeviceFormFactor;

import org.chromium.ui.base.PageTransition;

import java.util.HashMap;

@@ -46,55 +40,43 @@ import java.util.concurrent.TimeUnit;

* reader mode and reader mode preferences toolbar icon and hiding the

* browser controls when a reader mode page has finished loading.

-public class ReaderModeManager extends TabModelSelectorTabObserver

- implements InfoBarContainerObserver, ReaderModeManagerDelegate {

- /**

- * POSSIBLE means reader mode can be entered.

- */

+public class ReaderModeManager

+ extends TabModelSelectorTabObserver implements ReaderModeManagerDelegate {

+ /** POSSIBLE means reader mode can be entered. */

public static final int POSSIBLE = 0;

- /**

- * NOT_POSSIBLE means reader mode cannot be entered.

- */

+ /** NOT_POSSIBLE means reader mode cannot be entered. */

public static final int NOT_POSSIBLE = 1;

- /**

- * STARTED means reader mode is currently in reader mode.

- */

+ /** STARTED means reader mode is currently in reader mode. */

public static final int STARTED = 2;

- // The url of the last page visited if the last page was reader mode page. Otherwise null.

+ /** The url of the last page visited if the last page was reader mode page. Otherwise null. */

private String mReaderModePageUrl;

- // Whether the fact that the current web page was distillable or not has been recorded.

+ /** Whether the fact that the current web page was distillable or not has been recorded. */

private boolean mIsUmaRecorded;

- // The per-tab state of distillation.

+ /** The per-tab state of distillation. */

protected Map<Integer, ReaderModeTabInfo> mTabStatusMap;

- // The current tab ID. This will change as the user switches between tabs.

+ /** The current tab ID. This will change as the user switches between tabs. */

private int mTabId;

- // The ReaderModePanel that this class is managing.

- protected ReaderModePanel mReaderModePanel;

- // The ChromeActivity that this panel exists in.

+ /** The ChromeActivity that this panel exists in. */

private ChromeActivity mChromeActivity;

- // The primary means of getting the currently active tab.

+ /** The primary means of getting the currently active tab. */

private TabModelSelector mTabModelSelector;

- private boolean mIsFullscreenModeEntered;

- private boolean mIsFindToolbarShowing;

- private boolean mIsKeyboardShowing;

- // InfoBar tracking.

- private boolean mIsInfoBarContainerShown;

- // If Reader Mode is detecting all pages as distillable.

+ /** If Reader Mode is detecting all pages as distillable. */

private boolean mIsReaderHeuristicAlwaysTrue;

+ /** Whether or not the previous navigation should be removed. */

+ private boolean mShouldRemovePreviousNavigation;

+ /** The index of the last committed distiller page in history. */

+ private int mLastDistillerPageIndex;

public ReaderModeManager(TabModelSelector selector, ChromeActivity activity) {

super(selector);

@@ -129,29 +111,9 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

DomDistillerUIUtils.destroy(this);

mChromeActivity = null;

- mReaderModePanel = null;

mTabModelSelector = null;

}

- /**

- * @return A FindToolbarObserver capable of hiding the Reader Mode panel.

- */

- public FindToolbarObserver getFindToolbarObserver() {

- return new FindToolbarObserver() {

- @Override

- public void onFindToolbarShown() {

- mIsFindToolbarShowing = true;

- closeReaderPanel(StateChangeReason.UNKNOWN, true);

- }

- @Override

- public void onFindToolbarHidden() {

- mIsFindToolbarShowing = false;

- requestReaderPanelShow(StateChangeReason.UNKNOWN);

- }

- };

- }

// TabModelSelectorTabObserver:

@Override

@@ -159,7 +121,6 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

if (mTabModelSelector == null) return;

int shownTabId = shownTab.getId();

- Tab previousTab = mTabModelSelector.getTabById(mTabId);

mTabId = shownTabId;

// If the reader panel was dismissed, stop here.

@@ -171,20 +132,6 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

// Set this manager as the active one for the UI utils.

DomDistillerUIUtils.setReaderModeManagerDelegate(this);

- // Update infobar state based on current tab.

- if (shownTab.getInfoBarContainer() != null) {

- mIsInfoBarContainerShown = shownTab.getInfoBarContainer().hasInfoBars();

- }

- // Remove the infobar observer from the previous tab and attach it to the current one.

- if (previousTab != null && previousTab.getInfoBarContainer() != null) {

- previousTab.getInfoBarContainer().removeObserver(this);

- }

- if (shownTab.getInfoBarContainer() != null) {

- shownTab.getInfoBarContainer().addObserver(this);

- }

// If there is no state info for this tab, create it.

ReaderModeTabInfo tabInfo = mTabStatusMap.get(shownTabId);

if (tabInfo == null) {

@@ -202,7 +149,7 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

// Make sure there is a distillability delegate set on the WebContents.

setDistillabilityCallback(shownTabId);

- requestReaderPanelShow(StateChangeReason.UNKNOWN);

+ tryShowingInfoBar();

}

@Override

@@ -213,9 +160,7 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

@Override

public void onDestroyed(Tab tab) {

if (tab == null) return;

- if (tab.getInfoBarContainer() != null) {

- tab.getInfoBarContainer().removeObserver(this);

- }

// If the panel was not shown for the previous navigation, record it now.

ReaderModeTabInfo info = mTabStatusMap.get(tab.getId());

if (info != null && !info.isPanelShowRecorded()) {

@@ -224,6 +169,33 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

removeTabState(tab.getId());

}

+ @Override

+ public void onPageLoadStarted(Tab tab, String url) {

+ // Reader Mode should not pollute the navigation stack. To avoid this, watch for navigations

+ // and prepare to remove any that are "chrome-distiller" urls.

+ NavigationController controller = tab.getWebContents().getNavigationController();

+ int index = controller.getLastCommittedEntryIndex();

+ NavigationEntry entry = controller.getEntryAtIndex(index);

+ if (entry == null) return;

+ if (DomDistillerUrlUtils.isDistilledPage(entry.getUrl())) {

+ mShouldRemovePreviousNavigation = true;

+ mLastDistillerPageIndex = index;

+ }

+ @Override

+ public void onDidFinishNavigation(Tab tab, String url, boolean isInMainFrame,

+ boolean isErrorPage, boolean hasCommitted, boolean isSameDocument,

+ boolean isFragmentNavigation, Integer pageTransition, int errorCode,

+ int httpStatusCode) {

+ if (!hasCommitted || !mShouldRemovePreviousNavigation || !isInMainFrame) return;

+ mShouldRemovePreviousNavigation = false;

+ NavigationController controller = tab.getWebContents().getNavigationController();

+ controller.removeEntryAtIndex(mLastDistillerPageIndex);

+ }

/**

* Clean up the state associated with a tab.

* @param tabId The target tab ID.

@@ -264,62 +236,10 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

// Make sure there is a distillability delegate set on the WebContents.

setDistillabilityCallback(tab.getId());

}

- if (tab.getInfoBarContainer() != null) tab.getInfoBarContainer().addObserver(this);

- }

- @Override

- public void onToggleFullscreenMode(Tab tab, boolean enable) {

- // Temporarily hide the reader mode panel while fullscreen is enabled.

- if (enable) {

- mIsFullscreenModeEntered = true;

- closeReaderPanel(StateChangeReason.FULLSCREEN_ENTERED, false);

- } else {

- mIsFullscreenModeEntered = false;

- requestReaderPanelShow(StateChangeReason.FULLSCREEN_EXITED);

- }

- // InfoBarContainerObserver:

- @Override

- public void onAddInfoBar(InfoBarContainer container, InfoBar infoBar, boolean isFirst) {

- mIsInfoBarContainerShown = true;

- // If the panel is opened past the peeking state, obscure the infobar.

- if (mReaderModePanel != null && mReaderModePanel.isPanelOpened() && container != null) {

- container.setIsObscuredByOtherView(true);

- } else if (isFirst) {

- // Temporarily hides the reader mode button while the infobars are shown.

- closeReaderPanel(StateChangeReason.INFOBAR_SHOWN, false);

- }

- @Override

- public void onRemoveInfoBar(InfoBarContainer container, InfoBar infoBar, boolean isLast) {

- // Re-shows the reader mode button if necessary once the infobars are dismissed.

- if (isLast) {

- mIsInfoBarContainerShown = false;

- requestReaderPanelShow(StateChangeReason.INFOBAR_HIDDEN);

- }

- @Override

- public void onInfoBarContainerAttachedToWindow(boolean hasInfoBars) {

- mIsInfoBarContainerShown = hasInfoBars;

- if (mIsInfoBarContainerShown) {

- closeReaderPanel(StateChangeReason.INFOBAR_SHOWN, false);

- } else {

- requestReaderPanelShow(StateChangeReason.INFOBAR_HIDDEN);

- }

}

// ReaderModeManagerDelegate:

- @Override

- public void setReaderModePanel(ReaderModePanel panel) {

- mReaderModePanel = panel;

- }

@Override

public ChromeActivity getChromeActivity() {

return mChromeActivity;

@@ -351,9 +271,7 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

@Override

public void onClosed(StateChangeReason reason) {

- if (mReaderModePanel == null || mTabModelSelector == null) return;

- restoreInfobars();

+ if (mTabModelSelector == null) return;

// Only dismiss the panel if the close was a result of user interaction.

if (reason != StateChangeReason.FLING && reason != StateChangeReason.SWIPE

Theresa 2017/05/11 22:31:19 StateChangeReason doesn't really apply anymore rig

mdjones 2017/05/12 19:13:10 Done.

@@ -361,41 +279,13 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

return;

}

- // Record close button usage.

- if (reason == StateChangeReason.CLOSE_BUTTON) {

- RecordHistogram.recordBooleanHistogram("DomDistiller.BarCloseButtonUsage",

- mReaderModePanel.getPanelState() == PanelState.EXPANDED

- || mReaderModePanel.getPanelState() == PanelState.MAXIMIZED);

- }

+ RecordHistogram.recordBooleanHistogram("DomDistiller.InfoBarUsage", false);

int currentTabId = mTabModelSelector.getCurrentTabId();

if (!mTabStatusMap.containsKey(currentTabId)) return;

mTabStatusMap.get(currentTabId).setIsDismissed(true);

}

- @Override

- public void onPeek() {

- restoreInfobars();

- }

- /**

- * Restore any infobars that may have been hidden by Reader Mode.

- */

- private void restoreInfobars() {

- if (!mIsInfoBarContainerShown) return;

- Tab curTab = mTabModelSelector.getCurrentTab();

- if (curTab == null) return;

- InfoBarContainer container = curTab.getInfoBarContainer();

- if (container == null) return;

- container.setIsObscuredByOtherView(false);

- // Temporarily hides the reader mode button while the infobars are shown.

- closeReaderPanel(StateChangeReason.INFOBAR_SHOWN, false);

- }

@Override

public WebContents getBasePageWebContents() {

Tab tab = mTabModelSelector.getCurrentTab();

@@ -406,8 +296,7 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

@Override

public void closeReaderPanel(StateChangeReason reason, boolean animate) {

- if (mReaderModePanel == null) return;

- mReaderModePanel.closePanel(reason, animate);

+ // TODO(mdjones): Remove this method and dependencies.

Theresa 2017/05/11 22:31:19 InfoBar handles automatically dismissing itself?

mdjones 2017/05/12 19:13:10 In most cases where we want it, yes. That's the ni

}

@Override

@@ -416,17 +305,6 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

timeMs, TimeUnit.MILLISECONDS);

}

- @Override

- public void onLayoutChanged() {

- if (isKeyboardShowing()) {

- mIsKeyboardShowing = true;

- closeReaderPanel(StateChangeReason.KEYBOARD_SHOWN, false);

- } else if (mIsKeyboardShowing) {

- mIsKeyboardShowing = false;

- requestReaderPanelShow(StateChangeReason.KEYBOARD_HIDDEN);

- }

/**

* @return True if the keyboard might be showing. This is not 100% accurate; see

* UiUtils.isKeyboardShowing(...).

@@ -470,7 +348,6 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

// TODO(cjhopman): This should possibly ignore navigations that replace the entry

// (like those from history.replaceState()).

if (!hasCommitted || !isInMainFrame || isSameDocument) return;

- if (DomDistillerUrlUtils.isDistilledPage(url)) return;

// Make sure the tab was not destroyed.

ReaderModeTabInfo tabInfo = mTabStatusMap.get(readerTabId);

@@ -488,7 +365,7 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

if (tabInfo.getStatus() != POSSIBLE) {

closeReaderPanel(StateChangeReason.UNKNOWN, false);

} else {

- requestReaderPanelShow(StateChangeReason.UNKNOWN);

+ tryShowingInfoBar();

}

@@ -512,11 +389,9 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

}

/**

- * This is a wrapper for "requestPanelShow" that checks if reader mode is possible before

- * showing.

- * @param reason The reason the panel is requesting to be shown.

+ * Try showing the reader mode infobar.

- protected void requestReaderPanelShow(StateChangeReason reason) {

+ protected void tryShowingInfoBar() {

if (mTabModelSelector == null) return;

int currentTabId = mTabModelSelector.getCurrentTabId();

@@ -528,19 +403,29 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

&& getBasePageWebContents().getNavigationController().getUseDesktopUserAgent()

&& !mIsReaderHeuristicAlwaysTrue;

- if (mReaderModePanel == null || !mTabStatusMap.containsKey(currentTabId)

- || usingRequestDesktopSite

+ if (!mTabStatusMap.containsKey(currentTabId) || usingRequestDesktopSite

|| mTabStatusMap.get(currentTabId).getStatus() != POSSIBLE

|| mTabStatusMap.get(currentTabId).isDismissed()

- || mIsInfoBarContainerShown

- || mIsFindToolbarShowing

- || mIsFullscreenModeEntered

- || mIsKeyboardShowing

|| AccessibilityUtil.isAccessibilityEnabled()) {

return;

}

- mReaderModePanel.requestPanelShow(reason);

+ ReaderModeInfoBar.showReaderModeInfoBar(mTabModelSelector.getCurrentTab(), this);

+ }

+ /**

+ * Navigate the current tab to a Reader Mode URL.

+ */

+ public void navigateToReaderMode() {

+ RecordHistogram.recordBooleanHistogram("DomDistiller.InfoBarUsage", true);

+ WebContents baseWebContents = getBasePageWebContents();

+ if (baseWebContents == null || mChromeActivity == null || mTabModelSelector == null) return;

+ String url = baseWebContents.getUrl();

+ if (url == null) return;

+ DomDistillerTabUtils.distillCurrentPageAndView(getBasePageWebContents());

}

/**

@@ -562,22 +447,6 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

TabModel.TabLaunchType.FROM_LINK, mChromeActivity.getActivityTab());

}

- /**

- * @return Whether the Reader Mode panel is opened (state is EXPANDED or MAXIMIZED).

- */

- public boolean isPanelOpened() {

- if (mReaderModePanel == null) return false;

- return mReaderModePanel.isPanelOpened();

- }

- /**

- * @return The ReaderModePanel for testing.

- */

- @VisibleForTesting

- public ReaderModePanel getPanelForTesting() {

- return mReaderModePanel;

- }

/**

* Set the callback for updating reader mode status based on whether or not the page should

* be viewed in reader mode.

@@ -617,8 +486,7 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

tabInfo.setStatus(POSSIBLE);

// The user may have changed tabs.

if (tabId == mTabModelSelector.getCurrentTabId()) {

- // TODO(mdjones): Add reason DISTILLER_STATE_CHANGE.

- requestReaderPanelShow(StateChangeReason.UNKNOWN);

+ tryShowingInfoBar();

}

} else {

tabInfo.setStatus(NOT_POSSIBLE);

@@ -643,10 +511,8 @@ public class ReaderModeManager extends TabModelSelectorTabObserver

boolean enabled = CommandLine.getInstance().hasSwitch(ChromeSwitches.ENABLE_DOM_DISTILLER)

&& !CommandLine.getInstance().hasSwitch(

- ChromeSwitches.DISABLE_READER_MODE_BOTTOM_BAR)

- && !DeviceFormFactor.isTablet(context)

- && DomDistillerTabUtils.isDistillerHeuristicsEnabled()

- && !SysUtils.isLowEndDevice();

+ ChromeSwitches.DISABLE_READER_MODE_BOTTOM_BAR)

+ && DomDistillerTabUtils.isDistillerHeuristicsEnabled();

return enabled;

}