weblayer: adds API to know if Browser is restoring and when done

This way embedders have API to know when they can take action based on whether restore has completed. BUG=1135278 TEST=covered by tests Change-Id: I72d804a86d8c4eeafdc3b72bbcf16875a78eb477 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2451550 Commit-Queue: Scott Violet <sky@chromium.org> Reviewed-by: Clark DuVall <cduvall@chromium.org> Cr-Commit-Position: refs/heads/master@{#814450}
qweri0p · Oct 6, 2020 · 3692e6b · 3692e6b
1 parent e995da4
commit 3692e6b
Show file tree

Hide file tree

Showing 16 changed files with 281 additions and 40 deletions.
diff --git a/weblayer/BUILD.gn b/weblayer/BUILD.gn
@@ -299,6 +299,7 @@ source_set("weblayer_lib_base") {
     "public/browser.cc",
     "public/browser.h",
     "public/browser_observer.h",
+    "public/browser_restore_observer.h",
     "public/common/switches.cc",
     "public/common/switches.h",
     "public/cookie_manager.h",

diff --git a/...rowser/android/javatests/src/org/chromium/weblayer/test/BrowserFragmentLifecycleTest.java b/...rowser/android/javatests/src/org/chromium/weblayer/test/BrowserFragmentLifecycleTest.java
@@ -19,11 +19,13 @@
 import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import org.chromium.weblayer.Browser;
+import org.chromium.weblayer.BrowserRestoreCallback;
 import org.chromium.weblayer.Navigation;
 import org.chromium.weblayer.NavigationCallback;
 import org.chromium.weblayer.NavigationController;
 import org.chromium.weblayer.Profile;
 import org.chromium.weblayer.Tab;
+import org.chromium.weblayer.WebLayer;
 import org.chromium.weblayer.shell.InstrumentationActivity;
 
 import java.util.HashMap;
@@ -45,6 +47,16 @@ private Tab getTab() {
                 () -> mActivityTestRule.getActivity().getTab());
     }
 
+    private boolean isRestoringPreviousState() {
+        return TestThreadUtils.runOnUiThreadBlockingNoException(
+                () -> mActivityTestRule.getActivity().getBrowser().isRestoringPreviousState());
+    }
+
+    private int getSupportedMajorVersion() {
+        return TestThreadUtils.runOnUiThreadBlockingNoException(
+                () -> WebLayer.getSupportedMajorVersion(mActivityTestRule.getActivity()));
+    }
+
     @Test
     @SmallTest
     public void successfullyLoadsUrlAfterRecreation() {
@@ -127,12 +139,18 @@ private void restoresPreviousSession(Bundle extras) {
         extras.putString(InstrumentationActivity.EXTRA_PERSISTENCE_ID, "x");
         final String url = mActivityTestRule.getTestDataURL("simple_page.html");
         mActivityTestRule.launchShellWithUrl(url, extras);
+        if (getSupportedMajorVersion() >= 88) {
+            Assert.assertFalse(isRestoringPreviousState());
+        }
 
         mActivityTestRule.recreateActivity();
 
         Tab tab = getTab();
         Assert.assertNotNull(tab);
         waitForTabToFinishRestore(tab, url);
+        if (getSupportedMajorVersion() >= 88) {
+            Assert.assertFalse(isRestoringPreviousState());
+        }
     }
 
     @Test
@@ -201,7 +219,7 @@ public void restoresTabData() throws Throwable {
 
         Map<String, String> initialData = new HashMap<>();
         initialData.put("foo", "bar");
-        restoresTabData(extras, initialData);
+        restoreTabData(extras, initialData);
     }
 
     @Test
@@ -210,10 +228,10 @@ public void restoresTabData() throws Throwable {
     public void restoreTabDataAfterRecreate() throws Throwable {
         Map<String, String> initialData = new HashMap<>();
         initialData.put("foo", "bar");
-        restoresTabData(new Bundle(), initialData);
+        restoreTabData(new Bundle(), initialData);
     }
 
-    private void restoresTabData(Bundle extras, Map<String, String> initialData) {
+    private void restoreTabData(Bundle extras, Map<String, String> initialData) {
         String url = mActivityTestRule.getTestDataURL("simple_page.html");
         mActivityTestRule.launchShellWithUrl(url, extras);
 
@@ -304,4 +322,46 @@ public void browserAndTabIsDestroyedWhenFragmentDestroyed() throws Throwable {
             Assert.assertTrue(tab.isDestroyed());
         });
     }
+
+    @Test
+    @SmallTest
+    @MinWebLayerVersion(88)
+    public void restoreUsingOnRestoreCompleted() throws Throwable {
+        final String persistenceId = "x";
+        Bundle extras = new Bundle();
+        extras.putString(InstrumentationActivity.EXTRA_PERSISTENCE_ID, persistenceId);
+        CallbackHelper callbackHelper = new CallbackHelper();
+        InstrumentationActivity.registerOnCreatedCallback(
+                new InstrumentationActivity.OnCreatedCallback() {
+                    private int mBrowserCreateCount;
+                    @Override
+                    public void onCreated(Browser browser) {
+                        if (mBrowserCreateCount == 0) {
+                            // Initial creation.
+                            mBrowserCreateCount = 1;
+                            // isRestoringPreviousState() is true for the initial creation as
+                            // persistence code has to check disk, which is async.
+                            Assert.assertTrue(browser.isRestoringPreviousState());
+                        } else if (mBrowserCreateCount == 1) {
+                            // The activity was recreated.
+                            mBrowserCreateCount = 2;
+                            Assert.assertTrue(browser.isRestoringPreviousState());
+                            browser.registerBrowserRestoreCallback(new BrowserRestoreCallback() {
+                                @Override
+                                public void onRestoreCompleted() {
+                                    Assert.assertFalse(browser.isRestoringPreviousState());
+                                    callbackHelper.notifyCalled();
+                                }
+                            });
+                        } else {
+                            Assert.fail("Unexpected phase");
+                        }
+                    }
+                });
+        final String url = mActivityTestRule.getTestDataURL("simple_page.html");
+        mActivityTestRule.launchShellWithUrl(url, extras);
+
+        mActivityTestRule.recreateActivity();
+        callbackHelper.waitForFirst();
+    }
 }
diff --git a/weblayer/browser/browser_impl.cc b/weblayer/browser/browser_impl.cc
@@ -24,6 +24,7 @@
 #include "weblayer/browser/tab_impl.h"
 #include "weblayer/common/weblayer_paths.h"
 #include "weblayer/public/browser_observer.h"
+#include "weblayer/public/browser_restore_observer.h"
 
 #if defined(OS_ANDROID)
 #include "base/android/callback_android.h"
@@ -329,6 +330,14 @@ Tab* BrowserImpl::CreateTab() {
   return CreateTab(nullptr);
 }
 
+void BrowserImpl::OnRestoreCompleted() {
+  for (BrowserRestoreObserver& obs : browser_restore_observers_)
+    obs.OnRestoreCompleted();
+#if defined(OS_ANDROID)
+  Java_BrowserImpl_onRestoreCompleted(AttachCurrentThread(), java_impl_);
+#endif
+}
+
 void BrowserImpl::PrepareForShutdown() {
   browser_persister_.reset();
 }
@@ -342,6 +351,10 @@ std::vector<uint8_t> BrowserImpl::GetMinimalPersistenceState() {
   return GetMinimalPersistenceState(0);
 }
 
+bool BrowserImpl::IsRestoringPreviousState() {
+  return browser_persister_ && browser_persister_->is_restore_in_progress();
+}
+
 void BrowserImpl::AddObserver(BrowserObserver* observer) {
   browser_observers_.AddObserver(observer);
 }
@@ -350,6 +363,15 @@ void BrowserImpl::RemoveObserver(BrowserObserver* observer) {
   browser_observers_.RemoveObserver(observer);
 }
 
+void BrowserImpl::AddBrowserRestoreObserver(BrowserRestoreObserver* observer) {
+  browser_restore_observers_.AddObserver(observer);
+}
+
+void BrowserImpl::RemoveBrowserRestoreObserver(
+    BrowserRestoreObserver* observer) {
+  browser_restore_observers_.RemoveObserver(observer);
+}
+
 void BrowserImpl::VisibleSecurityStateOfActiveTabChanged() {
   if (visible_security_state_changed_callback_for_tests_)
     std::move(visible_security_state_changed_callback_for_tests_).Run();

diff --git a/weblayer/browser/browser_impl.h b/weblayer/browser/browser_impl.h
@@ -57,6 +57,9 @@ class BrowserImpl : public Browser {
       const std::string& guid);
   TabImpl* CreateTab(std::unique_ptr<content::WebContents> web_contents);
 
+  // Called from BrowserPersister when restore has completed.
+  void OnRestoreCompleted();
+
 #if defined(OS_ANDROID)
   bool CompositorHasSurface();
 
@@ -83,6 +86,9 @@ class BrowserImpl : public Browser {
   void OnFragmentStart(JNIEnv* env);
   void OnFragmentResume(JNIEnv* env);
   void OnFragmentPause(JNIEnv* env);
+  bool IsRestoringPreviousState(JNIEnv* env) {
+    return IsRestoringPreviousState();
+  }
 
   bool fragment_resumed() { return fragment_resumed_; }
 #endif
@@ -118,8 +124,11 @@ class BrowserImpl : public Browser {
   void PrepareForShutdown() override;
   std::string GetPersistenceId() override;
   std::vector<uint8_t> GetMinimalPersistenceState() override;
+  bool IsRestoringPreviousState() override;
   void AddObserver(BrowserObserver* observer) override;
   void RemoveObserver(BrowserObserver* observer) override;
+  void AddBrowserRestoreObserver(BrowserRestoreObserver* observer) override;
+  void RemoveBrowserRestoreObserver(BrowserRestoreObserver* observer) override;
   void VisibleSecurityStateOfActiveTabChanged() override;
 
  private:
@@ -149,6 +158,7 @@ class BrowserImpl : public Browser {
   base::android::ScopedJavaGlobalRef<jobject> java_impl_;
 #endif
   base::ObserverList<BrowserObserver> browser_observers_;
+  base::ObserverList<BrowserRestoreObserver> browser_restore_observers_;
   ProfileImpl* const profile_;
   std::vector<std::unique_ptr<Tab>> tabs_;
   TabImpl* active_tab_ = nullptr;

diff --git a/weblayer/browser/java/org/chromium/weblayer_private/BrowserImpl.java b/weblayer/browser/java/org/chromium/weblayer_private/BrowserImpl.java
@@ -494,6 +494,16 @@ public void onBrowserControlsOffsetsChanged(TabImpl tab, boolean isTop, int cont
         }
     }
 
+    @Override
+    public boolean isRestoringPreviousState() {
+        return BrowserImplJni.get().isRestoringPreviousState(mNativeBrowser);
+    }
+
+    @CalledByNative
+    private void onRestoreCompleted() throws RemoteException {
+        mClient.onRestoreCompleted();
+    }
+
     public View getFragmentView() {
         return getViewController().getView();
     }
@@ -645,5 +655,6 @@ void restoreStateIfNecessary(long nativeBrowserImpl, String persistenceId,
         void onFragmentStart(long nativeBrowserImpl);
         void onFragmentResume(long nativeBrowserImpl);
         void onFragmentPause(long nativeBrowserImpl);
+        boolean isRestoringPreviousState(long nativeBrowserImpl);
     }
 }
diff --git a/weblayer/browser/java/org/chromium/weblayer_private/interfaces/IBrowser.aidl b/weblayer/browser/java/org/chromium/weblayer_private/interfaces/IBrowser.aidl
@@ -43,4 +43,7 @@ interface IBrowser {
 
   // Added in 87.
   void setBrowserControlsOffsetsEnabled(in boolean enable) = 13;
+
+  // Added in 88.
+  boolean isRestoringPreviousState() = 14;
 }
diff --git a/weblayer/browser/java/org/chromium/weblayer_private/interfaces/IBrowserClient.aidl b/weblayer/browser/java/org/chromium/weblayer_private/interfaces/IBrowserClient.aidl
@@ -16,4 +16,7 @@ interface IBrowserClient {
   IRemoteFragment createMediaRouteDialogFragment() = 3;
   void onBrowserControlsOffsetsChanged(in boolean isTop,
                                        in int controlsOffset) = 4;
+
+  // Added in 88.
+  void onRestoreCompleted() = 5;
 }
diff --git a/weblayer/browser/persistence/browser_persister.cc b/weblayer/browser/persistence/browser_persister.cc
@@ -277,6 +277,9 @@ void BrowserPersister::OnGotCurrentSessionCommands(
   ScheduleRebuildOnNextSave();
 
   RestoreBrowserState(browser_, std::move(commands));
+
+  is_restore_in_progress_ = false;
+  browser_->OnRestoreCompleted();
 }
 
 void BrowserPersister::BuildCommandsForTab(TabImpl* tab, int index_in_browser) {

diff --git a/weblayer/browser/persistence/browser_persister.h b/weblayer/browser/persistence/browser_persister.h
@@ -53,6 +53,8 @@ class BrowserPersister : public sessions::CommandStorageManagerDelegate,
 
   ~BrowserPersister() override;
 
+  bool is_restore_in_progress() const { return is_restore_in_progress_; }
+
   void SaveIfNecessary();
 
   // Returns the key used to encrypt the file. Empty if not encrypted.
@@ -139,6 +141,9 @@ class BrowserPersister : public sessions::CommandStorageManagerDelegate,
                  &TabImpl::RemoveDataObserver>
       data_observer_{this};
 
+  // True while asynchronously reading the state to restore.
+  bool is_restore_in_progress_ = true;
+
   base::WeakPtrFactory<BrowserPersister> weak_factory_{this};
 };