Reland "Break down WebContentsStateBridge#createHistoricalTab()"

Reland note: Fixed version without NPE.

WebContentsStateBridge#createHistoricalTab() can be accomplished by
calling other methods:
- WebContentsStateBridge#restoreContentsFromByteBuffer()
- WebContentsStateBridge#createHistoricalTabFromContents()
- WebContents#destroy()

Replace it with these calls.

Bug: 1090048

Original change's description:
> Revert "Break down WebContentsStateBridge#createHistoricalTab()"
>
> This reverts commit 864ff6e1.
>
> Reason for revert: crbug.com/1100325
> TBR=dtrainor@chromium.org,jinsukkim@chromium.org,hnakashima@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 1090048
> Change-Id: I73d490b12526ab8f26171f8c4fbf42fcbe284c17
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2273682
> Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
> Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#783626}

TBR=dtrainor@chromium.org,jinsukkim@chromium.org,hnakashima@chromium.org


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

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

@@ -1076,7 +1076,8 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
     }
 
     /** @return WebContentsState representing the state of the WebContents (navigations, etc.) */
-    WebContentsState getFrozenContentsState() {
+    @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
+    public WebContentsState getFrozenContentsState() {
         return mFrozenContentsState;
     }
 

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

@@ -7,6 +7,8 @@ package org.chromium.chrome.browser.tab;
 import android.graphics.Color;
 
 import androidx.annotation.Nullable;
+
+import org.chromium.content_public.browser.WebContents;
 import org.chromium.ui.util.ColorUtils;
 
 /**
@@ -57,13 +59,18 @@ public class TabState {
      * Creates a historical tab from a tab being closed.
      */
     public static void createHistoricalTab(Tab tab) {
-        if (!tab.isFrozen()) {
-            WebContentsStateBridge.createHistoricalTabFromContents(tab.getWebContents());
-        } else {
+        if (tab.isFrozen()) {
             WebContentsState state = ((TabImpl) tab).getFrozenContentsState();
             if (state != null) {
-                WebContentsStateBridge.createHistoricalTab(state);
+                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

@@ -5,6 +5,7 @@
 package org.chromium.chrome.browser.tab;
 
 import androidx.annotation.Nullable;
+import androidx.annotation.VisibleForTesting;
 
 import org.chromium.base.annotations.NativeMethods;
 import org.chromium.content_public.browser.WebContents;
@@ -80,17 +81,13 @@ public class WebContentsStateBridge {
                 contentsState.buffer(), contentsState.version());
     }
 
-    public static void createHistoricalTab(WebContentsState contentsState) {
-        WebContentsStateBridgeJni.get().createHistoricalTab(
-                contentsState.buffer(), contentsState.version());
-    }
-
     public static void createHistoricalTabFromContents(WebContents webContents) {
         WebContentsStateBridgeJni.get().createHistoricalTabFromContents(webContents);
     }
 
     @NativeMethods
-    interface Natives {
+    @VisibleForTesting
+    public interface Natives {
         WebContents restoreContentsFromByteBuffer(
                 ByteBuffer buffer, int savedStateVersion, boolean initiallyHidden);
         ByteBuffer getContentsStateAsByteBuffer(Tab tab);
@@ -99,7 +96,6 @@ public class WebContentsStateBridge {
                 int referrerPolicy, Origin initiatorOrigin, boolean isIncognito);
         String getDisplayTitleFromByteBuffer(ByteBuffer state, int savedStateVersion);
         String getVirtualUrlFromByteBuffer(ByteBuffer state, int savedStateVersion);
-        void createHistoricalTab(ByteBuffer state, int savedStateVersion);
         void createHistoricalTabFromContents(WebContents webContents);
     }
 }

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

@@ -5,27 +5,41 @@
 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;
 
 /**
@@ -46,6 +60,23 @@ 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();
@@ -109,4 +140,58 @@ 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/android/web_contents_state.cc

@@ -626,19 +626,6 @@ JNI_WebContentsStateBridge_GetVirtualUrlFromByteBuffer(
   return result;
 }
 
-// Creates a historical tab entry from the serialized tab contents contained
-// within |state|.
-static void JNI_WebContentsStateBridge_CreateHistoricalTab(
-    JNIEnv* env,
-    const JavaParamRef<jobject>& state,
-    jint saved_state_version) {
-  std::unique_ptr<WebContents> web_contents(WebContents::FromJavaWebContents(
-      WebContentsState::RestoreContentsFromByteBuffer(
-          env, state, saved_state_version, true)));
-  if (web_contents.get())
-    CreateHistoricalTab(web_contents.get());
-}
-
 // static
 static void JNI_WebContentsStateBridge_CreateHistoricalTabFromContents(
     JNIEnv* env,