Bug 1704028 - [devtools] Enable touch simulation in remote frame. r=o…

…chameau,devtools-backward-compat-reviewers. This patch removes the `setTouchEventsOverride` method on the targetConfigurationCommand, as we're now enabling the touch simulation from the server, in `BrowsingContextActor#updateTargetConfiguration`. A new configuration property is added, `reloadOnTouchSimulationToggle`, so the actor is responsible for reloading the page if the user set the pref. The `touchSimulator` property is moved from the responsive actor to the browsingContext one to facilitate managing the touch simulation state. Differential Revision: https://phabricator.services.mozilla.com/D116103
mozilla · Jun 3, 2021 · a5de5d2 · a5de5d2
1 parent aaeff5b
commit a5de5d2
Show file tree

Hide file tree

Showing 9 changed files with 101 additions and 163 deletions.
diff --git a/devtools/client/responsive/test/browser/browser_scroll.js b/devtools/client/responsive/test/browser/browser_scroll.js
@@ -18,13 +18,8 @@ addRDMTask(TEST_URL, async function({ ui, manager }) {
   const browser = ui.getViewportBrowser();
 
   for (const mv in [true, false]) {
-    const reloadNeeded = await ui.updateTouchSimulation(mv);
-    if (reloadNeeded) {
-      info("Reload is needed -- waiting for it.");
-      const reload = waitForViewportLoad(ui);
-      browser.reload();
-      await reload;
-    }
+    await ui.updateTouchSimulation(mv);
+
     info("Setting focus on the browser.");
     browser.focus();
 

diff --git a/devtools/client/responsive/test/browser/head.js b/devtools/client/responsive/test/browser/head.js
@@ -852,16 +852,13 @@ function rotateViewport(ui) {
 
 // Call this to switch between on/off support for meta viewports.
 async function setTouchAndMetaViewportSupport(ui, value) {
-  const reloadNeeded = await ui.updateTouchSimulation(value);
-  if (reloadNeeded) {
-    info("Reload is needed -- waiting for it.");
-    const reload = waitForViewportLoad(ui);
-    const browser = ui.getViewportBrowser();
-    browser.reload();
-    await reload;
-    await promiseContentReflow(ui);
-  }
-  return reloadNeeded;
+  await ui.updateTouchSimulation(value);
+  info("Reload so the new configuration applies cleanly to the page");
+  const reload = waitForViewportLoad(ui);
+  const browser = ui.getViewportBrowser();
+  browser.reload();
+  await reload;
+  await promiseContentReflow(ui);
 }
 
 // This function checks that zoom, layout viewport width and height

diff --git a/devtools/client/responsive/ui.js b/devtools/client/responsive/ui.js
@@ -307,9 +307,11 @@ class ResponsiveUI {
       await this.updateDPPX(null);
       reloadNeeded |=
         (await this.updateUserAgent()) && this.reloadOnChange("userAgent");
-      reloadNeeded |=
-        (await this.updateTouchSimulation()) &&
-        this.reloadOnChange("touchSimulation");
+
+      // Don't reload on the server if we're already doing a reload on the client
+      const reloadOnTouchSimulationChange =
+        this.reloadOnChange("touchSimulation") && !reloadNeeded;
+      await this.updateTouchSimulation(null, reloadOnTouchSimulationChange);
       if (reloadNeeded) {
         await this.reloadBrowser();
       }
@@ -489,9 +491,12 @@ class ResponsiveUI {
     reloadNeeded |=
       (await this.updateUserAgent(userAgent)) &&
       this.reloadOnChange("userAgent");
-    reloadNeeded |=
-      (await this.updateTouchSimulation(touch)) &&
-      this.reloadOnChange("touchSimulation");
+
+    // Don't reload on the server if we're already doing a reload on the client
+    const reloadOnTouchSimulationChange =
+      this.reloadOnChange("touchSimulation") && !reloadNeeded;
+    await this.updateTouchSimulation(touch, reloadOnTouchSimulationChange);
+
     if (reloadNeeded) {
       this.reloadBrowser();
     }
@@ -516,12 +521,11 @@ class ResponsiveUI {
 
     await this.updateMaxTouchPointsEnabled(enabled);
 
-    const reloadNeeded =
-      (await this.updateTouchSimulation(enabled)) &&
-      this.reloadOnChange("touchSimulation");
-    if (reloadNeeded) {
-      this.reloadBrowser();
-    }
+    await this.updateTouchSimulation(
+      enabled,
+      this.reloadOnChange("touchSimulation")
+    );
+
     // Used by tests
     this.emit("touch-simulation-changed");
   }
@@ -547,9 +551,11 @@ class ResponsiveUI {
     await this.updateDPPX(null);
     reloadNeeded |=
       (await this.updateUserAgent()) && this.reloadOnChange("userAgent");
-    reloadNeeded |=
-      (await this.updateTouchSimulation()) &&
-      this.reloadOnChange("touchSimulation");
+
+    // Don't reload on the server if we're already doing a reload on the client
+    const reloadOnTouchSimulationChange =
+      this.reloadOnChange("touchSimulation") && !reloadNeeded;
+    await this.updateTouchSimulation(null, reloadOnTouchSimulationChange);
     if (reloadNeeded) {
       this.reloadBrowser();
     }
@@ -790,12 +796,11 @@ class ResponsiveUI {
     await this.updateScreenOrientation(type, angle);
     await this.updateMaxTouchPointsEnabled(touchSimulationEnabled);
 
-    let reloadNeeded = false;
     if (touchSimulationEnabled) {
-      reloadNeeded |=
-        (await this.updateTouchSimulation(touchSimulationEnabled)) &&
-        this.reloadOnChange("touchSimulation");
+      await this.updateTouchSimulation(touchSimulationEnabled);
     }
+
+    let reloadNeeded = false;
     if (userAgent) {
       reloadNeeded |=
         (await this.updateUserAgent(userAgent)) &&
@@ -867,32 +872,31 @@ class ResponsiveUI {
    * false, this method will clear all touch simulation and meta viewport
    * overrides, returning to default behavior for both settings.
    *
-   * @return boolean
-   *         Whether a reload is needed to apply the override change(s).
+   * @param {boolean} enabled
+   * @param {boolean} reloadOnTouchSimulationToggle: Set to true to trigger a page reload
+   *        if the touch simulation state changes.
    */
-  async updateTouchSimulation(enabled) {
-    let reloadNeeded;
+  async updateTouchSimulation(enabled, reloadOnTouchSimulationToggle) {
+    // Call setMetaViewportOverride so the server would be in the expected state when/if
+    // the document reloads (as part of the call to updateConfiguration).
     if (enabled) {
-      reloadNeeded = await this.commands.targetConfigurationCommand.setTouchEventsOverride(
-        "enabled"
-      );
-
       const metaViewportEnabled = Services.prefs.getBoolPref(
         "devtools.responsive.metaViewport.enabled",
         false
       );
       if (metaViewportEnabled) {
-        reloadNeeded |= await this.responsiveFront.setMetaViewportOverride(
+        await this.responsiveFront.setMetaViewportOverride(
           Ci.nsIDocShell.META_VIEWPORT_OVERRIDE_ENABLED
         );
       }
     } else {
-      reloadNeeded = await this.commands.targetConfigurationCommand.setTouchEventsOverride(
-        null
-      );
-      reloadNeeded |= await this.responsiveFront.clearMetaViewportOverride();
+      await this.responsiveFront.clearMetaViewportOverride();
     }
-    return reloadNeeded;
+
+    await this.commands.targetConfigurationCommand.updateConfiguration({
+      touchEventsOverride: enabled ? "enabled" : null,
+      reloadOnTouchSimulationToggle,
+    });
   }
 
   /**

diff --git a/devtools/server/actors/emulation/responsive.js b/devtools/server/actors/emulation/responsive.js
@@ -9,13 +9,6 @@ const Services = require("Services");
 const protocol = require("devtools/shared/protocol");
 const { responsiveSpec } = require("devtools/shared/specs/responsive");
 
-loader.lazyRequireGetter(
-  this,
-  "TouchSimulator",
-  "devtools/server/actors/emulation/touch-simulator",
-  true
-);
-
 const FLOATING_SCROLLBARS_SHEET = Services.io.newURI(
   "chrome://devtools/skin/floating-scrollbars-responsive-design.css"
 );
@@ -46,14 +39,12 @@ const ResponsiveActor = protocol.ActorClassWithSpec(responsiveSpec, {
 
   destroy() {
     this.clearNetworkThrottling();
-    this.toggleTouchSimulator({ enable: false });
     this.clearMetaViewportOverride();
 
     this.targetActor.off("window-ready", this.onWindowReady);
 
     this.targetActor = null;
     this.docShell = null;
-    this._touchSimulator = null;
 
     protocol.Actor.prototype.destroy.call(this);
   },
@@ -75,16 +66,6 @@ const ResponsiveActor = protocol.ActorClassWithSpec(responsiveSpec, {
     return this.conn._getOrCreateActor(form.consoleActor);
   },
 
-  get touchSimulator() {
-    if (!this._touchSimulator) {
-      this._touchSimulator = new TouchSimulator(
-        this.targetActor.chromeEventHandler
-      );
-    }
-
-    return this._touchSimulator;
-  },
-
   get win() {
     return this.docShell.chromeEventHandler.ownerGlobal;
   },
@@ -197,33 +178,7 @@ const ResponsiveActor = protocol.ActorClassWithSpec(responsiveSpec, {
    * @param {String} pickerType
    */
   setElementPickerState(state, pickerType) {
-    this.touchSimulator.setElementPickerState(state, pickerType);
-  },
-
-  /**
-   * Start or stop the touch simulator depending on the parameter
-   *
-   * @param {Object} options
-   * @param {Boolean} options.enable: Pass true to start the touch simulator. Any other
-   *                  value will stop it. Defaults to false.
-   * @returns {Boolean} Whether or not any action was done on the touch simulator.
-   */
-  toggleTouchSimulator({ enable = false } = {}) {
-    if (enable) {
-      if (this.touchSimulator.enabled) {
-        return false;
-      }
-
-      this.touchSimulator.start();
-      return true;
-    }
-
-    if (!this.touchSimulator.enabled) {
-      return false;
-    }
-
-    this.touchSimulator.stop();
-    return true;
+    this.targetActor.touchSimulator.setElementPickerState(state, pickerType);
   },
 
   /* Meta viewport override */

diff --git a/devtools/server/actors/target-configuration.js b/devtools/server/actors/target-configuration.js
@@ -36,6 +36,8 @@ const SUPPORTED_OPTIONS = {
   rdmPaneMaxTouchPoints: true,
   // Page orientation (used in RDM and doesn't apply if RDM isn't enabled)
   rdmPaneOrientation: true,
+  // Reload the page when the touch simulation state changes (only works alongside touchEventsOverride)
+  reloadOnTouchSimulationToggle: true,
   // Restore focus in the page after closing DevTools.
   restoreFocus: true,
   // Enable service worker testing over HTTP (instead of HTTPS only).

diff --git a/devtools/server/actors/targets/browsing-context.js b/devtools/server/actors/targets/browsing-context.js
@@ -73,6 +73,13 @@ loader.lazyRequireGetter(
   true
 );
 
+loader.lazyRequireGetter(
+  this,
+  "TouchSimulator",
+  "devtools/server/actors/emulation/touch-simulator",
+  true
+);
+
 function getWindowID(window) {
   return window.windowGlobalChild.innerWindowId;
 }
@@ -600,6 +607,11 @@ const browsingContextTargetPrototype = {
       this.threadActor._parentClosed = true;
     }
 
+    if (this._touchSimulator) {
+      this._touchSimulator.stop();
+      this._touchSimulator = null;
+    }
+
     this._detach();
     this.docShell = null;
     this._extraActors = null;
@@ -1236,16 +1248,34 @@ const browsingContextTargetPrototype = {
       return;
     }
 
+    let reload = false;
+    if (typeof options.touchEventsOverride !== "undefined") {
+      const enableTouchSimulator = options.touchEventsOverride === "enabled";
+
+      // We want to reload the document if it's a top level target on which the touch
+      // simulator will be toggled and the user has turned the "reload on touch simulation"
+      // settings on.
+      if (
+        enableTouchSimulator !== this.touchSimulator.enabled &&
+        options.reloadOnTouchSimulationToggle === true &&
+        this.isTopLevelTarget
+      ) {
+        reload = true;
+      }
+
+      if (enableTouchSimulator) {
+        this.touchSimulator.start();
+      } else {
+        this.touchSimulator.stop();
+      }
+    }
+
     if (!this.isTopLevelTarget) {
-      // DevTools target options should only apply to the top target and be
+      // Following DevTools target options should only apply to the top target and be
       // propagated through the browsing context tree via the platform.
       return;
     }
 
-    // Wait a tick so that the response packet can be dispatched before the
-    // subsequent navigation event packet.
-    let reload = false;
-
     if (
       typeof options.javascriptEnabled !== "undefined" &&
       options.javascriptEnabled !== this._getJavascriptEnabled()
@@ -1274,6 +1304,14 @@ const browsingContextTargetPrototype = {
     }
   },
 
+  get touchSimulator() {
+    if (!this._touchSimulator) {
+      this._touchSimulator = new TouchSimulator(this.chromeEventHandler);
+    }
+
+    return this._touchSimulator;
+  },
+
   /**
    * Opposite of the updateTargetConfiguration method, that resets document
    * state when closing the toolbox.

diff --git a/devtools/shared/commands/target-configuration/target-configuration-command.js b/devtools/shared/commands/target-configuration/target-configuration-command.js
@@ -77,35 +77,6 @@ class TargetConfigurationCommand {
     return this._commands.targetCommand.targetFront._javascriptEnabled;
   }
 
-  /**
-   * Enable or disable touch events simulation
-   *
-   * @param {String|null} flag: The value to set for the touchEventsOverride flag.
-   *                      Pass null to reset the flag to its original value.
-   * @returns {Boolean} Returns true if the page needs to be reloaded (so the page can
-   *                    acknowledge the new state).
-   */
-  async setTouchEventsOverride(flag) {
-    // We need to set the flag on the parent process
-    await this.updateConfiguration({
-      touchEventsOverride: flag,
-    });
-
-    // And start the touch simulation within the content process.
-    // Note that this only handle current top-level document. When Fission is enabled, this
-    // doesn't enable touch simulation in remote iframes (See Bug 1704028).
-    // This also does not handle further navigation to a different origin (aka target switch),
-    // which should be fixed in Bug 1704029.
-    const responsiveFront = await this._commands.targetCommand.targetFront.getFront(
-      "responsive"
-    );
-    const reloadNeeded = await responsiveFront.toggleTouchSimulator({
-      enable: flag === "enabled",
-    });
-
-    return reloadNeeded;
-  }
-
   /**
    * Change orientation type and angle (that can be accessed through screen.orientation in
    * the content page) and simulates the "orientationchange" event when the device screen