Web Inspector: Unable to delete breakpoint from worker script
authorjoepeck@webkit.org <joepeck@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 8 Dec 2016 20:23:41 +0000 (20:23 +0000)
committerjoepeck@webkit.org <joepeck@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 8 Dec 2016 20:23:41 +0000 (20:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=165578

Reviewed by Matt Baker.

Source/WebInspectorUI:

* UserInterface/Controllers/DebuggerManager.js:
(WebInspector.DebuggerManager.prototype._removeBreakpoint):
Match setting breakpoints. If this is a "URL breakpoint", affect
all targets. If this is a "Script breakpoint", affect just the
single target containing that Script.

LayoutTests:

* inspector/worker/debugger-shared-breakpoint-expected.txt: Added.
* inspector/worker/debugger-shared-breakpoint.html: Added.
Ensure setting / removing a breakpoint affects all Workers that share
a resource with the same URL.

* inspector/worker/resources/worker-debugger-pause.js:
Add an echo command that will be useful to ensure Workers are not paused.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@209557 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/inspector/worker/debugger-pause.html
LayoutTests/inspector/worker/debugger-shared-breakpoint-expected.txt [new file with mode: 0644]
LayoutTests/inspector/worker/debugger-shared-breakpoint.html [new file with mode: 0644]
LayoutTests/inspector/worker/resources/worker-debugger-pause.js
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js

index 6d2d3a2..fdb6c7e 100644 (file)
@@ -1,3 +1,18 @@
+2016-12-08  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: Unable to delete breakpoint from worker script
+        https://bugs.webkit.org/show_bug.cgi?id=165578
+
+        Reviewed by Matt Baker.
+
+        * inspector/worker/debugger-shared-breakpoint-expected.txt: Added.
+        * inspector/worker/debugger-shared-breakpoint.html: Added.
+        Ensure setting / removing a breakpoint affects all Workers that share
+        a resource with the same URL.
+
+        * inspector/worker/resources/worker-debugger-pause.js:
+        Add an echo command that will be useful to ensure Workers are not paused.
+
 2016-12-08  Ryan Haddad  <ryanhaddad@apple.com>
 
         Rebaseline fast/selectors/nth-last-child-bounds.html after r209548.
index 0c21246..b8c2d6a 100644 (file)
@@ -20,7 +20,7 @@ function test()
     }
 
     // In each test, the Worker pauses and the Main Thread is waiting to
-    // pause on the next statement. Do an InspectorTest.log, which evalutes
+    // pause on the next statement. Do an InspectorTest.log, which evaluates
     // JavaScript in the page and should pause. Then later run work.
     // In WebKit1, because the VM is shared between the inspector and
     // inspected page we need to put an artificial break between our
