[Chromeshine] Stop using the page-provided canonical URL

This means we won't handle AMP pages that don't use SXG(signed exchanges).
Trusting the canonical URL blindly has potential privacy risks. Trusting only
Google-provided AMP urls is also the wrong solution; we want to rely on the
long term solution that is SXG.

(cherry picked from commit 0e604b323671c3be03fbf0836f7c26b54ca89955)

Bug: 961814
Change-Id: Ib3544e8c478a3a9089d8c7414f7ea42bb65b8794
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1659511
Reviewed-by: Sky Malice <skym@chromium.org>
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#669854}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1665651
Reviewed-by: Patrick Noland <pnoland@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#433}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/usage_stats/PageViewObserver.java b/chrome/android/java/src/org/chromium/chrome/browser/usage_stats/PageViewObserver.java
index 0865c87..dd26d4a 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/usage_stats/PageViewObserver.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/usage_stats/PageViewObserver.java

@@ -21,7 +21,6 @@
 import org.chromium.chrome.browser.tabmodel.TabModelSelector;
 import org.chromium.chrome.browser.tabmodel.TabModelSelectorTabModelObserver;
 import org.chromium.chrome.browser.tabmodel.TabSelectionType;
-import org.chromium.content_public.browser.NavigationHandle;
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
@@ -58,7 +57,7 @@
             @Override
             public void onShown(Tab tab, @TabSelectionType int type) {
                 if (!tab.isLoading() && !tab.isBeingRestored()) {
-                    updateUsingCanonicalUrl(tab);
+                    updateUrl(tab.getUrl());
                 }
             }
 
@@ -82,25 +81,13 @@
             public void didFirstVisuallyNonEmptyPaint(Tab tab) {
                 assert tab == mCurrentTab;
 
-                updateUsingCanonicalUrl(tab);
+                updateUrl(tab.getUrl());
             }
 
             @Override
             public void onCrash(Tab tab) {
                 updateUrl(null);
             }
-
-            @Override
-            public void onDidFinishNavigation(Tab tab, NavigationHandle navigation) {
-                // We only check isLikelySubframeAmpNavigation here because this is the only
-                // observer method that fires for subframe navigations. The reason we have the
-                // isLikelySubframeAmpNavigation at all is that onDidFinishNavigation triggers very
-                // often, and we only want to bother getting the canonical URL if we think there's a
-                // good chance that it will actually be present.
-                if (isLikelySubframeAmpNavigation(navigation)) {
-                    updateUsingCanonicalUrl(tab);
-                }
-            }
         };
 
         mTabModelObserver = new TabModelSelectorTabModelObserver(tabModelSelector) {
@@ -146,22 +133,6 @@
         }
     }
 
