[go: nahoru, domu]

Allow live locations even if the debugger is disabled

Previously we relied on the debuggerModel to create a raw location,
which is used for creating live locations.

This changes raw locations (DebuggerWorkspaceBinding.Location)
to rely on scriptIds instead of scripts. As a result we can create a
live location even if we currently do not have a script.

As soon as the debugger is enabled and a script is parsed, the
live locations will be updated with the relevant information.

We use this change in the Linkifier, which now shows no link if
the debugger is disabled (e.g. if we profile), and updates the
location and shows the link as soon as the debugger is enabled.

Also-by: bmeurer@chromium.org
Also-by: pfaffe@chromium.org
Bug: chromium:1132260

Change-Id: I48dc6016099d5b82d702a4331f93301693a6a6f9
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2595386
Commit-Queue: Kim-Anh Tran <kimanh@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
diff --git a/front_end/bindings/DebuggerWorkspaceBinding.js b/front_end/bindings/DebuggerWorkspaceBinding.js
index 7b6b5f1..cd8697b 100644
--- a/front_end/bindings/DebuggerWorkspaceBinding.js
+++ b/front_end/bindings/DebuggerWorkspaceBinding.js
@@ -193,11 +193,7 @@
    * @return {!Promise<?Location>}
    */
   async createLiveLocation(rawLocation, updateDelegate, locationPool) {
-    const script = rawLocation.script();
-    if (!script) {
-      return null;
-    }
-    const modelData = this._debuggerModelToData.get(script.debuggerModel);
+    const modelData = this._debuggerModelToData.get(rawLocation.debuggerModel);
     if (!modelData) {
       return null;
     }
@@ -429,7 +425,7 @@
    * @param {!Location} location
    */
   _removeLiveLocation(location) {
-    const modelData = this._debuggerModelToData.get(location._script.debuggerModel);
+    const modelData = this._debuggerModelToData.get(location._rawLocation.debuggerModel);
     if (modelData) {
       modelData._disposeLocation(location);
     }
@@ -461,7 +457,7 @@
     this._resourceMapping = new ResourceScriptMapping(debuggerModel, workspace, debuggerWorkspaceBinding);
     this._compilerMapping = new CompilerScriptMapping(debuggerModel, workspace, debuggerWorkspaceBinding);
 
-    /** @type {!Platform.Multimap<!SDK.Script.Script, !Location>} */
+    /** @type {!Platform.Multimap<string, !Location>} */
     this._locations = new Platform.Multimap();
 
     debuggerModel.setBeforePausedCallback(this._beforePaused.bind(this));
@@ -474,10 +470,10 @@
    * @return {!Promise<!Location>}
    */
   async _createLiveLocation(rawLocation, updateDelegate, locationPool) {
-    console.assert(rawLocation.script() !== null);
-    const script = /** @type {!SDK.Script.Script} */ (rawLocation.script());
-    const location = new Location(script, rawLocation, this._debuggerWorkspaceBinding, updateDelegate, locationPool);
-    this._locations.set(script, location);
+    console.assert(rawLocation.scriptId !== '');
+    const scriptId = rawLocation.scriptId;
+    const location = new Location(scriptId, rawLocation, this._debuggerWorkspaceBinding, updateDelegate, locationPool);
+    this._locations.set(scriptId, location);
     await location.update();
     return location;
   }
@@ -486,7 +482,7 @@
    * @param {!Location} location
    */
   _disposeLocation(location) {
-    this._locations.delete(location._script, location);
+    this._locations.delete(location._scriptId, location);
   }
 
   /**
@@ -494,7 +490,7 @@
    */
   async _updateLocations(script) {
     const promises = [];
-    for (const location of this._locations.get(script)) {
+    for (const location of this._locations.get(script.scriptId)) {
       promises.push(location.update());
     }
     await Promise.all(promises);
@@ -558,15 +554,15 @@
 
 export class Location extends LiveLocationWithPool {
   /**
-   * @param {!SDK.Script.Script} script
+   * @param {string} scriptId
    * @param {!SDK.DebuggerModel.Location} rawLocation
    * @param {!DebuggerWorkspaceBinding} binding
    * @param {function(!LiveLocation): !Promise<?>} updateDelegate
    * @param {!LiveLocationPool} locationPool
    */
-  constructor(script, rawLocation, binding, updateDelegate, locationPool) {
+  constructor(scriptId, rawLocation, binding, updateDelegate, locationPool) {
     super(updateDelegate, locationPool);
-    this._script = script;
+    this._scriptId = scriptId;
     this._rawLocation = rawLocation;
     this._binding = binding;
   }
diff --git a/front_end/components/JSPresentationUtils.js b/front_end/components/JSPresentationUtils.js
index 99bfa74..9ede49d 100644
--- a/front_end/components/JSPresentationUtils.js
+++ b/front_end/components/JSPresentationUtils.js
@@ -54,6 +54,10 @@
   *@example {2} PH1
   */
   showSMoreFrames: 'Show {PH1} more frames',
+  /**
+   *@description Text indicating that source url of a link is currently unknown
+   */
+  unknownSource: 'unknown',
 };
 const str_ = i18n.i18n.registerUIStrings('components/JSPresentationUtils.js', UIStrings);
 const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
@@ -104,6 +108,11 @@
         }
         row.createChild('td').textContent = ' @ ';
         row.createChild('td').appendChild(link);
+        // Linkifier is using a workaround with the 'zero width space' (\u200b).
+        // TODO(szuend): Remove once the Linkfier is no longer using the workaround.
+        if (!link.textContent || link.textContent === '\u200b') {
+          link.textContent = i18nString(UIStrings.unknownSource);
+        }
         links.push(link);
       }
       if (shouldHide) {
diff --git a/front_end/components/Linkifier.js b/front_end/components/Linkifier.js
index b9c4812..2f5d5b6 100644
--- a/front_end/components/Linkifier.js
+++ b/front_end/components/Linkifier.js
@@ -281,17 +281,19 @@
     // Not initialising the anchor element with 'zero width space' (\u200b) causes a crash
     // in the layout engine.
     // TODO(szuend): Remove comment and workaround once the crash is fixed.
-    const anchor = /** @type {!HTMLElement} */ (Linkifier._createLink('\u200b', className, createLinkOptions));
+    const anchor = Linkifier._createLink(
+        fallbackAnchor && fallbackAnchor.textContent ? fallbackAnchor.textContent : '\u200b', className,
+        createLinkOptions);
     const info = Linkifier.linkInfo(anchor);
     if (!info) {
-      return null;
+      return fallbackAnchor;
     }
     info.enableDecorator = this._useLinkDecorator;
     info.fallback = fallbackAnchor;
 
     const pool = this._locationPoolByTarget.get(rawLocation.debuggerModel.target());
     if (!pool) {
-      return null;
+      return fallbackAnchor;
     }
     Bindings.DebuggerWorkspaceBinding.DebuggerWorkspaceBinding.instance()
         .createLiveLocation(rawLocation, this._updateAnchor.bind(this, anchor), pool)
@@ -395,7 +397,7 @@
     // Not initialising the anchor element with 'zero width space' (\u200b) causes a crash
     // in the layout engine.
     // TODO(szuend): Remove comment and workaround once the crash is fixed.
-    const anchor = /** @type {!HTMLElement} */ (Linkifier._createLink('\u200b', classes || ''));
+    const anchor = Linkifier._createLink('\u200b', classes || '');
     const info = Linkifier.linkInfo(anchor);
     if (!info) {
       return fallbackAnchor;
diff --git a/front_end/i18n/locales/en-US.json b/front_end/i18n/locales/en-US.json
index 3a67b45..196e410 100644
--- a/front_end/i18n/locales/en-US.json
+++ b/front_end/i18n/locales/en-US.json
@@ -119,6 +119,9 @@
   "components/JSPresentationUtils.js | showSMoreFrames": {
     "message": "Show {PH1} more frames"
   },
+  "components/JSPresentationUtils.js | unknownSource": {
+    "message": "unknown"
+  },
   "components/Linkifier.js | auto": {
     "message": "auto"
   },
diff --git a/front_end/sdk/DebuggerModel.js b/front_end/sdk/DebuggerModel.js
index f7a18bc..731e981 100644
--- a/front_end/sdk/DebuggerModel.js
+++ b/front_end/sdk/DebuggerModel.js
@@ -901,7 +901,7 @@
    * @return {!Location}
    */
   createRawLocation(script, lineNumber, columnNumber) {
-    return new Location(this, script.scriptId, lineNumber, columnNumber);
+    return this.createRawLocationByScriptId(script.scriptId, lineNumber, columnNumber);
   }
 
   /**
@@ -934,11 +934,10 @@
    * @param {!Protocol.Runtime.ScriptId} scriptId
    * @param {number} lineNumber
    * @param {number} columnNumber
-   * @return {?Location}
+   * @return {!Location}
    */
   createRawLocationByScriptId(scriptId, lineNumber, columnNumber) {
-    const script = this.scriptForId(scriptId);
-    return script ? this.createRawLocation(script, lineNumber, columnNumber) : null;
+    return new Location(this, scriptId, lineNumber, columnNumber);
   }
 
   /**
diff --git a/front_end/timeline/TimelineUIUtils.js b/front_end/timeline/TimelineUIUtils.js
index 3b0c7d3..666dac0 100644
--- a/front_end/timeline/TimelineUIUtils.js
+++ b/front_end/timeline/TimelineUIUtils.js
@@ -2380,6 +2380,11 @@
       if (linkifier) {
         const link = linkifier.maybeLinkifyConsoleCallFrame(target, topFrame);
         if (link) {
+          // Linkifier is using a workaround with the 'zero width space' (\u200b).
+          // TODO(szuend): Remove once the Linkifier is no longer using the workaround.
+          if (!link.textContent || link.textContent === '\u200b') {
+            link.textContent = ls`unknown`;
+          }
           stack.createChild('span').textContent = ' @ ';
           stack.createChild('span').appendChild(link);
         }
diff --git a/test/unittests/front_end/BUILD.gn b/test/unittests/front_end/BUILD.gn
index 92fce92..9f6657d 100644
--- a/test/unittests/front_end/BUILD.gn
+++ b/test/unittests/front_end/BUILD.gn
@@ -12,6 +12,7 @@
     "client_variations",
     "common",
     "component_helpers",
+    "components",
     "coverage",
     "dom_extension",
     "elements",
diff --git a/test/unittests/front_end/components/BUILD.gn b/test/unittests/front_end/components/BUILD.gn
new file mode 100644
index 0000000..18ccb8c
--- /dev/null
+++ b/test/unittests/front_end/components/BUILD.gn
@@ -0,0 +1,17 @@
+import("../../../../third_party/typescript/typescript.gni")
+
+ts_library("components") {
+  testonly = true
+  sources = [
+    "JSPresentationUtils_test.ts",
+    "Linkifier_test.ts",
+  ]
+
+  deps = [
+    "../../../../front_end/bindings:bundle",
+    "../../../../front_end/components:bundle",
+    "../../../../front_end/sdk:bundle",
+    "../../../../front_end/workspace:bundle",
+    "../helpers",
+  ]
+}
diff --git a/test/unittests/front_end/components/JSPresentationUtils_test.ts b/test/unittests/front_end/components/JSPresentationUtils_test.ts
new file mode 100644
index 0000000..4be2c8c
--- /dev/null
+++ b/test/unittests/front_end/components/JSPresentationUtils_test.ts
@@ -0,0 +1,61 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import type * as ComponentsModule from '../../../../front_end/components/components.js';
+import type * as BindingsModule from '../../../../front_end/bindings/bindings.js';
+import type * as WorkspaceModule from '../../../../front_end/workspace/workspace.js';
+
+import {createTarget} from '../helpers/EnvironmentHelpers.js';
+import {describeWithMockConnection} from '../helpers/MockConnection.js';
+
+const {assert} = chai;
+
+describeWithMockConnection('JSPresentationUtils', async () => {
+  let Components: typeof ComponentsModule;
+  let Bindings: typeof BindingsModule;
+  let Workspace: typeof WorkspaceModule;
+
+  before(async () => {
+    Components = await import('../../../../front_end/components/components.js');
+    Bindings = await import('../../../../front_end/bindings/bindings.js');
+    Workspace = await import('../../../../front_end/workspace/workspace.js');
+  });
+
+  function setUpEnvironment() {
+    const target = createTarget();
+    const linkifier = new Components.Linkifier.Linkifier(100, false, () => {});
+    linkifier.targetAdded(target);
+    const workspace = Workspace.Workspace.WorkspaceImpl.instance();
+    const forceNew = true;
+    const targetManager = target.targetManager();
+    const debuggerWorkspaceBinding =
+        Bindings.DebuggerWorkspaceBinding.DebuggerWorkspaceBinding.instance({forceNew, targetManager, workspace});
+    Bindings.ResourceMapping.ResourceMapping.instance({forceNew, targetManager, workspace});
+    Bindings.IgnoreListManager.IgnoreListManager.instance({forceNew, debuggerWorkspaceBinding});
+    return {target, linkifier};
+  }
+
+  function checkLinkContentForStackTracePreview(url: string, expectedLinkContent: string) {
+    const {target, linkifier} = setUpEnvironment();
+    const callFrame = {scriptId: 'scriptId', functionName: 'func', url, lineNumber: 0, columnNumber: 0};
+    const stackTrace = {callFrames: [callFrame]};
+    const options = {tabStops: false, stackTrace, contentUpdated: undefined} as
+        ComponentsModule.JSPresentationUtils.Options;
+    const {links} = Components.JSPresentationUtils.buildStackTracePreviewContents(target, linkifier, options);
+    assert.lengthOf(links, 1);
+    assert.strictEqual(links[0].textContent, expectedLinkContent);
+  }
+
+  it('uses \'unknown\' as link content if url is not available', () => {
+    const url = '';
+    const expectedLinkContent = 'unknown';
+    checkLinkContentForStackTracePreview(url, expectedLinkContent);
+  });
+
+  it('uses url as link content if url is available', () => {
+    const url = 'https://www.google.com/script.js';
+    const expectedLinkContent = 'www.google.com/script.js:1';
+    checkLinkContentForStackTracePreview(url, expectedLinkContent);
+  });
+});
diff --git a/test/unittests/front_end/components/Linkifier_test.ts b/test/unittests/front_end/components/Linkifier_test.ts
new file mode 100644
index 0000000..e554efb
--- /dev/null
+++ b/test/unittests/front_end/components/Linkifier_test.ts
@@ -0,0 +1,110 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import type * as ComponentsModule from '../../../../front_end/components/components.js';
+import type * as BindingsModule from '../../../../front_end/bindings/bindings.js';
+import type * as SDKModule from '../../../../front_end/sdk/sdk.js';
+import type * as WorkspaceModule from '../../../../front_end/workspace/workspace.js';
+
+import {createTarget} from '../helpers/EnvironmentHelpers.js';
+import {describeWithMockConnection, dispatchEvent} from '../helpers/MockConnection.js';
+import {assertNotNull} from '../../../../front_end/platform/platform.js';
+
+const {assert} = chai;
+
+describeWithMockConnection('Linkifier', async () => {
+  let SDK: typeof SDKModule;
+  let Components: typeof ComponentsModule;
+  let Bindings: typeof BindingsModule;
+  let Workspace: typeof WorkspaceModule;
+
+  before(async () => {
+    SDK = await import('../../../../front_end/sdk/sdk.js');
+    Components = await import('../../../../front_end/components/components.js');
+    Bindings = await import('../../../../front_end/bindings/bindings.js');
+    Workspace = await import('../../../../front_end/workspace/workspace.js');
+  });
+
+  function setUpEnvironment() {
+    const target = createTarget();
+    const linkifier = new Components.Linkifier.Linkifier(100, false, () => {});
+    linkifier.targetAdded(target);
+    const workspace = Workspace.Workspace.WorkspaceImpl.instance();
+    const forceNew = true;
+    const targetManager = target.targetManager();
+    const debuggerWorkspaceBinding =
+        Bindings.DebuggerWorkspaceBinding.DebuggerWorkspaceBinding.instance({forceNew, targetManager, workspace});
+    Bindings.ResourceMapping.ResourceMapping.instance({forceNew, targetManager, workspace});
+    Bindings.IgnoreListManager.IgnoreListManager.instance({forceNew, debuggerWorkspaceBinding});
+    return {target, linkifier};
+  }
+
+  it('creates an empty placeholder anchor if the debugger is disabled and no url exists', () => {
+    const {target, linkifier} = setUpEnvironment();
+
+    const debuggerModel = target.model(SDK.DebuggerModel.DebuggerModel);
+    assertNotNull(debuggerModel);
+    debuggerModel.suspendModel();
+
+    const scriptId = 'script';
+    const lineNumber = 4;
+    const url = '';
+    const anchor = linkifier.maybeLinkifyScriptLocation(target, scriptId, url, lineNumber);
+    assertNotNull(anchor);
+    assert.strictEqual(anchor.textContent, '\u200b');
+
+    const info = Components.Linkifier.Linkifier.linkInfo(anchor);
+    assertNotNull(info);
+    assert.isNull(info.uiLocation);
+  });
+
+  it('resolves url and updates link as soon as debugger is enabled', done => {
+    const {target, linkifier} = setUpEnvironment();
+
+    const debuggerModel = target.model(SDK.DebuggerModel.DebuggerModel);
+    assertNotNull(debuggerModel);
+    debuggerModel.suspendModel();
+
+    const scriptId = 'script';
+    const lineNumber = 4;
+    // Explicitly set url to empty string and let it resolve through the live location.
+    const url = '';
+    const anchor = linkifier.maybeLinkifyScriptLocation(target, scriptId, url, lineNumber);
+    assertNotNull(anchor);
+    assert.strictEqual(anchor.textContent, '\u200b');
+
+    debuggerModel.resumeModel();
+    const scriptParsedEvent = {
+      scriptId,
+      url: 'https://www.google.com/script.js',
+      startLine: 0,
+      startColumn: 0,
+      endLine: 10,
+      endColumn: 10,
+      executionContextId: 1234,
+      hash: '',
+      isLiveEdit: false,
+      sourceMapURL: undefined,
+      hasSourceURL: false,
+      hasSyntaxError: false,
+      length: 10,
+    };
+    dispatchEvent(target, 'Debugger.scriptParsed', scriptParsedEvent);
+
+    const callback: MutationCallback = function(mutations: MutationRecord[]) {
+      for (const mutation of mutations) {
+        if (mutation.type === 'childList') {
+          const info = Components.Linkifier.Linkifier.linkInfo(anchor);
+          assertNotNull(info);
+          assertNotNull(info.uiLocation);
+          assert.strictEqual(anchor.textContent, `script.js:${lineNumber + 1}`);
+          observer.disconnect();
+          done();
+        }
+      }
+    };
+    const observer = new MutationObserver(callback);
+    observer.observe(anchor, {childList: true});
+  });
+});
diff --git a/test/unittests/front_end/helpers/EnvironmentHelpers.ts b/test/unittests/front_end/helpers/EnvironmentHelpers.ts
index 3a8749f..775971d 100644
--- a/test/unittests/front_end/helpers/EnvironmentHelpers.ts
+++ b/test/unittests/front_end/helpers/EnvironmentHelpers.ts
@@ -81,6 +81,8 @@
     createSettingValue('Debugger', 'disableAsyncStackTraces', false),
     createSettingValue('Debugger', 'breakpointsActive', true),
     createSettingValue('Debugger', 'javaScriptDisabled', false),
+    createSettingValue('Debugger', 'skipContentScripts', false),
+    createSettingValue('Debugger', 'skipStackFramesPattern', '', 'regex'),
     createSettingValue('Elements', 'showDetailedInspectTooltip', true),
     createSettingValue('Network', 'cacheDisabled', false),
     createSettingValue('Rendering', 'avifFormatDisabled', false),