diff --git a/LayoutTests/inspector/worker/debugger-shared-breakpoint-expected.txt b/LayoutTests/inspector/worker/debugger-shared-breakpoint-expected.txt
new file mode 100644 (file)
index 0000000..c308a36
--- /dev/null
@@ -0,0 +1,27 @@
+Ensure setting and removing a breakpoint affects all Workers sharing a resource by URL.
+
+
+== Running test suite: Worker.Debugger.SharedBreakpoint
+-- Running test case: Worker.Debugger.SharedBreakpoint.CreateWorkers
+PASS: Two Workers created.
+PASS: Two Workers ready.
+
+-- Running test case: Worker.Debugger.SharedBreakpoint.BreakpointAdded
+PASS: All Targets paused.
+PASS: Active CalleFrame should be in a Worker.
+PASS: Pause reason should be a breakpoint.
+PAUSE AT triggerBreakpoint:9:5
+      5    
+      6    function triggerBreakpoint() {
+      7        let alpha = 1;
+ ->   8        |let beta = 2; // BREAKPOINT
+      9        let gamma = 3;
+     10    }
+     11    
+
+
+
+-- Running test case: Worker.Debugger.SharedBreakpoint.BreakpointRemoved
+PASS: Worker 1 should not have paused.
+PASS: Worker 2 should not have paused.
+
diff --git a/LayoutTests/inspector/worker/debugger-shared-breakpoint.html b/LayoutTests/inspector/worker/debugger-shared-breakpoint.html
new file mode 100644 (file)
index 0000000..c89f62b
--- /dev/null
@@ -0,0 +1,145 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
+<script src="../debugger/resources/log-pause-location.js"></script>
+<script>
+let worker1, worker2;
+
+function triggerCreateWorkers() {
+    worker1 = new Worker("resources/worker-debugger-pause.js");
+    worker2 = new Worker("resources/worker-debugger-pause.js");
+    worker1.onmessage = worker2.onmessage = function(event) {
+        TestPage.dispatchEventToFrontend("WorkerResponded");
+    };
+}
+
+function triggerWorkerBreakpointPauses() {
+    worker1.postMessage("triggerBreakpoint");
+    worker2.postMessage("triggerBreakpoint");
+}
+
+function triggerWorkerResponse() {
+    worker1.postMessage("triggerResponse");
+    worker2.postMessage("triggerResponse");
+}
+
+function test()
+{
+    let breakpoint = null;
+    let workerTarget1, workerTarget2;
+
+    // In each test, the Worker pauses and the Main Thread is waiting to
+    // pause on the next statement. Do an InspectorTest.log, which evaluates
+    // JavaScript in the page and should pause. Then later run work.
+    // In WebKit1, because the VM is shared between the inspector and
+    // inspected page we need to put an artificial break between our
+    // Inspector JavaScript, the Page JavaScript, and back to the Inspector.
+    function pauseTheMainThread() {
+        return new Promise((resolve, reject) => {
+            setTimeout(() => {
+                InspectorTest.log("");
+                setTimeout(resolve);
+            });
+        });
+    }
+
+    function areAllTargetsPaused() {
+        for (let target of WebInspector.targets) {
+            let targetData = WebInspector.debuggerManager.dataForTarget(target);
+            if (!targetData.paused)
+                return false;
+        }
+        return true;
+    }
+
+    function whenAllTargetsPaused() {
+        InspectorTest.assert(!areAllTargetsPaused());
+        return new Promise((resolve, reject) => {
+            let listener = WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.CallFramesDidChange, (event) => {
+                if (areAllTargetsPaused()) {
+                    WebInspector.debuggerManager.removeEventListener(WebInspector.DebuggerManager.Event.CallFramesDidChange, listener);
+                    resolve();
+                }
+            });
+        });
+    }
+
+    let suite = InspectorTest.createAsyncSuite("Worker.Debugger.SharedBreakpoint");
+
+    suite.addTestCase({
+        name: "Worker.Debugger.SharedBreakpoint.CreateWorkers",
+        description: "Create multiple workers.",
+        test(resolve, reject) {
+            InspectorTest.evaluateInPage(`triggerCreateWorkers(); triggerWorkerResponse();`);
+            WebInspector.targetManager.singleFireEventListener(WebInspector.TargetManager.Event.TargetAdded, (event) => {
+                workerTarget1 = event.data.target;
+                WebInspector.targetManager.singleFireEventListener(WebInspector.TargetManager.Event.TargetAdded, (event) => {
+                    workerTarget2 = event.data.target;
+                    InspectorTest.pass("Two Workers created.");
+                });
+            });
+            InspectorTest.singleFireEventListener("WorkerResponded", () => {
+                InspectorTest.singleFireEventListener("WorkerResponded", () => {
+                    InspectorTest.pass("Two Workers ready.");
+                    resolve();
+                });
+            });
+        }
+    });
+
+    suite.addTestCase({
+        name: "Worker.Debugger.SharedBreakpoint.BreakpointAdded",
+        description: "All workers should pause on the breakpoint after adding it.",
+        test(resolve, reject) {
+            InspectorTest.assert(workerTarget1.mainResource instanceof WebInspector.Script);
+            let location = workerTarget1.mainResource.createSourceCodeLocation(8, 0);
+            breakpoint = new WebInspector.Breakpoint(location);
+            WebInspector.debuggerManager.addBreakpoint(breakpoint);
+
+            InspectorTest.evaluateInPage(`triggerWorkerBreakpointPauses()`);
+            WebInspector.debuggerManager.singleFireEventListener(WebInspector.DebuggerManager.Event.Paused, (event) => {
+                pauseTheMainThread();
+                whenAllTargetsPaused().then(() => {
+                    let target = WebInspector.debuggerManager.activeCallFrame.target;
+                    let targetData = WebInspector.debuggerManager.dataForTarget(target);
+
+                    InspectorTest.pass("All Targets paused.");
+                    InspectorTest.expectEqual(target.type, WebInspector.Target.Type.Worker, "Active CalleFrame should be in a Worker.");
+                    InspectorTest.expectEqual(targetData.pauseReason, WebInspector.DebuggerManager.PauseReason.Breakpoint, "Pause reason should be a breakpoint.");
+                    window.loadLinesFromSourceCode(target.mainResource).then(() => {
+                        logPauseLocation();
+                        WebInspector.debuggerManager.resume().then(resolve, reject);
+                    });
+                });
+            });
+        }
+    });
+
+    suite.addTestCase({
+        name: "Worker.Debugger.SharedBreakpoint.BreakpointRemoved",
+        description: "No worker should pause on the breakpoint after removing it.",
+        test(resolve, reject) {
+            WebInspector.debuggerManager.removeBreakpoint(breakpoint);
+
+            InspectorTest.evaluateInPage(`triggerWorkerBreakpointPauses()`);
+            InspectorTest.evaluateInPage(`triggerWorkerResponse()`);
+
+            InspectorTest.singleFireEventListener("WorkerResponded", () => {
+                InspectorTest.pass("Worker 1 should not have paused.");
+                InspectorTest.singleFireEventListener("WorkerResponded", () => {
+                    InspectorTest.pass("Worker 2 should not have paused.");
+                    resolve();
+                });
+            });
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>Ensure setting and removing a breakpoint affects all Workers sharing a resource by URL.</p>
+</body>
+</html>
index cb3766b..73ecc19 100644 (file)
@@ -27,4 +27,6 @@ onmessage = function(event) {
         triggerException();
     else if (event.data === "triggerAssertion")
         triggerAssertion();
+    else if (event.data === "triggerResponse")
+        postMessage("response");
 }
index 1decb37..54a8d40 100644 (file)
@@ -1,3 +1,16 @@
+2016-12-08  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: Unable to delete breakpoint from worker script
+        https://bugs.webkit.org/show_bug.cgi?id=165578
+
+        Reviewed by Matt Baker.
+
+        * UserInterface/Controllers/DebuggerManager.js:
+        (WebInspector.DebuggerManager.prototype._removeBreakpoint):
+        Match setting breakpoints. If this is a "URL breakpoint", affect
+        all targets. If this is a "Script breakpoint", affect just the
+        single target containing that Script.
+
 2016-12-07  Devin Rousso  <dcrousso+webkit@gmail.com>
 
         REGRESSION(r203912): Web Inspector: Navigation sidebar widths are not saved
index 2af3426..f682cae 100644 (file)
@@ -919,11 +919,12 @@ WebInspector.DebuggerManager = class DebuggerManager extends WebInspector.Object
                 callback();
         }
 
-        if (breakpoint.target)
-            breakpoint.target.DebuggerAgent.removeBreakpoint(breakpoint.identifier, didRemoveBreakpoint.bind(this));
-        else {
+        if (breakpoint.contentIdentifier) {
             for (let target of WebInspector.targets)
                 target.DebuggerAgent.removeBreakpoint(breakpoint.identifier, didRemoveBreakpoint.bind(this));
+        } else if (breakpoint.scriptIdentifier) {
+            let target = breakpoint.target;
+            target.DebuggerAgent.removeBreakpoint(breakpoint.identifier, didRemoveBreakpoint.bind(this));
         }
     }