Bug 1381463 - Reload to clear UA when exiting RDM. r=ochameau

When exiting RDM with a device applied, we clear the UA override, but we did not
force a refresh of the page.  This means that the customized UA was still
accessible via `navigator.userAgent` and the page might have a non-default
layout if it tests the UA value.

To ensure there's no chance of confusion after exiting RDM, force a reload if
clearing emulation values determines it is needed.

MozReview-Commit-ID: Fkji12Utmis

--HG--
extra : rebase_source : a7ae8851165fae7aec5875d10f51cbb703b3f870

devtools/client/responsive.html/manager.js

@@ -373,7 +373,7 @@ ResponsiveUI.prototype = {
    *         Whether this call is actually destroying.  False means destruction
    *         was already in progress.
    */
-  destroy: Task.async(function* (options) {
+  async destroy(options) {
     if (this.destroying) {
       return false;
     }
@@ -389,7 +389,7 @@ ResponsiveUI.prototype = {
 
     // Ensure init has finished before starting destroy
     if (!isTabContentDestroying) {
-      yield this.inited;
+      await this.inited;
     }
 
     this.tab.removeEventListener("TabClose", this);
@@ -399,7 +399,20 @@ ResponsiveUI.prototype = {
 
     if (!isTabContentDestroying) {
       // Notify the inner browser to stop the frame script
-      yield message.request(this.toolWindow, "stop-frame-script");
+      await message.request(this.toolWindow, "stop-frame-script");
+    }
+
+    // Ensure the tab is reloaded if required when exiting RDM so that no emulated
+    // settings are left in a customized state.
+    if (!isTabContentDestroying) {
+      let reloadNeeded = false;
+      reloadNeeded |= await this.updateDPPX();
+      reloadNeeded |= await this.updateNetworkThrottling();
+      reloadNeeded |= await this.updateUserAgent();
+      reloadNeeded |= await this.updateTouchSimulation();
+      if (reloadNeeded) {
+        this.getViewportBrowser().reload();
+      }
     }
 
     // Destroy local state
@@ -415,7 +428,7 @@ ResponsiveUI.prototype = {
     // anything on shutdown client side.
     let clientClosed = this.client.close();
     if (!isTabContentDestroying) {
-      yield clientClosed;
+      await clientClosed;
     }
     this.client = this.emulationFront = null;
 
@@ -427,7 +440,7 @@ ResponsiveUI.prototype = {
     this.destroyed = true;
 
     return true;
-  }),
+  },
 
   connectToServer: Task.async(function* () {
     DebuggerServer.init();
@@ -487,9 +500,13 @@ ResponsiveUI.prototype = {
 
   onChangeDevice: Task.async(function* (event) {
     let { userAgent, pixelRatio, touch } = event.data.device;
+    // Bug 1428799: Should we reload on UA change as well?
     yield this.updateUserAgent(userAgent);
     yield this.updateDPPX(pixelRatio);
-    yield this.updateTouchSimulation(touch);
+    let reloadNeeded = yield this.updateTouchSimulation(touch);
+    if (reloadNeeded) {
+      this.getViewportBrowser().reload();
+    }
     // Used by tests
     this.emit("device-changed");
   }),
@@ -506,9 +523,12 @@ ResponsiveUI.prototype = {
     this.updateDPPX(pixelRatio);
   },
 
-  onChangeTouchSimulation(event) {
+  async onChangeTouchSimulation(event) {
     let { enabled } = event.data;
-    this.updateTouchSimulation(enabled);
+    let reloadNeeded = await this.updateTouchSimulation(enabled);
+    if (reloadNeeded) {
+      this.getViewportBrowser().reload();
+    }
     // Used by tests
     this.emit("touch-simulation-changed");
   },
@@ -527,25 +547,44 @@ ResponsiveUI.prototype = {
   },
 
   onRemoveDeviceAssociation: Task.async(function* (event) {
+    // Bug 1428799: Should we reload on UA change as well?
     yield this.updateUserAgent();
     yield this.updateDPPX();
-    yield this.updateTouchSimulation();
+    let reloadNeeded = yield this.updateTouchSimulation();
+    if (reloadNeeded) {
+      this.getViewportBrowser().reload();
+    }
     // Used by tests
     this.emit("device-association-removed");
   }),
 
+  /**
+   * Set or clear the emulated device pixel ratio.
+   *
+   * @return boolean
+   *         Whether a reload is needed to apply the change.
+   *         (This is always immediate, so it's always false.)
+   */
   updateDPPX: Task.async(function* (dppx) {
     if (!dppx) {
       yield this.emulationFront.clearDPPXOverride();
-      return;
+      return false;
     }
     yield this.emulationFront.setDPPXOverride(dppx);
+    return false;
   }),
 
+  /**
+   * Set or clear network throttling.
+   *
+   * @return boolean
+   *         Whether a reload is needed to apply the change.
+   *         (This is always immediate, so it's always false.)
+   */
   updateNetworkThrottling: Task.async(function* (enabled, profile) {
     if (!enabled) {
       yield this.emulationFront.clearNetworkThrottling();
-      return;
+      return false;
     }
     let data = throttlingProfiles.find(({ id }) => id == profile);
     let { download, upload, latency } = data;
@@ -554,29 +593,36 @@ ResponsiveUI.prototype = {
       uploadThroughput: upload,
       latency,
     });
+    return false;
   }),
 
-  updateUserAgent: Task.async(function* (userAgent) {
+  /**
+   * Set or clear the emulated user agent.
+   *
+   * @return boolean
+   *         Whether a reload is needed to apply the change.
+   */
+  updateUserAgent(userAgent) {
     if (!userAgent) {
-      yield this.emulationFront.clearUserAgentOverride();
-      return;
+      return this.emulationFront.clearUserAgentOverride();
     }
-    yield this.emulationFront.setUserAgentOverride(userAgent);
-  }),
+    return this.emulationFront.setUserAgentOverride(userAgent);
+  },
 
-  updateTouchSimulation: Task.async(function* (enabled) {
-    let reloadNeeded;
-    if (enabled) {
-      reloadNeeded = yield this.emulationFront.setTouchEventsOverride(
-        Ci.nsIDocShell.TOUCHEVENTS_OVERRIDE_ENABLED
-      );
-    } else {
-      reloadNeeded = yield this.emulationFront.clearTouchEventsOverride();
+  /**
+   * Set or clear touch simulation.
+   *
+   * @return boolean
+   *         Whether a reload is needed to apply the change.
+   */
+  updateTouchSimulation(enabled) {
+    if (!enabled) {
+      return this.emulationFront.clearTouchEventsOverride();
     }
-    if (reloadNeeded) {
-      this.getViewportBrowser().reload();
-    }
-  }),
+    return this.emulationFront.setTouchEventsOverride(
+      Ci.nsIDocShell.TOUCHEVENTS_OVERRIDE_ENABLED
+    );
+  },
 
   /**
    * Helper for tests. Assumes a single viewport for now.

devtools/client/responsive.html/test/browser/browser_device_change.js

@@ -3,8 +3,8 @@ http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
-// Tests changing viewport device
-const TEST_URL = "data:text/html;charset=utf-8,Device list test";
+// Tests changing viewport device (need HTTP load for proper UA testing)
+const TEST_URL = `${URL_ROOT}doc_page_state.html`;
 
 const DEFAULT_DPPX = window.devicePixelRatio;
 const DEFAULT_UA = Cc["@mozilla.org/network/protocol;1?name=http"]
@@ -28,7 +28,7 @@ const testDevice = {
 // Add the new device to the list
 addDeviceForTest(testDevice);
 
-addRDMTask(TEST_URL, function* ({ ui, manager }) {
+addRDMTask(TEST_URL, function* ({ ui }) {
   let { store } = ui.toolWindow;
 
   // Wait until the viewport has been added and the device list has been loaded
@@ -37,23 +37,29 @@ addRDMTask(TEST_URL, function* ({ ui, manager }) {
 
   // Test defaults
   testViewportDimensions(ui, 320, 480);
+  info("Should have default UA at the start of the test");
   yield testUserAgent(ui, DEFAULT_UA);
   yield testDevicePixelRatio(ui, DEFAULT_DPPX);
   yield testTouchEventsOverride(ui, false);
   testViewportDeviceSelectLabel(ui, "no device selected");
 
   // Test device with custom properties
+  let reloaded = waitForViewportLoad(ui);
   yield selectDevice(ui, "Fake Phone RDM Test");
+  yield reloaded;
   yield waitForViewportResizeTo(ui, testDevice.width, testDevice.height);
+  info("Should have device UA now that device is applied");
   yield testUserAgent(ui, testDevice.userAgent);
   yield testDevicePixelRatio(ui, testDevice.pixelRatio);
   yield testTouchEventsOverride(ui, true);
 
   // Test resetting device when resizing viewport
   let deviceRemoved = once(ui, "device-association-removed");
+  reloaded = waitForViewportLoad(ui);
   yield testViewportResize(ui, ".viewport-vertical-resize-handle",
     [-10, -10], [testDevice.width, testDevice.height - 10], [0, -10], ui);
-  yield deviceRemoved;
+  yield Promise.all([ deviceRemoved, reloaded ]);
+  info("Should have default UA after resizing viewport");
   yield testUserAgent(ui, DEFAULT_UA);
   yield testDevicePixelRatio(ui, DEFAULT_DPPX);
   yield testTouchEventsOverride(ui, false);
@@ -62,11 +68,42 @@ addRDMTask(TEST_URL, function* ({ ui, manager }) {
   // Test device with generic properties
   yield selectDevice(ui, "Laptop (1366 x 768)");
   yield waitForViewportResizeTo(ui, 1366, 768);
+  info("Should have default UA when using device without specific UA");
   yield testUserAgent(ui, DEFAULT_UA);
   yield testDevicePixelRatio(ui, 1);
   yield testTouchEventsOverride(ui, false);
 });
 
+add_task(async function () {
+  const tab = await addTab(TEST_URL);
+  const { ui } = await openRDM(tab);
+
+  let { store } = ui.toolWindow;
+
+  // Wait until the viewport has been added and the device list has been loaded
+  await waitUntilState(store, state => state.viewports.length == 1
+    && state.devices.listState == Types.deviceListState.LOADED);
+
+  // Select device with custom UA
+  let reloaded = waitForViewportLoad(ui);
+  await selectDevice(ui, "Fake Phone RDM Test");
+  await reloaded;
+  await waitForViewportResizeTo(ui, testDevice.width, testDevice.height);
+  info("Should have device UA now that device is applied");
+  await testUserAgent(ui, testDevice.userAgent);
+
+  // Browser will reload to clear the UA on RDM close
+  reloaded = waitForViewportLoad(ui);
+  await closeRDM(tab);
+  await reloaded;
+
+  // Ensure UA is reset to default after closing RDM
+  info("Should have default UA after closing RDM");
+  await testUserAgentFromBrowser(tab.linkedBrowser, DEFAULT_UA);
+
+  await removeTab(tab);
+});
+
 function testViewportDimensions(ui, w, h) {
   let viewport = ui.toolWindow.document.querySelector(".viewport-content");
 

devtools/client/responsive.html/test/browser/head.js

@@ -301,12 +301,7 @@ function waitForPageShow(browser) {
 }
 
 function waitForViewportLoad(ui) {
-  return new Promise(resolve => {
-    let browser = ui.getViewportBrowser();
-    browser.addEventListener("mozbrowserloadend", () => {
-      resolve();
-    }, { once: true });
-  });
+  return BrowserTestUtils.waitForContentEvent(ui.getViewportBrowser(), "load", true);
 }
 
 function load(browser, url) {
@@ -371,8 +366,12 @@ function* toggleTouchSimulation(ui) {
   yield Promise.all([ changed, loaded ]);
 }
 
-function* testUserAgent(ui, expected) {
-  let ua = yield ContentTask.spawn(ui.getViewportBrowser(), {}, function* () {
+function testUserAgent(ui, expected) {
+  testUserAgentFromBrowser(ui.getViewportBrowser(), expected);
+}
+
+async function testUserAgentFromBrowser(browser, expected) {
+  let ua = await ContentTask.spawn(browser, {}, function* () {
     return content.navigator.userAgent;
   });
   is(ua, expected, `UA should be set to ${expected}`);