From a5de5d21cf26040a0d66e43f542b0ded609f228f Mon Sep 17 00:00:00 2001 From: Nicolas Chevobbe <nchevobbe@mozilla.com> Date: Thu, 3 Jun 2021 11:28:00 +0000 Subject: [PATCH] Bug 1704028 - [devtools] Enable touch simulation in remote frame. r=ochameau,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 --- .../responsive/test/browser/browser_scroll.js | 9 +-- .../client/responsive/test/browser/head.js | 17 ++--- devtools/client/responsive/ui.js | 70 ++++++++++--------- .../server/actors/emulation/responsive.js | 47 +------------ .../server/actors/target-configuration.js | 2 + .../server/actors/targets/browsing-context.js | 48 +++++++++++-- .../target-configuration-command.js | 29 -------- ...rget_configuration_command_touch_events.js | 41 +++-------- devtools/shared/specs/target-configuration.js | 1 + 9 files changed, 101 insertions(+), 163 deletions(-) diff --git a/devtools/client/responsive/test/browser/browser_scroll.js b/devtools/client/responsive/test/browser/browser_scroll.js index 76f493d5d12cb..3cfce8d9dada7 100644 --- 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 index b2c105c08cffd..90f498b5f0036 100644 --- 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 index 7a489d3a3529c..da9af30d4f3d8 100644 --- 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 index 6b8db02942888..3ce4db89aed5d 100644 --- 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 index 08f4df3810edf..d2c442ba6eb08 100644 --- 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 index c1838c1cb6898..aad6f906da206 100644 --- 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 index bc6fffc8b0bc3..97ce765cfe058 100644 --- 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 diff --git a/devtools/shared/commands/target-configuration/tests/browser_target_configuration_command_touch_events.js b/devtools/shared/commands/target-configuration/tests/browser_target_configuration_command_touch_events.js index b329d5ea06d04..98d603e8b42fb 100644 --- a/devtools/shared/commands/target-configuration/tests/browser_target_configuration_command_touch_events.js +++ b/devtools/shared/commands/target-configuration/tests/browser_target_configuration_command_touch_events.js @@ -11,6 +11,9 @@ add_task(async function() { // Disable click hold and double tap zooming as it might interfere with the test await pushPref("ui.click_hold_context_menus", false); await pushPref("apz.allow_double_tap_zooming", false); + // We turn server-side target switching on so touch simulation is enabled when navigating + // to a different origin (See Bug 1704029). + await pushPref("devtools.target-switching.server.enabled", true); const tab = await addTab(TEST_URI); @@ -28,13 +31,12 @@ add_task(async function() { }); info("Enable touch simulation"); - await targetConfigurationCommand.setTouchEventsOverride("enabled"); + await targetConfigurationCommand.updateConfiguration({ + touchEventsOverride: "enabled", + }); await checkTopLevelDocumentTouchSimulation({ enabled: true }); await checkIframeTouchSimulation({ enabled: true, - // touch events are not emitted in remote frame when fission is enabled. - // This should be removed in Bug 1704028. - skipTouchEventsCheck: Services.appinfo.fissionAutostart, }); info("Reload the page"); @@ -59,9 +61,6 @@ add_task(async function() { ); await checkIframeTouchSimulation({ enabled: true, - // touch events are not emitted in remote frame when fission is enabled. - // This should be removed in Bug 1704028. - skipTouchEventsCheck: Services.appinfo.fissionAutostart, }); info( @@ -91,9 +90,6 @@ add_task(async function() { await checkTopLevelDocumentTouchSimulation({ enabled: true }); await checkIframeTouchSimulation({ enabled: true, - // touch events are not emitted in remote frame when fission is enabled. - // This should be removed in Bug 1704028. - skipTouchEventsCheck: Services.appinfo.fissionAutostart, }); const previousBrowsingContextId = gBrowser.selectedBrowser.browsingContext.id; @@ -124,9 +120,6 @@ add_task(async function() { ); await checkTopLevelDocumentTouchSimulation({ enabled: true, - // The touch simulator isn't working after a new browsing context is created. - // This should be removed in Bug 1704029. - skipTouchEventsCheck: true, }); is( @@ -136,10 +129,6 @@ add_task(async function() { ); await checkIframeTouchSimulation({ enabled: true, - // The touch simulator isn't working after a new browsing context is created, - // and touch events are not emitted in remote frame when fission is enabled. - // This can be removed once Bug 1704029 and Bug 1704028 are resolved. - skipTouchEventsCheck: true, }); info( @@ -234,10 +223,7 @@ async function isTouchEventEmitted(browserOrBrowsingContext) { return result !== "TIMEOUT"; } -async function checkTopLevelDocumentTouchSimulation({ - enabled, - skipTouchEventsCheck = false, -}) { +async function checkTopLevelDocumentTouchSimulation({ enabled }) { is( await matchesCoarsePointer(gBrowser.selectedBrowser), enabled, @@ -246,10 +232,6 @@ async function checkTopLevelDocumentTouchSimulation({ } on the top level document` ); - if (skipTouchEventsCheck) { - return; - } - is( await isTouchEventEmitted(gBrowser.selectedBrowser), enabled, @@ -269,10 +251,7 @@ function getIframeBrowsingContext() { ); } -async function checkIframeTouchSimulation({ - enabled, - skipTouchEventsCheck = false, -}) { +async function checkIframeTouchSimulation({ enabled }) { const iframeBC = await getIframeBrowsingContext(); is( await matchesCoarsePointer(iframeBC), @@ -280,10 +259,6 @@ async function checkIframeTouchSimulation({ `The touch simulation is ${enabled ? "enabled" : "disabled"} on the iframe` ); - if (skipTouchEventsCheck) { - return; - } - is( await isTouchEventEmitted(iframeBC), enabled, diff --git a/devtools/shared/specs/target-configuration.js b/devtools/shared/specs/target-configuration.js index 0fe3b6807eefb..d812f52dcebca 100644 --- a/devtools/shared/specs/target-configuration.js +++ b/devtools/shared/specs/target-configuration.js @@ -20,6 +20,7 @@ types.addDictType("target-configuration.configuration", { paintFlashing: "nullable:boolean", printSimulationEnabled: "nullable:boolean", rdmPaneOrientation: "nullable:json", + reloadOnTouchSimulationToggle: "nullable:boolean", restoreFocus: "nullable:boolean", serviceWorkersTestingEnabled: "nullable:boolean", touchEventsOverride: "nullable:string",