Skip FirebaseAppIndex page visit reporting in Incognito tabs

In incognito tabs, stop reporting page visits to Firebase.

Also move the reporting to didFirstVisuallyNonEmptyPaint(), which is
closer to when the user perceives a page to be visited.

Also skip creating the observer altogether on low-end devices as an
optimization.

Bug: 1004003
Change-Id: I592521ab9ce5894df48f7994de30a255d9280f22
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1805297Reviewed-by: Yue Zhang <yuezhanggg@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697764}

chrome/android/java/src/org/chromium/chrome/browser/AppIndexingUtil.java

@@ -55,14 +55,16 @@ public class AppIndexingUtil {
     }
 
     AppIndexingUtil(@Nullable TabModelSelector mTabModelSelectorImpl) {
-        if (mTabModelSelectorImpl != null) {
+        if (mTabModelSelectorImpl != null && isEnabledForDevice()) {
             mObserver = new TabModelSelectorTabObserver(mTabModelSelectorImpl) {
                 @Override
                 public void onPageLoadFinished(final Tab tab, String url) {
                     extractCopylessPasteMetadata(tab);
-                    if (!SysUtils.isLowEndDevice()) {
-                        getAppIndexingReporter().reportWebPageView(url, tab.getTitle());
-                    }
+                }
+
+                @Override
+                public void didFirstVisuallyNonEmptyPaint(Tab tab) {
+                    reportPageView(tab);
                 }
             };
         }
@@ -79,17 +81,14 @@ public class AppIndexingUtil {
      */
     @VisibleForTesting
     void extractCopylessPasteMetadata(final Tab tab) {
-        final String url = tab.getUrl();
-        boolean isHttpOrHttps = UrlUtilities.isHttpOrHttps(url);
-        if (!isEnabledForDevice() || tab.isIncognito() || !isHttpOrHttps) {
-            return;
-        }
+        if (!isEnabledForTab(tab)) return;
 
         // There are three conditions that can occur with respect to the cache.
         // 1. Cache hit, and an entity was found previously.
         // 2. Cache hit, but no entity was found. Ignore.
         // 3. Cache miss, we need to parse the page.
         // Note that page view is reported unconditionally.
+        final String url = tab.getUrl();
         if (wasPageVisitedRecently(url)) {
             if (lastPageVisitContainedEntity(url)) {
                 // Condition 1
@@ -120,6 +119,12 @@ public class AppIndexingUtil {
         }
     }
 
+    @VisibleForTesting
+    void reportPageView(Tab tab) {
+        if (!isEnabledForTab(tab)) return;
+        getAppIndexingReporter().reportWebPageView(tab.getUrl(), tab.getTitle());
+    }
+
     @VisibleForTesting
     static void setCallbackForTesting(Callback<WebPage> callback) {
         sCallbackForTesting = callback;
@@ -178,10 +183,18 @@ public class AppIndexingUtil {
         return SystemClock.elapsedRealtime();
     }
 
+    @VisibleForTesting
     boolean isEnabledForDevice() {
         return !SysUtils.isLowEndDevice();
     }
 
+    @VisibleForTesting
+    boolean isEnabledForTab(Tab tab) {
+        final String url = tab.getUrl();
+        boolean isHttpOrHttps = UrlUtilities.isHttpOrHttps(url);
+        return isEnabledForDevice() && !tab.isIncognito() && isHttpOrHttps;
+    }
+
     private LruCache<String, CacheEntry> getPageCache() {
         if (mPageCache == null) {
             mPageCache = new LruCache<String, CacheEntry>(CACHE_SIZE);

chrome/android/junit/src/org/chromium/chrome/browser/AppIndexingUtilTest.java

@@ -5,6 +5,7 @@
 package org.chromium.chrome.browser;
 
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.never;
@@ -69,19 +70,27 @@ public class AppIndexingUtilTest {
     }
 
     @Test
-    public void testNoCacheHit() {
+    public void testExtractCopylessPasteMetadata_Incognito() {
+        doReturn(true).when(mTab).isIncognito();
+
+        mUtil.extractCopylessPasteMetadata(mTab);
+        verify(mCopylessPaste, never()).getEntities(any());
+        verify(mReporter, never()).reportWebPage(any());
+    }
+
+    @Test
+    public void testExtractCopylessPasteMetadata_NoCacheHit() {
         mUtil.extractCopylessPasteMetadata(mTab);
         verify(mCopylessPaste).getEntities(any(CopylessPaste.GetEntitiesResponse.class));
         verify(mReporter).reportWebPage(any(WebPage.class));
     }
 
     @Test
-    public void testCacheHit() {
+    public void testExtractCopylessPasteMetadata_CacheHit() {
         mUtil.extractCopylessPasteMetadata(mTab);
         verify(mCopylessPaste).getEntities(any(CopylessPaste.GetEntitiesResponse.class));
         verify(mCopylessPaste).close();
         verify(mReporter).reportWebPage(any(WebPage.class));
-        verify(mReporter, never()).reportWebPageView(any(String.class), any(String.class));
 
         doReturn(1L).when(mUtil).getElapsedTime();
         mUtil.extractCopylessPasteMetadata(mTab);
@@ -90,7 +99,7 @@ public class AppIndexingUtilTest {
     }
 
     @Test
-    public void testCacheHit_expired() {
+    public void testExtractCopylessPasteMetadata_CacheHit_Expired() {
         mUtil.extractCopylessPasteMetadata(mTab);
 
         doReturn(60 * 60 * 1000L + 1).when(mUtil).getElapsedTime();
@@ -99,7 +108,7 @@ public class AppIndexingUtilTest {
     }
 
     @Test
-    public void testCacheHit_noEntity() {
+    public void testExtractCopylessPasteMetadata_CacheHit_NoEntity() {
         doAnswer(invocation -> {
             CopylessPaste.GetEntitiesResponse callback =
                     (CopylessPaste.GetEntitiesResponse) invocation.getArguments()[0];
@@ -114,6 +123,20 @@ public class AppIndexingUtilTest {
         verifyNoMoreInteractions(mReporter);
     }
 
+    @Test
+    public void testReportPageView_Incognito() {
+        doReturn(true).when(mTab).isIncognito();
+
+        mUtil.reportPageView(mTab);
+        verify(mReporter, never()).reportWebPageView(any(), any());
+    }
+
+    @Test
+    public void testReportPageView() {
+        mUtil.reportPageView(mTab);
+        verify(mReporter).reportWebPageView(eq("http://www.test.com"), eq("My neat website"));
+    }
+
     private Url createUrl(String s) {
         Url url = new Url();
         url.url = s;