-    private void updateUsingCanonicalUrl(Tab tab) {
-        if (tab.getWebContents() == null || tab.getWebContents().getMainFrame() == null) {
-            updateUrl(tab.getUrl());
-            return;
-        }
-
-        tab.getWebContents().getMainFrame().getCanonicalUrlForSharing((canonicalUrl) -> {
-            if (tab != mCurrentTab) return;
-            String urlToUse = tab.getUrl();
-            if (canonicalUrl != null && canonicalUrl.length() > 0) {
-                urlToUse = canonicalUrl;
-            }
-            updateUrl(urlToUse);
-        });
-    }
-
     /**
      * Updates our state from the previous url to {@code newUrl}. This can result in any/all of the
      * following:
@@ -232,7 +203,7 @@
         // If the newly active tab is hidden, we don't want to check its URL yet; we'll wait until
         // the onShown event fires.
         if (mCurrentTab != null && !tab.isHidden()) {
-            updateUsingCanonicalUrl(tab);
+            updateUrl(tab.getUrl());
         }
     }
 
@@ -274,18 +245,4 @@
         String host = Uri.parse(url).getHost();
         return host == null ? "" : host;
     }
-
-    private boolean isLikelySubframeAmpNavigation(NavigationHandle navigation) {
-        if (navigation.isInMainFrame()) return false;
-        String subframeUrl = navigation.getUrl();
-        if (subframeUrl == null || subframeUrl.length() == 0) return false;
-
-        // Our heuristic for AMP pages, based on AMPPageLoadMetricsObserver, is to look for a
-        // subframe navigation that contains "amp_js_v" in the query. This might produce a false
-        // positive, but we're OK with that; we'll just call updateUrl an extra time. This is a
-        // no-op if the URL hasn't actually changed.
-        String query = Uri.parse(subframeUrl).getQuery();
-        if (query == null) return false;
-        return query.contains(AMP_QUERY_PARAM);
-    }
 }

diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/usage_stats/TabSuspensionTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/usage_stats/TabSuspensionTest.java
index a5a77c8..75229240 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/usage_stats/TabSuspensionTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/usage_stats/TabSuspensionTest.java

@@ -42,15 +42,12 @@
 import org.chromium.components.safe_browsing.SafeBrowsingApiBridge;
 import org.chromium.content_public.browser.LoadUrlParams;
 import org.chromium.content_public.browser.test.util.CriteriaHelper;
-import org.chromium.content_public.browser.test.util.JavaScriptUtils;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import org.chromium.content_public.common.ContentSwitches;
 import org.chromium.net.test.EmbeddedTestServer;
 import org.chromium.ui.base.PageTransition;
 
 import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
 
 /**
  * Integration tests for {@link PageViewObserver} and {@link SuspendedTab}
@@ -63,9 +60,6 @@
 public class TabSuspensionTest {
     private static final String STARTING_FQDN = "example.com";
     private static final String DIFFERENT_FQDN = "www.google.com";
-    private static final String MAINFRAME_TEST_PAGE =
-            "/chrome/test/data/android/usage_stats/amp_root.html";
-    private static final String SUBFRAME_TEST_PAGE = "/chrome/test/data/android/test.html";
 
     @Rule
     public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
@@ -75,13 +69,12 @@
     @Mock
     private UsageStatsBridge mUsageStatsBridge;
     @Mock
-    private EventTracker mEventTracker;
-    @Mock
     private SuspensionTracker mSuspensionTracker;
 
     private ChromeTabbedActivity mActivity;
     private PageViewObserver mPageViewObserver;
     private TokenTracker mTokenTracker;
+    private EventTracker mEventTracker;
     private Tab mTab;
     private EmbeddedTestServer mTestServer;
     private String mStartingUrl;
@@ -90,10 +83,12 @@
     @Before
     public void setUp() throws InterruptedException {
         MockitoAnnotations.initMocks(this);
-        // TokenTracker holds a promise, and Promises can only be used on a single thread, so we
-        // have to initialize it on the thread where it will be used.
-        TestThreadUtils.runOnUiThreadBlocking(
-                () -> { mTokenTracker = new TokenTracker(mUsageStatsBridge); });
+        // TokenTracker and EventTracker hold a promise, and Promises can only be used on a single
+        // thread, so we have to initialize them on the thread where they will be used.
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            mTokenTracker = new TokenTracker(mUsageStatsBridge);
+            mEventTracker = new EventTracker(mUsageStatsBridge);
+        });
         mTestServer = EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext());
         mStartingUrl = mTestServer.getURLWithHostName(STARTING_FQDN, "/defaultresponse");
         mDifferentUrl = mTestServer.getURLWithHostName(DIFFERENT_FQDN, "/defaultresponse");
@@ -286,38 +281,6 @@
         });
     }
 
-    @Test
-    @MediumTest
-    public void testAmpPage() throws InterruptedException, TimeoutException {
-        String mainframeUrl = mTestServer.getURLWithHostName(DIFFERENT_FQDN, MAINFRAME_TEST_PAGE);
-        mActivityTestRule.loadUrl(mainframeUrl);
-        doReturn(true).when(mSuspensionTracker).isWebsiteSuspended(STARTING_FQDN);
-
-        String iframeSrc =
-                mTestServer.getURLWithHostName(STARTING_FQDN, SUBFRAME_TEST_PAGE + "?amp_js_v=0");
-        String code = "document.getElementById('iframe_test_id').src='" + iframeSrc + "';";
-        JavaScriptUtils.executeJavaScriptAndWaitForResult(
-                mTab.getWebContents(), code, 3, TimeUnit.SECONDS);
-        waitForSuspendedTabToShow(mTab, STARTING_FQDN);
-
-        doReturn(false).when(mSuspensionTracker).isWebsiteSuspended(STARTING_FQDN);
-        unsuspendDomain(STARTING_FQDN);
-        assertSuspendedTabHidden(mTab);
-
-        // Un-suspension reloads the page, so we need to wait for it to load and re-navigate the
-        // iframe back to the suspended domain.
-        ChromeTabUtils.waitForTabPageLoaded(mTab, mainframeUrl);
-        JavaScriptUtils.executeJavaScriptAndWaitForResult(
-                mTab.getWebContents(), code, 3, TimeUnit.SECONDS);
-
-        doReturn(true).when(mSuspensionTracker).isWebsiteSuspended(STARTING_FQDN);
-        suspendDomain(STARTING_FQDN);
-        waitForSuspendedTabToShow(mTab, STARTING_FQDN);
-
-        mActivityTestRule.loadUrl(mDifferentUrl);
-        assertSuspendedTabHidden(mTab);
-    }
-
     private void startLoadingUrl(Tab tab, String url) {
         TestThreadUtils.runOnUiThreadBlocking(
                 () -> { tab.loadUrl(new LoadUrlParams(url, PageTransition.TYPED)); });