Allow device mode to be toggled for chrome:// pages

... but do not actually enable device mode for chrome:// pages.

This restores the user flow of enabling device emulation on NTP
before navigating to the site that the user wants to test.

Fixed: 388411685, 388411685
Change-Id: If9b883b7337fe321fccd52a350bd939479c85146
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6289404
Auto-Submit: Yang Guo <[email protected]>
Commit-Queue: Yang Guo <[email protected]>
Reviewed-by: Philip Pfaffe <[email protected]>

Author: hashseed

front_end/panels/emulation/DeviceModeWrapper.ts

@@ -19,13 +19,11 @@ export class DeviceModeWrapper extends UI.Widget.VBox {
   private deviceModeView: DeviceModeView|null;
   private readonly toggleDeviceModeAction: UI.ActionRegistration.Action;
   private showDeviceModeSetting: Common.Settings.Setting<boolean>;
-  private enableOncePossible: boolean;
 
   private constructor(inspectedPagePlaceholder: InspectedPagePlaceholder) {
     super();
     this.inspectedPagePlaceholder = inspectedPagePlaceholder;
     this.deviceModeView = null;
-    this.enableOncePossible = false;
     this.toggleDeviceModeAction = UI.ActionRegistry.ActionRegistry.instance().getAction('emulation.toggle-device-mode');
     const model = EmulationModel.DeviceModeModel.DeviceModeModel.instance();
     this.showDeviceModeSetting = model.enabledSetting();
@@ -35,7 +33,7 @@ export class DeviceModeWrapper extends UI.Widget.VBox {
         SDK.OverlayModel.OverlayModel, SDK.OverlayModel.Events.SCREENSHOT_REQUESTED,
         this.screenshotRequestedFromOverlay, this);
     SDK.TargetManager.TargetManager.instance().addEventListener(
-        SDK.TargetManager.Events.INSPECTED_URL_CHANGED, this.inspectedUrlChanged, this);
+        SDK.TargetManager.Events.INSPECTED_URL_CHANGED, () => this.update(), this);
     this.update(true);
   }
 
@@ -56,26 +54,6 @@ export class DeviceModeWrapper extends UI.Widget.VBox {
     return deviceModeWrapperInstance;
   }
 
-  inspectedUrlChanged(event: {data: SDK.Target.Target}): void {
-    const url = event.data.inspectedURL();
-    // Only allow device mode for non chrome:// pages.
-    const canEnable = url !== null && !Common.ParsedURL.schemeIs(url, 'chrome:');
-    this.toggleDeviceModeAction.setEnabled(canEnable);
-    if (!canEnable && this.isDeviceModeOn()) {
-      // Device mode is enabled, but we navigated to a chrome:// page.
-      // Remember to enable device mode again when next possible.
-      this.toggleDeviceMode();
-      this.enableOncePossible = true;
-    }
-    if (canEnable && !this.isDeviceModeOn() && this.enableOncePossible) {
-      // Device mode is not enabled, could be enabled, and we have a reminder to enable.
-      this.toggleDeviceMode();
-      this.enableOncePossible = false;
-    }
-
-    this.update();
-  }
-
   toggleDeviceMode(): void {
     this.showDeviceModeSetting.set(!this.showDeviceModeSetting.get());
   }
