Bug 165578 – Web Inspector: Unable to delete breakpoint from worker script

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Matt Baker]

Summary:
Unable to delete breakpoint from worker script.

Steps to Reproduce:
1. Goto https://nerget.com/rayjs-mt/rayjs.html
2. Set a breakpoint at render-task.js:17
3. Hit Go!
  => Pauses at breakpoint
4. Delete breakpoint and resume
  => Pauses at line where breakpoint once lived

[Joseph Pecoraro]

Other code uses this pattern:

    if (breakpoint.contentIdentifier) {
        // Affect all targets
    } else if (breakpoint.scriptIdentifier) {
        // Affect only breakpoint.target
    }

I should be using this pattern in DebuggerManager.prototype._removeBreakpoint just like we do in DebuggerManager.prototype._setBreakpoint. Testing a fix now.

[Joseph Pecoraro]

Yep, that addresses the issue. I'll should be able to write a test for this.

[Joseph Pecoraro]

To clarify, the breakpoint is removed from only 1 Worker and not all Workers that shared a resource with the same URL.

[Joseph Pecoraro]

Created attachment 296468 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Comment on attachment 296468 [details]
[PATCH] Proposed Fix

Forgot to update test results in patch.

[Joseph Pecoraro]

Created attachment 296472 [details]
[PATCH] Proposed Fix

[Matt Baker]

Comment on attachment 296472 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=296472&action=review

Nice test! r=me

> LayoutTests/inspector/worker/debugger-shared-breakpoint.html:33
> +    // pause on the next statement. Do an InspectorTest.log, which evalutes

evaluates

> LayoutTests/inspector/worker/debugger-shared-breakpoint.html:74
> +            InspectorTest.evaluateInPage(`triggerCreateWorkers(); triggerWorkerResponse();`);

Seems like a template literal isn't needed here.

> LayoutTests/inspector/worker/debugger-shared-breakpoint.html:100
> +            InspectorTest.evaluateInPage(`triggerWorkerBreakpointPauses()`);

Ditto.

> LayoutTests/inspector/worker/debugger-shared-breakpoint.html:126
> +            InspectorTest.evaluateInPage(`triggerWorkerResponse()`);

Ditto.

[Joseph Pecoraro]

Comment on attachment 296472 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=296472&action=review

>> LayoutTests/inspector/worker/debugger-shared-breakpoint.html:74
>> +            InspectorTest.evaluateInPage(`triggerCreateWorkers(); triggerWorkerResponse();`);
> 
> Seems like a template literal isn't needed here.

It is easy to use a template literal for code to not have to worry about strings that may appear in the code.

[Joseph Pecoraro]

<https://trac.webkit.org/changeset/209557>