Bug 1661480 - Restore scrolling position and form data. r=droeh

When migrating RestoreState to actors we didn't consider that the child actor
gets recreated at every navigation, as its lifetime is tied to the inner
window.

This means that restoring state in one step is not possible, as restoring the
history will trigger a navigation from `about:blank` to the restored page.

To achieve this, we split restoring in two steps and we keep the state on the
parent actor instead of the child.

We move the restoring logic to a newly added GeckoViewContent parent actor,
which is more readibly accessible from both geckoview.js and
GeckoViewContent.jsm.

Differential Revision: https://phabricator.services.mozilla.com/D88637

mobile/android/actors/GeckoViewContentChild.jsm

@@ -33,6 +33,12 @@ XPCOMUtils.defineLazyModuleGetters(this, {
 var EXPORTED_SYMBOLS = ["GeckoViewContentChild"];
 
 class GeckoViewContentChild extends GeckoViewActorChild {
+  actorCreated() {
+    this.pageShow = new Promise(resolve => {
+      this.receivedPageShow = resolve;
+    });
+  }
+
   toPixels(aLength, aType) {
     const { contentWindow } = this;
     if (aType === SCREEN_LENGTH_TYPE_PIXEL) {
@@ -179,99 +185,17 @@ class GeckoViewContentChild extends GeckoViewActorChild {
         }, 500);
         break;
       }
-      case "GeckoView:RestoreState": {
-        const { contentWindow, docShell } = this;
-        const { history, formdata, scrolldata, loadOptions } = message.data;
-
+      case "RestoreSessionState": {
+        this.restoreSessionState(message);
+        break;
+      }
+      case "RestoreHistoryAndNavigate": {
+        const { history, loadOptions } = message.data;
         if (history) {
-          const restoredHistory = SessionHistory.restore(docShell, history);
-
-          contentWindow.addEventListener(
-            "load",
-            _ => {
-              if (formdata) {
-                Utils.restoreFrameTreeData(
-                  contentWindow,
-                  formdata,
-                  (frame, data) => {
-                    // restore() will return false, and thus abort restoration for the
-                    // current |frame| and its descendants, if |data.url| is given but
-                    // doesn't match the loaded document's URL.
-                    return SessionStoreUtils.restoreFormData(
-                      frame.document,
-                      data
-                    );
-                  }
-                );
-              }
-            },
-            { capture: true, mozSystemGroup: true, once: true }
+          const restoredHistory = SessionHistory.restore(
+            this.docShell,
+            history
           );
-
-          const scrollRestore = _ => {
-            if (contentWindow.location != "about:blank") {
-              if (scrolldata) {
-                Utils.restoreFrameTreeData(
-                  contentWindow,
-                  scrolldata,
-                  (frame, data) => {
-                    if (data.scroll) {
-                      SessionStoreUtils.restoreScrollPosition(frame, data);
-                    }
-                  }
-                );
-              }
-              contentWindow.removeEventListener("pageshow", scrollRestore, {
-                capture: true,
-                mozSystemGroup: true,
-              });
-            }
-          };
-
-          contentWindow.addEventListener("pageshow", scrollRestore, {
-            capture: true,
-            mozSystemGroup: true,
-          });
-
-          const progressFilter = Cc[
-            "@mozilla.org/appshell/component/browser-status-filter;1"
-          ].createInstance(Ci.nsIWebProgress);
-          const flags = Ci.nsIWebProgress.NOTIFY_LOCATION;
-
-          const progressListener = {
-            QueryInterface: ChromeUtils.generateQI(["nsIWebProgressListener"]),
-
-            onLocationChange(aWebProgress, aRequest, aLocationURI, aFlags) {
-              debug`onLocationChange`;
-
-              if (
-                scrolldata &&
-                scrolldata.zoom &&
-                scrolldata.zoom.displaySize
-              ) {
-                const utils = contentWindow.windowUtils;
-                // Restore zoom level.
-                utils.setRestoreResolution(
-                  scrolldata.zoom.resolution,
-                  scrolldata.zoom.displaySize.width,
-                  scrolldata.zoom.displaySize.height
-                );
-              }
-
-              progressFilter.removeProgressListener(this);
-              const webProgress = docShell
-                .QueryInterface(Ci.nsIInterfaceRequestor)
-                .getInterface(Ci.nsIWebProgress);
-              webProgress.removeProgressListener(progressFilter);
-            },
-          };
-
-          progressFilter.addProgressListener(progressListener, flags);
-          const webProgress = docShell
-            .QueryInterface(Ci.nsIInterfaceRequestor)
-            .getInterface(Ci.nsIWebProgress);
-          webProgress.addProgressListener(progressFilter, flags);
-
           this.loadEntry(loadOptions, restoredHistory);
         }
         break;
@@ -329,6 +253,41 @@ class GeckoViewContentChild extends GeckoViewActorChild {
     return null;
   }
 
+  async restoreSessionState(message) {
+    // Make sure we showed something before restoring scrolling and form data
+    await this.pageShow;
+
+    const { contentWindow } = this;
+    const { formdata, scrolldata } = message.data;
+
+    if (formdata) {
+      Utils.restoreFrameTreeData(contentWindow, formdata, (frame, data) => {
+        // restore() will return false, and thus abort restoration for the
+        // current |frame| and its descendants, if |data.url| is given but
+        // doesn't match the loaded document's URL.
+        return SessionStoreUtils.restoreFormData(frame.document, data);
+      });
+    }
+
+    if (scrolldata) {
+      Utils.restoreFrameTreeData(contentWindow, scrolldata, (frame, data) => {
+        if (data.scroll) {
+          SessionStoreUtils.restoreScrollPosition(frame, data);
+        }
+      });
+    }
+
+    if (scrolldata && scrolldata.zoom && scrolldata.zoom.displaySize) {
+      const utils = contentWindow.windowUtils;
+      // Restore zoom level.
+      utils.setRestoreResolution(
+        scrolldata.zoom.resolution,
+        scrolldata.zoom.displaySize.width,
+        scrolldata.zoom.displaySize.height
+      );
+    }
+  }
+
   // eslint-disable-next-line complexity
   handleEvent(aEvent) {
     debug`handleEvent: ${aEvent.type}`;
@@ -338,6 +297,11 @@ class GeckoViewContentChild extends GeckoViewActorChild {
     }
 
     switch (aEvent.type) {
+      case "pageshow": {
+        this.receivedPageShow();
+        break;
+      }
+
       case "mozcaretstatechanged":
         if (
           aEvent.reason === "presscaret" ||

mobile/android/actors/GeckoViewContentParent.jsm

@@ -0,0 +1,60 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+"use strict";
+
+var EXPORTED_SYMBOLS = ["GeckoViewContentParent"];
+
+const { GeckoViewUtils } = ChromeUtils.import(
+  "resource://gre/modules/GeckoViewUtils.jsm"
+);
+
+const { debug, warn } = GeckoViewUtils.initLogging("GeckoViewContentParent");
+
+class GeckoViewContentParent extends JSWindowActorParent {
+  get browser() {
+    return this.browsingContext.top.embedderElement;
+  }
+
+  async collectState() {
+    return this.sendQuery("CollectSessionState");
+  }
+
+  restoreState({ history, loadOptions, formdata, scrolldata }) {
+    // Restoring is made of two parts. First we need to restore the history
+    // of the tab and navigating to the current page, after the page
+    // navigates to the current page we need to restore the state of the
+    // page (scroll position, form data, etc).
+    //
+    // We can't do everything in one step inside the child actor because
+    // the actor is recreated when navigating, so we need to keep the state
+    // on the parent side until we navigate.
+    this.sendAsyncMessage("RestoreHistoryAndNavigate", {
+      history,
+      loadOptions,
+    });
+
+    if (!formdata && !scrolldata) {
+      return;
+    }
+
+    const self = this;
+    const progressFilter = Cc[
+      "@mozilla.org/appshell/component/browser-status-filter;1"
+    ].createInstance(Ci.nsIWebProgress);
+
+    const progressListener = {
+      QueryInterface: ChromeUtils.generateQI(["nsIWebProgressListener"]),
+
+      onLocationChange(aWebProgress, aRequest, aLocationURI, aFlags) {
+        self.sendAsyncMessage("RestoreSessionState", { formdata, scrolldata });
+        progressFilter.removeProgressListener(this);
+        self.browser.removeProgressListener(progressFilter);
+      },
+    };
+
+    const flags = Ci.nsIWebProgress.NOTIFY_LOCATION;
+    progressFilter.addProgressListener(progressListener, flags);
+    this.browser.addProgressListener(progressFilter, flags);
+  }
+}

mobile/android/actors/moz.build

@@ -10,6 +10,7 @@ FINAL_TARGET_FILES.actors += [
     'ContentDelegateChild.jsm',
     'ContentDelegateParent.jsm',
     'GeckoViewContentChild.jsm',
+    'GeckoViewContentParent.jsm',
     'LoadURIDelegateChild.jsm',
     'WebBrowserChromeChild.jsm',
 ]

mobile/android/chrome/geckoview/geckoview.js

@@ -171,9 +171,7 @@ var ModuleManager = {
     // to collect it and restore it in the other process when switching.
     // TODO: This should go away when we migrate the history to the main
     // process Bug 1507287.
-    const { history } = await this.getActor("GeckoViewContent").sendQuery(
-      "CollectSessionState"
-    );
+    const { history } = await this.getActor("GeckoViewContent").collectState();
     // Ignore scroll and form data since we're navigating away from this page
     // anyway
     const sessionState = { history };
@@ -236,11 +234,7 @@ var ModuleManager = {
       module.enabled = true;
     });
 
-    this.getActor("GeckoViewContent").sendAsyncMessage(
-      "GeckoView:RestoreState",
-      sessionState
-    );
-
+    this.getActor("GeckoViewContent").restoreState(sessionState);
     this.browser.focus();
     return true;
   },
@@ -543,10 +537,14 @@ function startup() {
         resource: "resource://gre/modules/GeckoViewContent.jsm",
         actors: {
           GeckoViewContent: {
+            parent: {
+              moduleURI: "resource:///actors/GeckoViewContentParent.jsm",
+            },
             child: {
               moduleURI: "resource:///actors/GeckoViewContentChild.jsm",
               events: {
                 mozcaretstatechanged: { capture: true, mozSystemGroup: true },
+                pageshow: { mozSystemGroup: true },
               },
             },
             allFrames: true,

mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/ProgressDelegateTest.kt

@@ -293,7 +293,6 @@ class ProgressDelegateTest : BaseSessionTest() {
         waitForScroll(offset, timeout, "pageTop")
     }
 
-    @Ignore // Bug 1547849
     @WithDisplay(width = 400, height = 400)
     @Test fun saveAndRestoreState() {
         sessionRule.setPrefsUntilTestEnd(mapOf("dom.visualviewport.enabled" to true))

mobile/android/modules/geckoview/GeckoViewContent.jsm

@@ -155,8 +155,7 @@ class GeckoViewContent extends GeckoViewModule {
         }
         break;
       case "GeckoView:RestoreState":
-        // TODO: this needs parent process history to work properly
-        this.actor.sendAsyncMessage("GeckoView:RestoreState", aData);
+        this.actor.restoreState(aData);
         break;
     }
   }