Move WebContentsState to CriticalPersistedTabData

To support the work of moving Tab attributes out of Tab to
objects which implement the UserData interface so the Tab
doesn't become too big and unwieldy

Bug: 1109345
Change-Id: Ife994234aa53ae1f12dde47cee0abe91765b3491
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2318625
Commit-Queue: David Maunder <davidjm@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792700}

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

@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.tab;
 
 import org.chromium.base.annotations.NativeMethods;
 import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.tab.state.CriticalPersistedTabData;
 import org.chromium.content_public.browser.WebContents;
 
 /**
@@ -17,7 +18,7 @@ public class HistoricalTabSaver {
      */
     public static void createHistoricalTab(Tab tab, Profile profile) {
         if (tab.isFrozen()) {
-            WebContentsState state = ((TabImpl) tab).getFrozenContentsState();
+            WebContentsState state = CriticalPersistedTabData.from(tab).getWebContentsState();
             if (state != null) {
                 WebContents webContents =
                         WebContentsStateBridge.restoreContentsFromByteBuffer(state, profile, true);

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

@@ -139,14 +139,6 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
 
     private @Nullable @TabCreationState Integer mCreationState;
 
-    /**
-     * Navigation state of the WebContents as returned by nativeGetContentsStateAsByteBuffer(),
-     * stored to be inflated on demand using unfreezeContents(). If this is not null, there is no
-     * WebContents around. Upon tab switch WebContents will be unfrozen and the variable will be set
-     * to null.
-     */
-    private WebContentsState mFrozenContentsState;
-
     /**
      * URL load to be performed lazily when the Tab is next shown.
      */
@@ -808,7 +800,10 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
 
             // If there is a frozen WebContents state or a pending lazy load, don't create a new
             // WebContents. Restoring will be done when showing the tab in the foreground.
-            if (getFrozenContentsState() != null || getPendingLoadParams() != null) return;
+            if (CriticalPersistedTabData.from(this).getWebContentsState() != null
+                    || getPendingLoadParams() != null) {
+                return;
+            }
 
             boolean creatingWebContents = webContents == null;
             if (creatingWebContents) {
@@ -852,7 +847,7 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
      */
     void restoreFieldsFromState(TabState state) {
         assert state != null;
-        mFrozenContentsState = state.contentsState;
+        CriticalPersistedTabData.from(this).setWebContentsState(state.contentsState);
         mTimestampMillis = state.timestampMillis;
         mUrl = new GURL(state.contentsState.getVirtualUrlFromState());
         mTitle = state.contentsState.getDisplayTitleFromState();
@@ -1044,12 +1039,6 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
         mIsShowingErrorPage = isShowingErrorPage;
     }
 
-    /** @return WebContentsState representing the state of the WebContents (navigations, etc.) */
-    @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
-    public WebContentsState getFrozenContentsState() {
-        return mFrozenContentsState;
-    }
-
     /**
      * Shows a native page for url if it's a valid chrome-native URL. Otherwise, does nothing.
      * @param url The url of the current navigation.
@@ -1386,7 +1375,8 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
             TraceEvent.begin("Tab.restoreIfNeeded");
             // Restore is needed for a tab that is loaded for the first time. WebContents will
             // be restored from a saved state.
-            if ((isFrozen() && mFrozenContentsState != null && !unfreezeContents())
+            if ((isFrozen() && CriticalPersistedTabData.from(this).getWebContentsState() != null
+                        && !unfreezeContents())
                     || !needsReload()) {
                 return;
             }
@@ -1408,10 +1398,12 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
         boolean restored = true;
         try {
             TraceEvent.begin("Tab.unfreezeContents");
-            assert mFrozenContentsState != null;
+            WebContentsState webContentsState =
+                    CriticalPersistedTabData.from(this).getWebContentsState();
+            assert webContentsState != null;
 
             WebContents webContents = WebContentsStateBridge.restoreContentsFromByteBuffer(
-                    mFrozenContentsState, Profile.getLastUsedRegularProfile(), isHidden());
+                    webContentsState, Profile.getLastUsedRegularProfile(), isHidden());
             if (webContents == null) {
                 // State restore failed, just create a new empty web contents as that is the best
                 // that can be done at this point. TODO(jcivelli) http://b/5910521 - we should show
@@ -1423,7 +1415,7 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
             View compositorView = getActivity().getCompositorViewHolder();
             webContents.setSize(compositorView.getWidth(), compositorView.getHeight());
 
-            mFrozenContentsState = null;
+            CriticalPersistedTabData.from(this).setWebContentsState(null);
             initWebContents(webContents);
 
             if (!restored) {
@@ -1456,11 +1448,13 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
      */
     @CalledByNative
     private void deleteNavigationEntriesFromFrozenState(long predicate) {
-        if (mFrozenContentsState == null) return;
+        WebContentsState webContentsState =
+                CriticalPersistedTabData.from(this).getWebContentsState();
+        if (webContentsState == null) return;
         WebContentsState newState =
-                WebContentsStateBridge.deleteNavigationEntries(mFrozenContentsState, predicate);
+                WebContentsStateBridge.deleteNavigationEntries(webContentsState, predicate);
         if (newState != null) {
-            mFrozenContentsState = newState;
+            CriticalPersistedTabData.from(this).setWebContentsState(newState);
             notifyNavigationEntriesDeleted();
         }
     }

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

@@ -42,7 +42,9 @@ public class TabStateExtractor {
      * @param tab The {@link TabImpl} from which to extract the WebContents state.
      **/
     public static WebContentsState getWebContentsState(TabImpl tab) {
-        if (tab.getFrozenContentsState() != null) return tab.getFrozenContentsState();
+        if (CriticalPersistedTabData.from(tab).getWebContentsState() != null) {
+            return CriticalPersistedTabData.from(tab).getWebContentsState();
+        }
 
         // Native call returns null when buffer allocation needed to serialize the state failed.
         ByteBuffer buffer = getWebContentsStateAsByteBuffer(tab);

chrome/android/javatests/src/org/chromium/chrome/browser/tab/state/CriticalPersistedTabDataTest.java

@@ -19,8 +19,10 @@ import org.chromium.base.test.BaseJUnit4ClassRunner;
 import org.chromium.base.test.util.DisabledTest;
 import org.chromium.chrome.browser.tab.MockTab;
 import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.chrome.browser.tab.WebContentsState;
 import org.chromium.chrome.test.ChromeBrowserTestRule;
 
+import java.nio.ByteBuffer;
 import java.util.concurrent.Semaphore;
 
 /**
@@ -35,13 +37,19 @@ public class CriticalPersistedTabDataTest {
     private static final int PARENT_ID = 2;
     private static final int ROOT_ID = 3;
     private static final int CONTENT_STATE_VERSION = 42;
-    private static final byte[] CONTENT_STATE = {9, 10};
+    private static final byte[] WEB_CONTENTS_STATE_BYTES = {9, 10};
+    private static final WebContentsState WEB_CONTENTS_STATE =
+            new WebContentsState(ByteBuffer.allocate(WEB_CONTENTS_STATE_BYTES.length));
     private static final long TIMESTAMP = 203847028374L;
     private static final String APP_ID = "AppId";
     private static final String OPENER_APP_ID = "OpenerAppId";
     private static final int THEME_COLOR = 5;
     private static final int LAUNCH_TYPE_AT_CREATION = 3;
 
+    static {
+        WEB_CONTENTS_STATE.buffer().put(WEB_CONTENTS_STATE_BYTES);
+    }
+
     private CriticalPersistedTabData mCriticalPersistedTabData;
 
     private static Tab mockTab(int id, boolean isEncrypted) {
@@ -81,7 +89,7 @@ public class CriticalPersistedTabDataTest {
                     PersistedTabDataConfiguration.get(CriticalPersistedTabData.class, isEncrypted);
             CriticalPersistedTabData criticalPersistedTabData =
                     new CriticalPersistedTabData(mockTab(TAB_ID, isEncrypted), PARENT_ID, ROOT_ID,
-                            TIMESTAMP, CONTENT_STATE, CONTENT_STATE_VERSION, OPENER_APP_ID,
+                            TIMESTAMP, WEB_CONTENTS_STATE, CONTENT_STATE_VERSION, OPENER_APP_ID,
                             THEME_COLOR, LAUNCH_TYPE_AT_CREATION, config.storage, config.id);
             criticalPersistedTabData.save();
             CriticalPersistedTabData.from(mockTab(TAB_ID, isEncrypted), callback);
@@ -97,7 +105,8 @@ public class CriticalPersistedTabDataTest {
         Assert.assertEquals(mCriticalPersistedTabData.getThemeColor(), THEME_COLOR);
         Assert.assertEquals(
                 mCriticalPersistedTabData.getTabLaunchTypeAtCreation(), LAUNCH_TYPE_AT_CREATION);
-        Assert.assertArrayEquals(mCriticalPersistedTabData.getContentStateBytes(), CONTENT_STATE);
+        Assert.assertArrayEquals(mCriticalPersistedTabData.getWebContentsState().buffer().array(),
+                WEB_CONTENTS_STATE_BYTES);
 
         ThreadUtils.runOnUiThreadBlocking(() -> {
             mCriticalPersistedTabData.delete();

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

@@ -21,6 +21,8 @@ import org.robolectric.annotation.Config;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.base.test.util.JniMocker;
 import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.tab.state.CriticalPersistedTabData;
+import org.chromium.chrome.browser.tasks.tab_management.TabUiUnitTestUtils;
 import org.chromium.content_public.browser.WebContents;
 
 import java.nio.ByteBuffer;
@@ -42,6 +44,8 @@ public class HistoricalTabSaverUnitTest {
     @Mock
     public TabImpl mTabImplMock;
     @Mock
+    public CriticalPersistedTabData mCriticalPersistedTabData;
+    @Mock
     public Profile mProfileMock;
     @Mock
     public Profile mOffTheRecordProfileMock;
@@ -53,6 +57,8 @@ public class HistoricalTabSaverUnitTest {
         MockitoAnnotations.initMocks(this);
         mocker.mock(HistoricalTabSaverJni.TEST_HOOKS, mHistoricalTabSaverJni);
         mocker.mock(WebContentsStateBridgeJni.TEST_HOOKS, mWebContentsStateBridgeJni);
+        TabUiUnitTestUtils.prepareTab(
+                mTabImplMock, CriticalPersistedTabData.class, mCriticalPersistedTabData);
     }
 
     @Test
@@ -68,7 +74,7 @@ public class HistoricalTabSaverUnitTest {
     @Test
     public void testCreateHistoricalTab_Frozen_NullWebContentsState_HistoricalTabNotCreated() {
         doReturn(true).when(mTabImplMock).isFrozen();
-        doReturn(null).when(mTabImplMock).getFrozenContentsState();
+        doReturn(null).when(mCriticalPersistedTabData).getWebContentsState();
 
         HistoricalTabSaver.createHistoricalTab(mTabImplMock, mProfileMock);
 
@@ -82,7 +88,7 @@ public class HistoricalTabSaverUnitTest {
         webContentsState.setVersion(123);
 
         doReturn(true).when(mTabImplMock).isFrozen();
-        doReturn(webContentsState).when(mTabImplMock).getFrozenContentsState();
+        doReturn(webContentsState).when(mCriticalPersistedTabData).getWebContentsState();
         doReturn(mOffTheRecordProfileMock).when(mProfileMock).getPrimaryOTRProfile();
         doReturn(null)
                 .when(mWebContentsStateBridgeJni)
@@ -101,7 +107,7 @@ public class HistoricalTabSaverUnitTest {
         webContentsState.setVersion(123);
 
         doReturn(true).when(mTabImplMock).isFrozen();
-        doReturn(webContentsState).when(mTabImplMock).getFrozenContentsState();
+        doReturn(webContentsState).when(mCriticalPersistedTabData).getWebContentsState();
         doReturn(mOffTheRecordProfileMock).when(mProfileMock).getPrimaryOTRProfile();
         doReturn(mWebContentsMock)
                 .when(mWebContentsStateBridgeJni)

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

@@ -20,6 +20,7 @@ import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.robolectric.annotation.Config;
 
+import org.chromium.base.UserDataHost;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.base.test.util.JniMocker;
 import org.chromium.chrome.browser.profiles.Profile;
@@ -62,6 +63,7 @@ public class TabStateExtractorTest {
         mocker.mock(WebContentsStateBridgeJni.TEST_HOOKS, mWebContentsBridgeJni);
 
         Profile.setLastUsedProfileForTesting(mProfileMock);
+        doReturn(new UserDataHost()).when(mTabMock).getUserDataHost();
     }
 
     @Test

chrome/browser/tab/java/src/org/chromium/chrome/browser/tab/state/CriticalPersistedTabData.java

@@ -16,8 +16,10 @@ import org.chromium.base.Log;
 import org.chromium.base.ObserverList;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TabLaunchType;
+import org.chromium.chrome.browser.tab.WebContentsState;
 import org.chromium.chrome.browser.tab.proto.CriticalPersistedTabData.CriticalPersistedTabDataProto;
 
+import java.nio.ByteBuffer;
 import java.util.Locale;
 
 /**
@@ -33,7 +35,13 @@ public class CriticalPersistedTabData extends PersistedTabData {
     private int mParentId;
     private int mRootId;
     private long mTimestampMillis;
-    private byte[] mContentStateBytes;
+    /**
+     * Navigation state of the WebContents as returned by nativeGetContentsStateAsByteBuffer(),
+     * stored to be inflated on demand using unfreezeContents(). If this is not null, there is no
+     * WebContents around. Upon tab switch WebContents will be unfrozen and the variable will be set
+     * to null.
+     */
+    private WebContentsState mWebContentsState;
     private int mContentStateVersion;
     private String mOpenerAppId;
     private int mThemeColor;
@@ -65,14 +73,14 @@ public class CriticalPersistedTabData extends PersistedTabData {
      */
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     CriticalPersistedTabData(Tab tab, int parentId, int rootId, long timestampMillis,
-            byte[] contentStateBytes, int contentStateVersion, String openerAppId, int themeColor,
-            int launchTypeAtCreation, PersistedTabDataStorage persistedTabDataStorage,
-            String persistedTabDataId) {
+            WebContentsState webContentsState, int contentStateVersion, String openerAppId,
+            int themeColor, int launchTypeAtCreation,
+            PersistedTabDataStorage persistedTabDataStorage, String persistedTabDataId) {
         super(tab, persistedTabDataStorage, persistedTabDataId);
         mParentId = parentId;
         mRootId = rootId;
         mTimestampMillis = timestampMillis;
-        mContentStateBytes = contentStateBytes;
+        mWebContentsState = webContentsState;
         mContentStateVersion = contentStateVersion;
         mOpenerAppId = openerAppId;
         mThemeColor = themeColor;
@@ -148,7 +156,11 @@ public class CriticalPersistedTabData extends PersistedTabData {
             mParentId = criticalPersistedTabDataProto.getParentId();
             mRootId = criticalPersistedTabDataProto.getRootId();
             mTimestampMillis = criticalPersistedTabDataProto.getTimestampMillis();
-            mContentStateBytes = criticalPersistedTabDataProto.getContentStateBytes().toByteArray();
+            byte[] webContentsStateBytes =
+                    criticalPersistedTabDataProto.getWebContentsStateBytes().toByteArray();
+            mWebContentsState =
+                    new WebContentsState(ByteBuffer.allocate(webContentsStateBytes.length));
+            mWebContentsState.buffer().put(webContentsStateBytes);
             mContentStateVersion = criticalPersistedTabDataProto.getContentStateVersion();
             mOpenerAppId = criticalPersistedTabDataProto.getOpenerAppId();
             mThemeColor = criticalPersistedTabDataProto.getThemeColor();
@@ -255,7 +267,7 @@ public class CriticalPersistedTabData extends PersistedTabData {
                 .setParentId(mParentId)
                 .setRootId(mRootId)
                 .setTimestampMillis(mTimestampMillis)
-                .setContentStateBytes(ByteString.copyFrom(mContentStateBytes))
+                .setWebContentsStateBytes(ByteString.copyFrom(mWebContentsState.buffer()))
                 .setContentStateVersion(mContentStateVersion)
                 .setOpenerAppId(mOpenerAppId)
                 .setThemeColor(mThemeColor)
@@ -326,8 +338,12 @@ public class CriticalPersistedTabData extends PersistedTabData {
     /**
      * @return content state bytes for the {@link Tab}
      */
-    public byte[] getContentStateBytes() {
-        return mContentStateBytes;
+    public WebContentsState getWebContentsState() {
+        return mWebContentsState;
+    }
+
+    public void setWebContentsState(WebContentsState webContentsState) {
+        mWebContentsState = webContentsState;
     }
 
     /**

chrome/browser/tab/java/src/org/chromium/chrome/browser/tab/state/proto/critical_persisted_tab_data.proto

@@ -18,8 +18,8 @@ message CriticalPersistedTabDataProto {
   // Timestamp when Tab was created.
   optional int64 timestamp_millis = 3;
 
-  // Content State.
-  optional bytes content_state_bytes = 4;
+  // WebContentsState.
+  optional bytes web_contents_state_bytes = 4;
 
   // Content State version.
   optional int32 content_state_version = 5;