Android: Tab.createHistoricalTab to TabState

Moves |createHistoricalTab| to TabState as a static helper method.
TabState already provides a method creating a historical Tab from
WebContentsState, so could be a good fit for hosting another
creating Tab from WebContents.

Bug: 995903
Change-Id: Ied744eb3292a9b0793f0baf93abdaf84ac8aac0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1846613Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704428}

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

@@ -1726,19 +1726,6 @@ public class Tab {
         return mTimestampMillis;
     }
 
-    /**
-     * Restores a tab either frozen or from state.
-     * TODO(aurimas): investigate reducing the visibility of this method after TabModel refactoring.
-     */
-    public void createHistoricalTab() {
-        if (!isFrozen()) {
-            assert mNativeTabAndroid != 0;
-            TabJni.get().createHistoricalTab(mNativeTabAndroid, Tab.this);
-        } else if (mFrozenContentsState != null) {
-            mFrozenContentsState.createHistoricalTab();
-        }
-    }
-
     /**
      * Delete navigation entries from frozen state matching the predicate.
      * @param predicate Handle for a deletion predicate interpreted by native code.
@@ -1897,7 +1884,6 @@ public class Tab {
                 long intentReceivedTimestamp);
         void setActiveNavigationEntryTitleForUrl(
                 long nativeTabAndroid, Tab caller, String url, String title);
-        void createHistoricalTab(long nativeTabAndroid, Tab caller);
         void loadOriginalImage(long nativeTabAndroid, Tab caller);
         void attachDetachedTab(long nativeTabAndroid, Tab caller);
     }

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

@@ -131,14 +131,6 @@ public class TabState {
             newState.setVersion(TabState.CONTENTS_STATE_CURRENT_VERSION);
             return newState;
         }
-
-        /**
-         * Creates a WebContents for the ContentsState and adds it as an historical tab, then
-         * deletes the WebContents.
-         */
-        public void createHistoricalTab() {
-            TabStateJni.get().createHistoricalTab(mBuffer, mVersion);
-        }
     }
 
     /** Navigation history of the WebContents. */
@@ -605,6 +597,20 @@ public class TabState {
         sChannelNameOverrideForTest = name;
     }
 
+    /**
+     * Creates a historical tab from a tab being closed.
+     */
+    public static void createHistoricalTab(Tab tab) {
+        if (!tab.isFrozen()) {
+            TabStateJni.get().createHistoricalTabFromContents(tab.getWebContents());
+        } else {
+            WebContentsState state = tab.getFrozenContentsState();
+            if (state != null) {
+                TabStateJni.get().createHistoricalTab(state.buffer(), state.version());
+            }
+        }
+    }
+
     @NativeMethods
     interface Natives {
         WebContents restoreContentsFromByteBuffer(
@@ -616,5 +622,6 @@ public class TabState {
         String getDisplayTitleFromByteBuffer(ByteBuffer state, int savedStateVersion);
         String getVirtualUrlFromByteBuffer(ByteBuffer state, int savedStateVersion);
         void createHistoricalTab(ByteBuffer state, int savedStateVersion);
+        void createHistoricalTabFromContents(WebContents webContents);
     }
 }

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

@@ -12,6 +12,7 @@ import org.chromium.chrome.browser.ntp.RecentlyClosedBridge;
 import org.chromium.chrome.browser.partnercustomizations.HomepageManager;
 import org.chromium.chrome.browser.tab.InterceptNavigationDelegateImpl;
 import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.chrome.browser.tab.TabState;
 import org.chromium.chrome.browser.tabmodel.TabCreatorManager.TabCreator;
 import org.chromium.chrome.browser.util.FeatureUtilities;
 import org.chromium.chrome.browser.util.MathUtils;
@@ -461,8 +462,9 @@ public class TabModelImpl extends TabModelJniBridge {
         }
 
         if (!uponExit && canUndo && supportsPendingClosures()) {
-            for (TabModelObserver obs : mObservers)
+            for (TabModelObserver obs : mObservers) {
                 obs.multipleTabsPendingClosure(closedTabs, true);
+            }
         }
     }
 
@@ -624,7 +626,7 @@ public class TabModelImpl extends TabModelJniBridge {
         if (mTabContentManager != null) mTabContentManager.removeTabThumbnail(tab.getId());
         mTabSaver.removeTabFromQueues(tab);
 
-        if (!isIncognito()) tab.createHistoricalTab();
+        if (!isIncognito()) TabState.createHistoricalTab(tab);
 
         for (TabModelObserver obs : mObservers) obs.didCloseTab(tab.getId(), tab.isIncognito());
         if (notifyTabClosureCommitted) {

chrome/browser/android/tab_android.cc

@@ -27,7 +27,6 @@
 #include "chrome/browser/profiles/profile_android.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
-#include "chrome/browser/sessions/tab_restore_service_factory.h"
 #include "chrome/browser/sync/glue/synced_tab_delegate_android.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/ui/android/context_menu_helper.h"
@@ -40,8 +39,6 @@
 #include "chrome/browser/ui/tab_contents/core_tab_helper.h"
 #include "chrome/browser/ui/tab_helpers.h"
 #include "chrome/common/url_constants.h"
-#include "components/sessions/content/content_live_tab.h"
-#include "components/sessions/core/tab_restore_service.h"
 #include "components/url_formatter/url_fixer.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/devtools_agent_host.h"
@@ -429,34 +426,6 @@ void TabAndroid::SetActiveNavigationEntryTitleForUrl(
     entry->SetTitle(title);
 }
 
-// static
-void TabAndroid::CreateHistoricalTabFromContents(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);
-}
-
-void TabAndroid::CreateHistoricalTab(JNIEnv* env,
-                                     const JavaParamRef<jobject>& obj) {
-  TabAndroid::CreateHistoricalTabFromContents(web_contents());
-}
-
 void TabAndroid::LoadOriginalImage(JNIEnv* env,
                                    const JavaParamRef<jobject>& obj) {
   content::RenderFrameHost* render_frame_host =

chrome/browser/android/tab_android.h

@@ -161,12 +161,6 @@ class TabAndroid {
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& obj);
 
-  void CreateHistoricalTab(JNIEnv* env,
-                           const base::android::JavaParamRef<jobject>& obj);
-
-  static void CreateHistoricalTabFromContents(
-      content::WebContents* web_contents);
-
   void LoadOriginalImage(JNIEnv* env,
                          const base::android::JavaParamRef<jobject>& obj);
 

chrome/browser/android/tab_state.cc

@@ -19,9 +19,13 @@
 #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"
@@ -374,6 +378,27 @@ 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(
@@ -612,5 +637,14 @@ static void JNI_TabState_CreateHistoricalTab(JNIEnv* env,
       WebContentsState::RestoreContentsFromByteBuffer(
           env, state, saved_state_version, true)));
   if (web_contents.get())
-    TabAndroid::CreateHistoricalTabFromContents(web_contents.get());
+    CreateHistoricalTab(web_contents.get());
+}
+
+// static
+static void JNI_TabState_CreateHistoricalTabFromContents(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& jweb_contents) {
+  auto* web_contents = content::WebContents::FromJavaWebContents(jweb_contents);
+  if (web_contents)
+    CreateHistoricalTab(web_contents);
 }