@@ -106,14 +84,20 @@ export class DeviceModeWrapper extends UI.Widget.VBox {
 
   update(force?: boolean): void {
     this.toggleDeviceModeAction.setToggled(this.showDeviceModeSetting.get());
-    if (!force) {
-      const showing = this.deviceModeView?.isShowing();
-      if (this.showDeviceModeSetting.get() === showing) {
-        return;
-      }
+
+    // Only allow device mode for non chrome:// pages.
+    function allowDeviceMode(): boolean {
+      const target = SDK.TargetManager.TargetManager.instance().primaryPageTarget();
+      const url = target?.inspectedURL();
+      return url ? !Common.ParsedURL.schemeIs(url, 'chrome:') : false;
+    }
+
+    const shouldShow = this.showDeviceModeSetting.get() && allowDeviceMode();
+    if (!force && shouldShow === this.deviceModeView?.isShowing()) {
+      return;
     }
 
-    if (this.showDeviceModeSetting.get()) {
+    if (shouldShow) {
       if (!this.deviceModeView) {
         this.deviceModeView = new DeviceModeView();
       }

test/e2e/emulation/disable-device-mode_test.ts

@@ -9,9 +9,8 @@ import {
 } from '../../shared/helper.js';
 import {
   clickDeviceModeToggler,
-  deviceModeButtonCanEnable,
   deviceModeIsEnabled,
-  openDeviceToolbar,
+  deviceModeIsToggled,
   reloadDockableFrontEnd,
 } from '../helpers/emulation-helpers.js';
 
@@ -22,50 +21,25 @@ describe('Disable device mode for chrome:// pages', () => {
   });
 
   it('chrome:// pages disable device mode', async () => {
-    // Button is untoggled and enabled initially.
-    assert.isFalse(await deviceModeIsEnabled());
-    assert.isTrue(await deviceModeButtonCanEnable());
-    // Button is untoggled and enabled for about://blank.
-    await goTo('about://blank');
-    assert.isFalse(await deviceModeIsEnabled());
-    assert.isTrue(await deviceModeButtonCanEnable());
-    // Button is untoggled and enabled for chrome://version.
+    // Button is untoggled and device mode is off for chrome://version.
     await goTo('chrome://version');
+    assert.isFalse(await deviceModeIsToggled());
     assert.isFalse(await deviceModeIsEnabled());
-    assert.isFalse(await deviceModeButtonCanEnable());
-    // Clicking on the button does not enable device mode.
+    // Clicking on the button toggles the button but does not enable device mode.
     await clickDeviceModeToggler();
+    assert.isTrue(await deviceModeIsToggled());
     assert.isFalse(await deviceModeIsEnabled());
-    assert.isFalse(await deviceModeButtonCanEnable());
-    // Button is untoggled and enabled for about://blank.
-    await goTo('about://blank');
-    assert.isFalse(await deviceModeIsEnabled());
-    assert.isTrue(await deviceModeButtonCanEnable());
-    // Clicking on the button enables device mode.
-    await openDeviceToolbar();
-    assert.isTrue(await deviceModeIsEnabled());
-    assert.isTrue(await deviceModeButtonCanEnable());
-  });
-
-  it('device mode turns back on automatically', async () => {
-    // Button is untoggled and enabled initially.
-    assert.isFalse(await deviceModeIsEnabled());
-    assert.isTrue(await deviceModeButtonCanEnable());
-    // Button is untoggled and enabled for about://blank.
+    // Button is is toggled and device mode is on for about://blank.
     await goTo('about://blank');
-    assert.isFalse(await deviceModeIsEnabled());
-    assert.isTrue(await deviceModeButtonCanEnable());
-    // Clicking on the button enables device mode.
-    await openDeviceToolbar();
+    assert.isTrue(await deviceModeIsToggled());
     assert.isTrue(await deviceModeIsEnabled());
-    assert.isTrue(await deviceModeButtonCanEnable());
-    // Navigating to chrome://version disables device mode.
+    // Navigating back to chrome://version turns device mode off but leaves the button toggled.
     await goTo('chrome://version');
+    assert.isTrue(await deviceModeIsToggled());
+    assert.isFalse(await deviceModeIsEnabled());
+    // Clicking on the button untoggles the button and disables device mode.
+    await clickDeviceModeToggler();
+    assert.isFalse(await deviceModeIsToggled());
     assert.isFalse(await deviceModeIsEnabled());
-    assert.isFalse(await deviceModeButtonCanEnable());
-    // Navigating back to about://blank re-enables device mode.
-    await goTo('about://blank');
-    assert.isTrue(await deviceModeIsEnabled());
-    assert.isTrue(await deviceModeButtonCanEnable());
   });
 });

test/e2e/helpers/emulation-helpers.ts

@@ -36,7 +36,7 @@ export const reloadDockableFrontEnd = async () => {
   await reloadDevTools({canDock: true});
 };
 
-export const deviceModeIsEnabled = async () => {
+export const deviceModeIsToggled = async () => {
   const deviceToolbarToggler = await waitFor(DEVICE_TOOLBAR_TOGGLER_SELECTOR);
   const pressed = await deviceToolbarToggler.evaluate(element => {
     const button = element.shadowRoot?.querySelector('.primary-toggle') as HTMLButtonElement;
@@ -45,6 +45,13 @@ export const deviceModeIsEnabled = async () => {
   return pressed === 'true';
 };
 
+export const deviceModeIsEnabled = async () => {
+  // Check the userAgent string to see whether emulation is really enabled.
+  const {target} = getBrowserAndPages();
+  const userAgent = await target.evaluate(() => navigator.userAgent);
+  return userAgent.includes('Mobile');
+};
+
 export const clickDeviceModeToggler = async () => {
   const deviceToolbarToggler = await waitFor(DEVICE_TOOLBAR_TOGGLER_SELECTOR);
   await clickElement(deviceToolbarToggler);
@@ -58,14 +65,6 @@ export const openDeviceToolbar = async () => {
   await waitFor(DEVICE_TOOLBAR_SELECTOR);
 };
 
-export const deviceModeButtonCanEnable = async () => {
-  const deviceToolbarToggler = await waitFor(DEVICE_TOOLBAR_TOGGLER_SELECTOR);
-  return await deviceToolbarToggler.evaluate(element => {
-    const button = element.shadowRoot?.querySelector('.primary-toggle') as HTMLButtonElement;
-    return !button.disabled;
-  });
-};
-
 export const showMediaQueryInspector = async () => {
   const inspector = await $(MEDIA_QUERY_INSPECTOR_SELECTOR);
   if (inspector) {