Split HistoricalTabSaver out of WebContentsStateBridge and TabState

historical_tab_saver.cc offers a method that has separate concerns from
web_contents_state.cc and has different dependencies.

Bug: 1090048
Change-Id: Iee173293eb30337d8bf2eba4f2f4bd0d9a4a8a7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2265374
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784900}

chrome/android/BUILD.gn

@@ -3139,6 +3139,7 @@ generate_jni("chrome_jni_headers") {
     "java/src/org/chromium/chrome/browser/suggestions/mostvisited/MostVisitedSitesBridge.java",
     "java/src/org/chromium/chrome/browser/sync/ProfileSyncService.java",
     "java/src/org/chromium/chrome/browser/sync/TrustedVaultClient.java",
+    "java/src/org/chromium/chrome/browser/tab/HistoricalTabSaver.java",
     "java/src/org/chromium/chrome/browser/tab/TabBrowserControlsConstraintsHelper.java",
     "java/src/org/chromium/chrome/browser/tab/TabFavicon.java",
     "java/src/org/chromium/chrome/browser/tab/TabImpl.java",

chrome/android/chrome_java_sources.gni

@@ -1504,6 +1504,7 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/sync/ui/TrustedVaultKeyRetrievalProxyActivity.java",
   "java/src/org/chromium/chrome/browser/tab/AccessibilityVisibilityHandler.java",
   "java/src/org/chromium/chrome/browser/tab/AuthenticatorNavigationInterceptorTabHelper.java",
+  "java/src/org/chromium/chrome/browser/tab/HistoricalTabSaver.java",
   "java/src/org/chromium/chrome/browser/tab/InterceptNavigationDelegateClientImpl.java",
   "java/src/org/chromium/chrome/browser/tab/InterceptNavigationDelegateTabHelper.java",
   "java/src/org/chromium/chrome/browser/tab/RedirectHandlerTabHelper.java",

chrome/android/chrome_junit_test_java_sources.gni

@@ -216,6 +216,7 @@ chrome_junit_test_java_sources = [
   "junit/src/org/chromium/chrome/browser/suggestions/SuggestionsImageFetcherTest.java",
   "junit/src/org/chromium/chrome/browser/suggestions/tile/TileGroupUnitTest.java",
   "junit/src/org/chromium/chrome/browser/survey/ChromeSurveyControllerTest.java",
+  "junit/src/org/chromium/chrome/browser/tab/HistoricalTabSaverUnitTest.java",
   "junit/src/org/chromium/chrome/browser/tab/TabAttributesTest.java",
   "junit/src/org/chromium/chrome/browser/tab/TabBrowserControlsConstraintsHelperTest.java",
   "junit/src/org/chromium/chrome/browser/tab/TabBrowserControlsOffsetHelperTest.java",
@@ -225,7 +226,7 @@ chrome_junit_test_java_sources = [
   "junit/src/org/chromium/chrome/browser/tab/TabViewManagerUnitTest.java",
   "junit/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorImplTest.java",
   "junit/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorProfileSupplierTest.java",
-  "junit/src/org/chromium/chrome/browser/tabstate/TabStateUnitTest.java",
+  "junit/src/org/chromium/chrome/browser/tabstate/TabStateFileManagerUnitTest.java",
   "junit/src/org/chromium/chrome/browser/tasks/EngagementTimeUtilTest.java",
   "junit/src/org/chromium/chrome/browser/tasks/JourneyManagerTest.java",
   "junit/src/org/chromium/chrome/browser/toolbar/LocationBarFocusScrimHandlerTest.java",

chrome/android/java/src/org/chromium/chrome/browser/tab/HistoricalTabSaver.java

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.tab;
+
+import org.chromium.base.annotations.NativeMethods;
+import org.chromium.content_public.browser.WebContents;
+
+/**
+ * Saves historical tabs.
+ */
+public class HistoricalTabSaver {
+    /**
+     * Creates a historical tab from a tab being closed.
+     */
+    public static void createHistoricalTab(Tab tab) {
+        if (tab.isFrozen()) {
+            WebContentsState state = ((TabImpl) tab).getFrozenContentsState();
+            if (state != null) {
+                WebContents webContents =
+                        WebContentsStateBridge.restoreContentsFromByteBuffer(state, true);
+                if (webContents != null) {
+                    createHistoricalTabFromContents(webContents);
+                    webContents.destroy();
+                }
+            }
+        } else {
+            createHistoricalTabFromContents(tab.getWebContents());
+        }
+    }
+
+    private static void createHistoricalTabFromContents(WebContents webContents) {
+        HistoricalTabSaverJni.get().createHistoricalTabFromContents(webContents);
+    }
+
+    @NativeMethods
+    interface Natives {
+        void createHistoricalTabFromContents(WebContents webContents);
+    }
+}

chrome/android/java/src/org/chromium/chrome/browser/tab/TabState.java

@@ -8,7 +8,6 @@ import android.graphics.Color;
 
 import androidx.annotation.Nullable;
 
-import org.chromium.content_public.browser.WebContents;
 import org.chromium.ui.util.ColorUtils;
 
 /**
@@ -54,23 +53,4 @@ public class TabState {
     public boolean hasThemeColor() {
         return themeColor != UNSPECIFIED_THEME_COLOR && ColorUtils.isValidThemeColor(themeColor);
     }
-
-    /**
-     * Creates a historical tab from a tab being closed.
-     */
-    public static void createHistoricalTab(Tab tab) {
-        if (tab.isFrozen()) {
-            WebContentsState state = ((TabImpl) tab).getFrozenContentsState();
-            if (state != null) {
-                WebContents webContents =
-                        WebContentsStateBridge.restoreContentsFromByteBuffer(state, true);
-                if (webContents != null) {
-                    WebContentsStateBridge.createHistoricalTabFromContents(webContents);
-                    webContents.destroy();
-                }
-            }
-        } else {
-            WebContentsStateBridge.createHistoricalTabFromContents(tab.getWebContents());
-        }
-    }
 }

chrome/android/java/src/org/chromium/chrome/browser/tab/WebContentsStateBridge.java

@@ -81,10 +81,6 @@ public class WebContentsStateBridge {
                 contentsState.buffer(), contentsState.version());
     }
 
-    public static void createHistoricalTabFromContents(WebContents webContents) {
-        WebContentsStateBridgeJni.get().createHistoricalTabFromContents(webContents);
-    }
-
     @NativeMethods
     @VisibleForTesting
     public interface Natives {
@@ -96,6 +92,5 @@ public class WebContentsStateBridge {
                 int referrerPolicy, Origin initiatorOrigin, boolean isIncognito);
         String getDisplayTitleFromByteBuffer(ByteBuffer state, int savedStateVersion);
         String getVirtualUrlFromByteBuffer(ByteBuffer state, int savedStateVersion);
-        void createHistoricalTabFromContents(WebContents webContents);
     }
 }

chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelImpl.java

@@ -13,12 +13,12 @@ import org.chromium.chrome.browser.ChromeTabbedActivity;
 import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager;
 import org.chromium.chrome.browser.homepage.HomepageManager;
 import org.chromium.chrome.browser.ntp.RecentlyClosedBridge;
+import org.chromium.chrome.browser.tab.HistoricalTabSaver;
 import org.chromium.chrome.browser.tab.InterceptNavigationDelegateTabHelper;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TabCreationState;
 import org.chromium.chrome.browser.tab.TabLaunchType;
 import org.chromium.chrome.browser.tab.TabSelectionType;
-import org.chromium.chrome.browser.tab.TabState;
 import org.chromium.chrome.browser.tabmodel.NextTabPolicy.NextTabPolicySupplier;
 import org.chromium.chrome.browser.tabmodel.TabCreatorManager.TabCreator;
 import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities;
@@ -658,7 +658,7 @@ public class TabModelImpl extends TabModelJniBridge {
         if (mTabContentManager != null) mTabContentManager.removeTabThumbnail(tab.getId());
         mTabSaver.removeTabFromQueues(tab);
 
-        if (!isIncognito()) TabState.createHistoricalTab(tab);
+        if (!isIncognito()) HistoricalTabSaver.createHistoricalTab(tab);
 
         for (TabModelObserver obs : mObservers) obs.didCloseTab(tab.getId(), tab.isIncognito());
         if (notifyTabClosureCommitted) {

chrome/android/junit/src/org/chromium/chrome/browser/tab/HistoricalTabSaverUnitTest.java

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.tab;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.robolectric.annotation.Config;
+
+import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.base.test.util.JniMocker;
+import org.chromium.content_public.browser.WebContents;
+
+import java.nio.ByteBuffer;
+
+/**
+ * Unit tests for {@link HistoricalTabSaver}.
+ */
+@RunWith(BaseRobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
+public class HistoricalTabSaverUnitTest {
+    @Rule
+    public JniMocker mocker = new JniMocker();
+
+    @Mock
+    HistoricalTabSaver.Natives mHistoricalTabSaverJni;
+    @Mock
+    WebContentsStateBridge.Natives mWebContentsStateBridgeJni;
+
+    @Mock
+    public TabImpl mTabImplMock;
+    @Mock
+    public WebContents mWebContentsMock;
+
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+        mocker.mock(HistoricalTabSaverJni.TEST_HOOKS, mHistoricalTabSaverJni);
+        mocker.mock(WebContentsStateBridgeJni.TEST_HOOKS, mWebContentsStateBridgeJni);
+    }
+
+    @Test
+    public void testCreateHistoricalTab_NotFrozen_HistoricalTabCreated() {
+        doReturn(false).when(mTabImplMock).isFrozen();
+        doReturn(mWebContentsMock).when(mTabImplMock).getWebContents();
+
+        HistoricalTabSaver.createHistoricalTab(mTabImplMock);
+
+        verify(mHistoricalTabSaverJni).createHistoricalTabFromContents(eq(mWebContentsMock));
+    }
+
+    @Test
+    public void testCreateHistoricalTab_Frozen_NullWebContentsState_HistoricalTabNotCreated() {
+        doReturn(true).when(mTabImplMock).isFrozen();
+        doReturn(null).when(mTabImplMock).getFrozenContentsState();
+
+        HistoricalTabSaver.createHistoricalTab(mTabImplMock);
+
+        verify(mHistoricalTabSaverJni, never()).createHistoricalTabFromContents(any());
+    }
+
+    @Test
+    public void testCreateHistoricalTab_Frozen_RestoreFailed_HistoricalTabNotCreated() {
+        ByteBuffer buffer = ByteBuffer.allocate(1);
+        WebContentsState webContentsState = new WebContentsState(buffer);
+        webContentsState.setVersion(123);
+
+        doReturn(true).when(mTabImplMock).isFrozen();
+        doReturn(webContentsState).when(mTabImplMock).getFrozenContentsState();
+        doReturn(null)
+                .when(mWebContentsStateBridgeJni)
+                .restoreContentsFromByteBuffer(eq(buffer), eq(123), eq(true));
+
+        HistoricalTabSaver.createHistoricalTab(mTabImplMock);
+
+        verify(mHistoricalTabSaverJni, never()).createHistoricalTabFromContents(any());
+    }
+
+    @Test
+    public void testCreateHistoricalTab_Frozen_Restored_HistoricalTabCreated() {
+        ByteBuffer buffer = ByteBuffer.allocate(1);
+        WebContentsState webContentsState = new WebContentsState(buffer);
+        webContentsState.setVersion(123);
+
+        doReturn(true).when(mTabImplMock).isFrozen();
+        doReturn(webContentsState).when(mTabImplMock).getFrozenContentsState();
+        doReturn(mWebContentsMock)
+                .when(mWebContentsStateBridgeJni)
+                .restoreContentsFromByteBuffer(eq(buffer), eq(123), eq(true));
+
+        HistoricalTabSaver.createHistoricalTab(mTabImplMock);
+
+        verify(mHistoricalTabSaverJni).createHistoricalTabFromContents(eq(mWebContentsMock));
+    }
+}

chrome/android/junit/src/org/chromium/chrome/browser/tabstate/TabStateUnitTest.java → chrome/android/junit/src/org/chromium/chrome/browser/tabstate/TabStateFileManagerUnitTest.java

@@ -5,49 +5,35 @@
 package org.chromium.chrome.browser.tabstate;
 
 import static org.junit.Assert.assertEquals;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.verify;
 
 import androidx.annotation.Nullable;
 
-import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.MockitoAnnotations;
 import org.robolectric.annotation.Config;
 
 import org.chromium.base.StreamUtil;
 import org.chromium.base.test.BaseRobolectricTestRunner;
-import org.chromium.base.test.util.JniMocker;
-import org.chromium.chrome.browser.tab.TabImpl;
 import org.chromium.chrome.browser.tab.TabLaunchType;
 import org.chromium.chrome.browser.tab.TabState;
 import org.chromium.chrome.browser.tab.TabStateFileManager;
 import org.chromium.chrome.browser.tab.WebContentsState;
-import org.chromium.chrome.browser.tab.WebContentsStateBridge;
-import org.chromium.chrome.browser.tab.WebContentsStateBridgeJni;
-import org.chromium.content_public.browser.WebContents;
 
 import java.io.DataOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
-import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
 
 /**
- * Unit tests for TabState.
+ * Unit tests for {@link TabStateFileManager}.
  */
 @RunWith(BaseRobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
-public class TabStateUnitTest {
+public class TabStateFileManagerUnitTest {
     private static final byte[] CONTENTS_STATE_BYTES = new byte[] {1, 2, 3};
     private static final long TIMESTAMP = 10L;
     private static final int PARENT_ID = 1;
@@ -60,23 +46,6 @@ public class TabStateUnitTest {
     @Rule
     public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
-    @Rule
-    public JniMocker mocker = new JniMocker();
-
-    @Mock
-    WebContentsStateBridge.Natives mWebContentsStateBridgeJni;
-
-    @Mock
-    public TabImpl mTabImplMock;
-    @Mock
-    public WebContents mWebContentsMock;
-
-    @Before
-    public void setUp() {
-        MockitoAnnotations.initMocks(this);
-        mocker.mock(WebContentsStateBridgeJni.TEST_HOOKS, mWebContentsStateBridgeJni);
-    }
-
     @Test
     public void testSaveTabStateWithMemoryMappedContentsState() throws IOException {
         File file = createTestTabStateFile();
@@ -140,58 +109,4 @@ public class TabStateUnitTest {
         }
         return file;
     }
-
-    @Test
-    public void testSaveHistoricalTab_NotFrozen_HistoricalTabCreated() {
-        doReturn(false).when(mTabImplMock).isFrozen();
-        doReturn(mWebContentsMock).when(mTabImplMock).getWebContents();
-
-        TabState.createHistoricalTab(mTabImplMock);
-
-        verify(mWebContentsStateBridgeJni).createHistoricalTabFromContents(eq(mWebContentsMock));
-    }
-
-    @Test
-    public void testSaveHistoricalTab_Frozen_NullWebContentsState_HistoricalTabNotCreated() {
-        doReturn(true).when(mTabImplMock).isFrozen();
-        doReturn(null).when(mTabImplMock).getFrozenContentsState();
-
-        TabState.createHistoricalTab(mTabImplMock);
-
-        verify(mWebContentsStateBridgeJni, never()).createHistoricalTabFromContents(any());
-    }
-
-    @Test
-    public void testSaveHistoricalTab_Frozen_RestoreFailed_HistoricalTabNotCreated() {
-        ByteBuffer buffer = ByteBuffer.allocate(1);
-        WebContentsState webContentsState = new WebContentsState(buffer);
-        webContentsState.setVersion(123);
-
-        doReturn(true).when(mTabImplMock).isFrozen();
-        doReturn(webContentsState).when(mTabImplMock).getFrozenContentsState();
-        doReturn(null)
-                .when(mWebContentsStateBridgeJni)
-                .restoreContentsFromByteBuffer(eq(buffer), eq(123), eq(true));
-
-        TabState.createHistoricalTab(mTabImplMock);
-
-        verify(mWebContentsStateBridgeJni, never()).createHistoricalTabFromContents(any());
-    }
-
-    @Test
-    public void testSaveHistoricalTab_Frozen_Restored_HistoricalTabCreated() {
-        ByteBuffer buffer = ByteBuffer.allocate(1);
-        WebContentsState webContentsState = new WebContentsState(buffer);
-        webContentsState.setVersion(123);
-
-        doReturn(true).when(mTabImplMock).isFrozen();
-        doReturn(webContentsState).when(mTabImplMock).getFrozenContentsState();
-        doReturn(mWebContentsMock)
-                .when(mWebContentsStateBridgeJni)
-                .restoreContentsFromByteBuffer(eq(buffer), eq(123), eq(true));
-
-        TabState.createHistoricalTab(mTabImplMock);
-
-        verify(mWebContentsStateBridgeJni).createHistoricalTabFromContents(eq(mWebContentsMock));
-    }
 }

chrome/browser/BUILD.gn

@@ -2379,6 +2379,7 @@ static_library("browser") {
       "android/foreign_session_helper.cc",
       "android/foreign_session_helper.h",
       "android/headers_classifier.cc",
+      "android/historical_tab_saver.cc",
       "android/history/browsing_history_bridge.cc",
       "android/history/browsing_history_bridge.h",
       "android/history/history_deletion_bridge.cc",

chrome/browser/android/historical_tab_saver.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/android/jni_android.h"
+#include "base/logging.h"
+#include "chrome/android/chrome_jni_headers/HistoricalTabSaver_jni.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sessions/tab_restore_service_factory.h"
+#include "chrome/common/url_constants.h"
+#include "components/sessions/content/content_live_tab.h"
+#include "components/sessions/core/tab_restore_service.h"
+#include "content/public/browser/web_contents.h"
+
+using base::android::JavaParamRef;
+
+namespace {
+
+void CreateHistoricalTab(content::WebContents* web_contents) {
+  DCHECK(web_contents);
+
+  sessions::TabRestoreService* service =
+      TabRestoreServiceFactory::GetForProfile(
+          Profile::FromBrowserContext(web_contents->GetBrowserContext()));
+  if (!service)
+    return;
+
+  // Exclude internal pages from being marked as recent when they are closed.
+  const GURL& tab_url = web_contents->GetURL();
+  if (tab_url.SchemeIs(content::kChromeUIScheme) ||
+      tab_url.SchemeIs(chrome::kChromeNativeScheme) ||
+      tab_url.SchemeIs(url::kAboutScheme)) {
+    return;
+  }
+
+  // TODO(jcivelli): is the index important?
+  service->CreateHistoricalTab(
+      sessions::ContentLiveTab::GetForWebContents(web_contents), -1);
+}
+}  // anonymous namespace
+
+// Static JNI methods.
+
+// static
+static void JNI_HistoricalTabSaver_CreateHistoricalTabFromContents(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& jweb_contents) {
+  auto* web_contents = content::WebContents::FromJavaWebContents(jweb_contents);
+  if (web_contents)
+    CreateHistoricalTab(web_contents);
+}

chrome/browser/android/web_contents_state.cc

@@ -21,13 +21,9 @@
 #include "chrome/browser/android/tab_android.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
-#include "chrome/browser/sessions/tab_restore_service_factory.h"
-#include "chrome/common/url_constants.h"
-#include "components/sessions/content/content_live_tab.h"
 #include "components/sessions/content/content_serialized_navigation_builder.h"
 #include "components/sessions/core/serialized_navigation_entry.h"
 #include "components/sessions/core/session_command.h"
-#include "components/sessions/core/tab_restore_service.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/restore_type.h"
@@ -380,27 +376,6 @@ WebContents* RestoreContentsFromByteBuffer(void* data,
   return web_contents.release();
 }
 
-void CreateHistoricalTab(content::WebContents* web_contents) {
-  DCHECK(web_contents);
-
-  sessions::TabRestoreService* service =
-      TabRestoreServiceFactory::GetForProfile(
-          Profile::FromBrowserContext(web_contents->GetBrowserContext()));
-  if (!service)
-    return;
-
-  // Exclude internal pages from being marked as recent when they are closed.
-  const GURL& tab_url = web_contents->GetURL();
-  if (tab_url.SchemeIs(content::kChromeUIScheme) ||
-      tab_url.SchemeIs(chrome::kChromeNativeScheme) ||
-      tab_url.SchemeIs(url::kAboutScheme)) {
-    return;
-  }
-
-  // TODO(jcivelli): is the index important?
-  service->CreateHistoricalTab(
-      sessions::ContentLiveTab::GetForWebContents(web_contents), -1);
-}
 }  // anonymous namespace
 
 ScopedJavaLocalRef<jobject> WebContentsState::GetContentsStateAsByteBuffer(
@@ -625,12 +600,3 @@ JNI_WebContentsStateBridge_GetVirtualUrlFromByteBuffer(
                                                     saved_state_version);
   return result;
 }
-
-// static
-static void JNI_WebContentsStateBridge_CreateHistoricalTabFromContents(
-    JNIEnv* env,
-    const JavaParamRef<jobject>& jweb_contents) {
-  auto* web_contents = content::WebContents::FromJavaWebContents(jweb_contents);
-  if (web_contents)
-    CreateHistoricalTab(web_contents);
-}