Feed: Leave it up to Offline Pages to decide how to open Feed articles.

Previously, Feed articles would always open the offline version of a
page if it exists, which isn't ideal for users who are online.

This CL changes this behavior to open the article normally, which
leaves it up to Offline Pages to decide whether or not to open the
offline version of the page (e.g. when offline).

TBR=twellington@chromium.org,wychen@chromium.org

Bug: 1069906
Change-Id: I5723e418747867411511ede72f43c323b493b154
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2151175Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Commit-Queue: Scott Little <sclittle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#768972}

chrome/android/features/start_surface/internal/java/src/org/chromium/chrome/features/start_surface/ExploreSurfaceActionHandler.java

@@ -9,23 +9,18 @@ import android.app.Activity;
 import androidx.annotation.NonNull;
 
 import org.chromium.chrome.browser.feed.FeedLoggingBridge;
-import org.chromium.chrome.browser.feed.FeedOfflineIndicator;
 import org.chromium.chrome.browser.feed.action.FeedActionHandler;
 import org.chromium.chrome.browser.feed.library.api.client.knowncontent.ContentMetadata;
 import org.chromium.chrome.browser.native_page.NativePageNavigationDelegate;
-import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.profiles.Profile;
 
 /** Implementation of the {@link ActionApi} for the explore surface. */
 class ExploreSurfaceActionHandler extends FeedActionHandler {
     ExploreSurfaceActionHandler(@NonNull NativePageNavigationDelegate delegate,
             @NonNull Runnable suggestionConsumedObserver,
-            @NonNull FeedOfflineIndicator offlineIndicator,
-            @NonNull OfflinePageBridge offlinePageBridge,
             @NonNull FeedLoggingBridge loggingBridge,
             Activity activity, Profile profile) {
-        super(delegate, suggestionConsumedObserver, offlineIndicator, offlinePageBridge,
-                loggingBridge, activity, profile);
+        super(delegate, suggestionConsumedObserver, loggingBridge, activity, profile);
     }
 
     // TODO(crbug.com/982018): Support download and lean more actions.

chrome/android/features/start_surface/internal/java/src/org/chromium/chrome/features/start_surface/ExploreSurfaceCoordinator.java

@@ -17,7 +17,6 @@ import org.chromium.chrome.browser.feed.library.api.client.stream.Stream;
 import org.chromium.chrome.browser.feed.shared.FeedSurfaceDelegate;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.ntp.snippets.SectionHeaderView;
-import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.start_surface.R;
 import org.chromium.ui.modelutil.PropertyModel;
@@ -87,8 +86,6 @@ class ExploreSurfaceCoordinator implements FeedSurfaceDelegate {
         ExploreSurfaceActionHandler exploreSurfaceActionHandler =
                 new ExploreSurfaceActionHandler(mExploreSurfaceNavigationDelegate,
                         FeedProcessScopeFactory.getFeedConsumptionObserver(),
-                        FeedProcessScopeFactory.getFeedOfflineIndicator(),
-                        OfflinePageBridge.getForProfile(profile),
                         FeedProcessScopeFactory.getFeedLoggingBridge(), mActivity, profile);
 
         SectionHeaderView sectionHeaderView = null;

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedLoggingBridge.java

@@ -335,16 +335,15 @@ public class FeedLoggingBridge implements BasicLoggingApi {
      * Reports how long a user spends on the page.
      *
      * @param visitTimeMs Time spent reading the page.
-     * @param isOffline If the page is viewed in offline mode or not.
      * @param returnToNtp User backed to NTP after visit the page.
      */
-    public void onContentTargetVisited(long visitTimeMs, boolean isOffline, boolean returnToNtp) {
+    public void onContentTargetVisited(long visitTimeMs, boolean returnToNtp) {
         // We cannot assume that the|mNativeFeedLoggingBridge| is always available like other
         // methods. This method is called by objects not controlled by Feed lifetimes, and destroy()
         // may have already been called if Feed is disabled by policy.
         if (mNativeFeedLoggingBridge != 0) {
-            FeedLoggingBridgeJni.get().onContentTargetVisited(mNativeFeedLoggingBridge,
-                    FeedLoggingBridge.this, visitTimeMs, isOffline, returnToNtp);
+            FeedLoggingBridgeJni.get().onContentTargetVisited(
+                    mNativeFeedLoggingBridge, FeedLoggingBridge.this, visitTimeMs, returnToNtp);
         }
     }
 
@@ -546,7 +545,7 @@ public class FeedLoggingBridge implements BasicLoggingApi {
         void onTaskFinished(long nativeFeedLoggingBridge, FeedLoggingBridge caller, int task,
                 int delayTimeMs, int taskTimeMs);
         void onContentTargetVisited(long nativeFeedLoggingBridge, FeedLoggingBridge caller,
-                long visitTimeMs, boolean isOffline, boolean returnToNtp);
+                long visitTimeMs, boolean returnToNtp);
         void reportScrolledAfterOpen(long nativeFeedLoggingBridge, FeedLoggingBridge caller);
     }
 }

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/action/FeedActionHandler.java

@@ -11,19 +11,16 @@ import androidx.annotation.NonNull;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.browser.feed.FeedLoggingBridge;
-import org.chromium.chrome.browser.feed.FeedOfflineIndicator;
 import org.chromium.chrome.browser.feed.library.api.client.knowncontent.ContentMetadata;
 import org.chromium.chrome.browser.feed.library.api.host.action.ActionApi;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.help.HelpAndFeedback;
 import org.chromium.chrome.browser.native_page.NativePageNavigationDelegate;
 import org.chromium.chrome.browser.ntp.NewTabPage;
-import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.suggestions.NavigationRecorder;
 import org.chromium.chrome.browser.suggestions.SuggestionsConfig;
 import org.chromium.chrome.browser.tab.Tab;
-import org.chromium.components.offline_items_collection.LaunchLocation;
 import org.chromium.content_public.browser.LoadUrlParams;
 import org.chromium.content_public.common.Referrer;
 import org.chromium.network.mojom.ReferrerPolicy;
@@ -38,8 +35,6 @@ import java.util.HashMap;
 public class FeedActionHandler implements ActionApi {
     private final NativePageNavigationDelegate mDelegate;
     private final Runnable mSuggestionConsumedObserver;
-    private final FeedOfflineIndicator mOfflineIndicator;
-    private final OfflinePageBridge mOfflinePageBridge;
     private final FeedLoggingBridge mLoggingBridge;
     private final Activity mActivity;
     private final Profile mProfile;
@@ -64,21 +59,15 @@ public class FeedActionHandler implements ActionApi {
      * handling some of the actions.
      * @param suggestionConsumedObserver An observer that is interested in any time a suggestion is
      * consumed by the user.
-     * @param offlineIndicator Tracks offline pages and can supply this handler with offline ids.
-     * @param offlinePageBridge Capable of updating {@link LoadUrlParams} to include offline ids.
      * @param loggingBridge Reports pages visiting time.
      */
     public FeedActionHandler(@NonNull NativePageNavigationDelegate delegate,
             @NonNull Runnable suggestionConsumedObserver,
-            @NonNull FeedOfflineIndicator offlineIndicator,
-            @NonNull OfflinePageBridge offlinePageBridge,
             @NonNull FeedLoggingBridge loggingBridge,
             Activity activity,
             Profile profile) {
         mDelegate = delegate;
         mSuggestionConsumedObserver = suggestionConsumedObserver;
-        mOfflineIndicator = offlineIndicator;
-        mOfflinePageBridge = offlinePageBridge;
         mLoggingBridge = loggingBridge;
         mActivity = activity;
         mProfile = profile;
@@ -86,7 +75,7 @@ public class FeedActionHandler implements ActionApi {
 
     @Override
     public void openUrl(String url) {
-        openOfflineIfPossible(WindowOpenDisposition.CURRENT_TAB, url);
+        openAndRecord(WindowOpenDisposition.CURRENT_TAB, createLoadUrlParams(url));
         mLoggingBridge.reportFeedInteraction();
     }
 
@@ -109,7 +98,7 @@ public class FeedActionHandler implements ActionApi {
 
     @Override
     public void openUrlInNewTab(String url) {
-        openOfflineIfPossible(WindowOpenDisposition.NEW_BACKGROUND_TAB, url);
+        openAndRecord(WindowOpenDisposition.NEW_BACKGROUND_TAB, createLoadUrlParams(url));
         mLoggingBridge.reportFeedInteraction();
     }
 
@@ -120,7 +109,7 @@ public class FeedActionHandler implements ActionApi {
 
     @Override
     public void openUrlInNewWindow(String url) {
-        openOfflineIfPossible(WindowOpenDisposition.NEW_WINDOW, url);
+        openAndRecord(WindowOpenDisposition.NEW_WINDOW, createLoadUrlParams(url));
     }
 
     @Override
@@ -194,44 +183,14 @@ public class FeedActionHandler implements ActionApi {
         return params;
     }
 
-    /**
-     * Opens the given url in offline mode if possible by setting load params so the offline
-     * interceptor will handle the request. If there is no offline id, fall back to just opening the
-     * |url| through the |mDelegate|.
-     *
-     * @param disposition How to open the article. Should not be OFF_THE_RECORD, as offline pages
-     * does not support opening articles while incognito.
-     * @param url The url of the article. Should match what was previously requested by Feed to
-     * OfflineIndicatorApi implementation exactly.
-     */
-    private void openOfflineIfPossible(int disposition, String url) {
-        Long maybeOfflineId = mOfflineIndicator.getOfflineIdIfPageIsOfflined(url);
-        if (maybeOfflineId == null) {
-            openAndRecord(disposition, createLoadUrlParams(url), /*isOffline*/ false);
-        } else {
-            mOfflinePageBridge.getLoadUrlParamsByOfflineId(
-                    maybeOfflineId, LaunchLocation.SUGGESTION, (loadUrlParams) -> {
-                        if (loadUrlParams == null) {
-                            // Fall back to opening online if the lookup failed.
-                            openAndRecord(
-                                    disposition, createLoadUrlParams(url), /*isOffline*/ false);
-                        } else {
-                            // Offline headers need to be moved to be read correctly.
-                            loadUrlParams.setVerbatimHeaders(loadUrlParams.getExtraHeadersString());
-                            openAndRecord(disposition, loadUrlParams, /*isOffline*/ true);
-                        }
-                    });
-        }
-    }
     /**
      * Opens the given resource, specified by params, and records how much time the user spends on
      * the suggested page.
      *
      * @param disposition How to open the article.
      * @param loadUrlParams Parameters specifying the URL to load and other navigation details.
-     * @param isOffline If the page should open in offline mode or not, for metrics reporting.
      */
-    private void openAndRecord(int disposition, LoadUrlParams loadUrlParams, boolean isOffline) {
+    private void openAndRecord(int disposition, LoadUrlParams loadUrlParams) {
         Tab loadingTab = mDelegate.openUrl(disposition, loadUrlParams);
         if (loadingTab != null) {
             // Records how long the user spending on the suggested page, and whether the user got
@@ -239,7 +198,7 @@ public class FeedActionHandler implements ActionApi {
             NavigationRecorder.record(loadingTab,
                     visitData
                     -> mLoggingBridge.onContentTargetVisited(
-                            visitData.duration, isOffline, NewTabPage.isNTPUrl(visitData.endUrl)));
+                            visitData.duration, NewTabPage.isNTPUrl(visitData.endUrl)));
         }
         mSuggestionConsumedObserver.run();
     }

chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

@@ -49,7 +49,6 @@ import org.chromium.chrome.browser.lifecycle.LifecycleObserver;
 import org.chromium.chrome.browser.lifecycle.PauseResumeWithNativeObserver;
 import org.chromium.chrome.browser.native_page.ContextMenuManager;
 import org.chromium.chrome.browser.ntp.snippets.SectionHeaderView;
-import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.omnibox.LocationBar.OmniboxFocusReason;
 import org.chromium.chrome.browser.omnibox.voice.VoiceRecognitionHandler;
 import org.chromium.chrome.browser.profiles.Profile;
@@ -424,8 +423,6 @@ public class NewTabPage implements NativePage, InvalidationAwareThumbnailProvide
         Profile profile = Profile.fromWebContents(mTab.getWebContents());
         ActionApi actionApi = new FeedActionHandler(mNewTabPageManager.getNavigationDelegate(),
                 FeedProcessScopeFactory.getFeedConsumptionObserver(),
-                FeedProcessScopeFactory.getFeedOfflineIndicator(),
-                OfflinePageBridge.getForProfile(profile),
                 FeedProcessScopeFactory.getFeedLoggingBridge(), activity, profile);
         LayoutInflater inflater = LayoutInflater.from(activity);
         mNewTabPageLayout = (NewTabPageLayout) inflater.inflate(R.layout.new_tab_page_layout, null);

chrome/android/junit/src/org/chromium/chrome/browser/feed/action/FeedActionHandlerTest.java

@@ -5,11 +5,9 @@
 package org.chromium.chrome.browser.feed.action;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyLong;
-import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.reset;
@@ -34,10 +32,7 @@ import org.robolectric.annotation.Config;
 import org.chromium.base.Callback;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.chrome.browser.feed.FeedLoggingBridge;
-import org.chromium.chrome.browser.feed.FeedOfflineIndicator;
-import org.chromium.chrome.browser.feed.library.api.client.knowncontent.ContentMetadata;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
-import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.suggestions.SuggestionsNavigationDelegate;
 import org.chromium.chrome.browser.tab.TabImpl;
 import org.chromium.chrome.test.util.browser.Features;
@@ -47,8 +42,6 @@ import org.chromium.content_public.browser.WebContents;
 import org.chromium.content_public.browser.WebContentsObserver;
 import org.chromium.ui.mojom.WindowOpenDisposition;
 
-import java.util.Collections;
-
 /**
  * Unit tests for {@link FeedActionHandler}.
  */
@@ -57,7 +50,6 @@ import java.util.Collections;
 @Features.EnableFeatures(ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS)
 public class FeedActionHandlerTest {
     private static final String TEST_URL = "http://www.one.com/";
-    private static final Long OFFLINE_ID = 12345L;
 
     @Rule
     public TestRule mFeaturesProcessorRule = new Features.JUnitProcessor();
@@ -67,10 +59,6 @@ public class FeedActionHandlerTest {
     @Mock
     private Runnable mSuggestionConsumedObserver;
     @Mock
-    private FeedOfflineIndicator mOfflineIndicator;
-    @Mock
-    private OfflinePageBridge mOfflinePageBridge;
-    @Mock
     private FeedLoggingBridge mLoggingBridge;
     @Mock
     private TabImpl mTab;
@@ -84,8 +72,6 @@ public class FeedActionHandlerTest {
     @Captor
     private ArgumentCaptor<LoadUrlParams> mLoadParamsCapture;
     @Captor
-    private ArgumentCaptor<Long> mOfflineIdCapture;
-    @Captor
     private ArgumentCaptor<Callback<LoadUrlParams>> mLoadUrlParamsCallbackCaptor;
     @Captor
     ArgumentCaptor<WebContentsObserver> mWebContentsObserverCaptor;
@@ -93,16 +79,6 @@ public class FeedActionHandlerTest {
     int mLastCommittedEntryIndex;
     private FeedActionHandler mActionHandler;
 
-    private void verifyOpenedOffline(int expectedDisposition) {
-        assertEquals(OFFLINE_ID, mOfflineIdCapture.getValue());
-        verify(mDelegate, times(1))
-                .openUrl(mDispositionCapture.capture(), mLoadParamsCapture.capture());
-        assertEquals(expectedDisposition, (int) mDispositionCapture.getValue());
-        assertTrue(
-                mLoadParamsCapture.getValue().getVerbatimHeaders().contains(OFFLINE_ID.toString()));
-        verify(mSuggestionConsumedObserver, times(1)).run();
-    }
-
     private void verifyOpenedOnline(int expectedDisposition) {
         verify(mDelegate, times(1))
                 .openUrl(mDispositionCapture.capture(), mLoadParamsCapture.capture());
@@ -114,8 +90,8 @@ public class FeedActionHandlerTest {
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
-        mActionHandler = new FeedActionHandler(mDelegate, mSuggestionConsumedObserver,
-                mOfflineIndicator, mOfflinePageBridge, mLoggingBridge, null, null);
+        mActionHandler = new FeedActionHandler(
+                mDelegate, mSuggestionConsumedObserver, mLoggingBridge, null, null);
 
         // Setup mocks such that when NavigationRecorder#record is called, it immediately invokes
         // the passed callback.
@@ -132,61 +108,17 @@ public class FeedActionHandlerTest {
                 .addObserver(mWebContentsObserverCaptor.capture());
     }
 
-    private void answerWithGoodParams() {
-        LoadUrlParams params = new LoadUrlParams("");
-        params.setExtraHeaders(Collections.singletonMap("", OFFLINE_ID.toString()));
-        answerWithGivenParams(params);
-    }
-
-    // Configures mOfflinePageBridge to run the passed callback when getLoadUrlParamsByOfflineId is
-    // called. If this isn't setup, the callback will never be invoked.
-    private void answerWithGivenParams(LoadUrlParams params) {
-        doAnswer(invocation -> {
-            mLoadUrlParamsCallbackCaptor.getValue().onResult(params);
-            return null;
-        })
-                .when(mOfflinePageBridge)
-                .getLoadUrlParamsByOfflineId(mOfflineIdCapture.capture(), anyInt(),
-                        mLoadUrlParamsCallbackCaptor.capture());
-    }
-
-    @Test
-    @SmallTest
-    public void testOpenUrlOffline() {
-        when(mOfflineIndicator.getOfflineIdIfPageIsOfflined(TEST_URL)).thenReturn(OFFLINE_ID);
-        answerWithGoodParams();
-        mActionHandler.openUrl(TEST_URL);
-        verifyOpenedOffline(WindowOpenDisposition.CURRENT_TAB);
-        verify(mLoggingBridge, times(1))
-                .onContentTargetVisited(anyLong(), /*isOffline*/ eq(true), anyBoolean());
-    }
-
     @Test
     @SmallTest
     public void testOpenUrlOnline() {
-        when(mOfflineIndicator.getOfflineIdIfPageIsOfflined(TEST_URL)).thenReturn(null);
         mActionHandler.openUrl(TEST_URL);
         verifyOpenedOnline(WindowOpenDisposition.CURRENT_TAB);
-        verify(mLoggingBridge, times(1))
-                .onContentTargetVisited(anyLong(), /*isOffline*/ eq(false), anyBoolean());
+        verify(mLoggingBridge, times(1)).onContentTargetVisited(anyLong(), anyBoolean());
     }
 
     @Test
     @SmallTest
-    public void testOpenUrlOfflineWithNullParams() {
-        // The indicator will give an offline id, but the model returns a null LoadUrlParams.
-        when(mOfflineIndicator.getOfflineIdIfPageIsOfflined(TEST_URL)).thenReturn(OFFLINE_ID);
-        answerWithGivenParams(null);
-        mActionHandler.openUrl(TEST_URL);
-        verifyOpenedOnline(WindowOpenDisposition.CURRENT_TAB);
-        verify(mLoggingBridge, times(1))
-                .onContentTargetVisited(anyLong(), /*isOffline*/ eq(false), anyBoolean());
-    }
-
-    @Test
-    @SmallTest
-    public void testOpenUrlInIncognitoModeWithOffline() {
-        when(mOfflineIndicator.getOfflineIdIfPageIsOfflined(TEST_URL)).thenReturn(OFFLINE_ID);
+    public void testOpenUrlInIncognitoMode() {
         mActionHandler.openUrlInIncognitoMode(TEST_URL);
 
         // Even though this page has an offlined version, it should not be used because offline
@@ -194,58 +126,20 @@ public class FeedActionHandlerTest {
         verifyOpenedOnline(WindowOpenDisposition.OFF_THE_RECORD);
     }
 
-    @Test
-    @SmallTest
-    public void testOpenUrlInNewTabOffline() {
-        when(mOfflineIndicator.getOfflineIdIfPageIsOfflined(TEST_URL)).thenReturn(OFFLINE_ID);
-        answerWithGoodParams();
-        mActionHandler.openUrlInNewTab(TEST_URL);
-        verifyOpenedOffline(WindowOpenDisposition.NEW_BACKGROUND_TAB);
-        verify(mLoggingBridge, times(1))
-                .onContentTargetVisited(anyLong(), /*isOffline*/ eq(true), anyBoolean());
-    }
-
     @Test
     @SmallTest
     public void testOpenUrlInNewTabOnline() {
-        when(mOfflineIndicator.getOfflineIdIfPageIsOfflined(TEST_URL)).thenReturn(null);
         mActionHandler.openUrlInNewTab(TEST_URL);
         verifyOpenedOnline(WindowOpenDisposition.NEW_BACKGROUND_TAB);
-        verify(mLoggingBridge, times(1))
-                .onContentTargetVisited(anyLong(), /*isOffline*/ eq(false), anyBoolean());
-    }
-
-    @Test
-    @SmallTest
-    public void testOpenUrlInNewWindowOffline() {
-        when(mOfflineIndicator.getOfflineIdIfPageIsOfflined(TEST_URL)).thenReturn(OFFLINE_ID);
-        answerWithGoodParams();
-        mActionHandler.openUrlInNewWindow(TEST_URL);
-        verifyOpenedOffline(WindowOpenDisposition.NEW_WINDOW);
-        verify(mLoggingBridge, times(1))
-                .onContentTargetVisited(anyLong(), /*isOffline*/ eq(true), anyBoolean());
+        verify(mLoggingBridge, times(1)).onContentTargetVisited(anyLong(), anyBoolean());
     }
 
     @Test
     @SmallTest
     public void testOpenUrlInNewWindowOnline() {
-        when(mOfflineIndicator.getOfflineIdIfPageIsOfflined(TEST_URL)).thenReturn(null);
         mActionHandler.openUrlInNewWindow(TEST_URL);
         verifyOpenedOnline(WindowOpenDisposition.NEW_WINDOW);
-        verify(mLoggingBridge, times(1))
-                .onContentTargetVisited(anyLong(), /*isOffline*/ eq(false), anyBoolean());
-    }
-
-    @Test
-    @SmallTest
-    public void testDownloadUrlWithOffline() {
-        when(mOfflineIndicator.getOfflineIdIfPageIsOfflined(TEST_URL)).thenReturn(OFFLINE_ID);
-        ContentMetadata metadata = new ContentMetadata(TEST_URL, "", 0, null, null, null, null);
-        mActionHandler.downloadUrl(metadata);
-
-        // Even though this page has an offlined version, this is not a request to open the page,
-        // and as such the load params should not be updated with the offline id.
-        verifyOpenedOnline(WindowOpenDisposition.SAVE_TO_DISK);
+        verify(mLoggingBridge, times(1)).onContentTargetVisited(anyLong(), anyBoolean());
     }
 
     @Test
@@ -255,10 +149,8 @@ public class FeedActionHandlerTest {
         reset(mDelegate);
         when(mDelegate.openUrl(anyInt(), any(LoadUrlParams.class))).thenReturn(null);
 
-        when(mOfflineIndicator.getOfflineIdIfPageIsOfflined(TEST_URL)).thenReturn(null);
         mActionHandler.openUrl(TEST_URL);
         verifyOpenedOnline(WindowOpenDisposition.CURRENT_TAB);
-        verify(mLoggingBridge, times(0))
-                .onContentTargetVisited(anyLong(), anyBoolean(), anyBoolean());
+        verify(mLoggingBridge, times(0)).onContentTargetVisited(anyLong(), anyBoolean());
     }
 }

chrome/browser/android/feed/feed_logging_bridge.cc

@@ -259,15 +259,9 @@ void FeedLoggingBridge::OnTaskFinished(
 void FeedLoggingBridge::OnContentTargetVisited(JNIEnv* j_env,
                                                const JavaRef<jobject>& j_this,
                                                const jlong visit_time_ms,
-                                               const jboolean j_is_offline,
                                                const jboolean j_return_to_ntp) {
-  if (j_is_offline) {
-    feed_logging_metrics_->OnSuggestionOfflinePageVisited(
-        base::TimeDelta::FromMilliseconds(visit_time_ms), j_return_to_ntp);
-  } else {
-    feed_logging_metrics_->OnSuggestionArticleVisited(
-        base::TimeDelta::FromMilliseconds(visit_time_ms), j_return_to_ntp);
-  }
+  feed_logging_metrics_->OnSuggestionArticleVisited(
+      base::TimeDelta::FromMilliseconds(visit_time_ms), j_return_to_ntp);
 }
 
 void FeedLoggingBridge::ReportScrolledAfterOpen(

chrome/browser/android/feed/feed_logging_bridge.h

@@ -154,7 +154,6 @@ class FeedLoggingBridge {
   void OnContentTargetVisited(JNIEnv* j_env,
                               const base::android::JavaRef<jobject>& j_this,
                               const jlong visit_time_ms,
-                              const jboolean j_is_offline,
                               const jboolean j_return_to_ntp);
 
   void ReportScrolledAfterOpen(JNIEnv* j_env,

components/feed/core/feed_logging_metrics.cc

@@ -603,14 +603,6 @@ void FeedLoggingMetrics::OnSuggestionArticleVisited(base::TimeDelta visit_time,
   RecordSuggestionPageVisited(return_to_ntp);
 }
 
-void FeedLoggingMetrics::OnSuggestionOfflinePageVisited(
-    base::TimeDelta visit_time,
-    bool return_to_ntp) {
-  base::UmaHistogramLongTimes(
-      "NewTabPage.ContentSuggestions.VisitDuration.Downloads", visit_time);
-  RecordSuggestionPageVisited(return_to_ntp);
-}
-
 void FeedLoggingMetrics::OnMoreButtonShown(int position) {
   // The "more" card can appear in addition to the actual suggestions, so add
   // one extra bucket to this histogram.

components/feed/core/feed_logging_metrics.h

@@ -74,9 +74,6 @@ class FeedLoggingMetrics {
   void OnSuggestionArticleVisited(base::TimeDelta visit_time,
                                   bool return_to_ntp);
 
-  void OnSuggestionOfflinePageVisited(base::TimeDelta visit_time,
-                                      bool return_to_ntp);
-
   // Should only be called once per NTP for each "more" button.
   void OnMoreButtonShown(int position);