Attachment 358391 Details for Bug 193158 – Patch

[patch] Patch

bug-193158-20190104171224.patch (text/plain), 18.46 KB, created by Devin Rousso on 2019-01-04 16:12:25 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Devin Rousso

Created: 2019-01-04 16:12:25 PST

Size: 18.46 KB

patch

obsolete

>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 409dd675b23ffc9f0383d53c96a132f59a916e3c..710ce75ed416fae8b4899fecde785b8081ab38a1 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,36 @@
>+2019-01-04  Devin Rousso  <drousso@apple.com>
>+
>+        Web Inspector: Audit: disable breakpoints when running Audit
>+        https://bugs.webkit.org/show_bug.cgi?id=193158
>+        <rdar://problem/47057083>
>+
>+        Reviewed by Joseph Pecoraro.
>+
>+        * UserInterface/Controllers/DebuggerManager.js:
>+        (WI.DebuggerManager):
>+        (WI.DebuggerManager.prototype._startDisablingBreakpointsTemporarily):
>+        (WI.DebuggerManager.prototype._stopDisablingBreakpointsTemporarily):
>+        (WI.DebuggerManager.prototype._handleAuditManagerTestScheduled): Added.
>+        (WI.DebuggerManager.prototype._handleAuditManagerTestCompleted): Added.
>+
>+        * UserInterface/Views/DebuggerSidebarPanel.js:
>+        (WI.DebuggerSidebarPanel.prototype._timelineCapturingWillStart):
>+        (WI.DebuggerSidebarPanel.prototype._timelineCapturingStopped):
>+        (WI.DebuggerSidebarPanel.prototype._handleAuditManagerTestScheduled): Added.
>+        (WI.DebuggerSidebarPanel.prototype._handleAuditManagerTestCompleted): Added.
>+        (WI.DebuggerSidebarPanel.prototype._updateBreakpointsDisabledBanner):
>+        * UserInterface/Views/DebuggerSidebarPanel.css:
>+        (.sidebar > .panel.navigation.debugger .warning-banner + .warning-banner): Added.
>+
>+        * UserInterface/Base/Main.js:
>+        (WI.loaded):
>+        * UserInterface/Test/Test.js:
>+        (WI.loaded):
>+        Move `WI.auditManager` higher in the managers list so that it can be used in
>+        `WI.DebuggerManager`'s constructor.
>+
>+        * Localizations/en.lproj/localizedStrings.js:
>+
> 2019-01-04  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         Web Inspector: Add a Setting Toggle for Source Maps
>diff --git a/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js b/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js
>index 0721c0958999e2c21872577661075aab1b447ea1..db4d51858742ebc8321edef91bc3b4855f64c8c0 100644
>--- a/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js
>+++ b/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js
>@@ -276,6 +276,7 @@ localizedStrings["Debug: "] = "Debug: ";
> localizedStrings["Debugger"] = "Debugger";
> localizedStrings["Debugger Paused"] = "Debugger Paused";
> localizedStrings["Debugger Statement"] = "Debugger Statement";
>+localizedStrings["Debugger disabled during Audit"] = "Debugger disabled during Audit";
> localizedStrings["Debugger disabled during Timeline recording"] = "Debugger disabled during Timeline recording";
> localizedStrings["Debugger:"] = "Debugger:";
> localizedStrings["Debugs"] = "Debugs";
>@@ -896,6 +897,7 @@ localizedStrings["Step out (%s or %s)"] = "Step out (%s or %s)";
> localizedStrings["Step over (%s or %s)"] = "Step over (%s or %s)";
> localizedStrings["Stiffness"] = "Stiffness";
> localizedStrings["Stop"] = "Stop";
>+localizedStrings["Stop Audit"] = "Stop Audit";
> localizedStrings["Stop Recording"] = "Stop Recording";
> localizedStrings["Stop element selection (%s)"] = "Stop element selection (%s)";
> localizedStrings["Stop recording"] = "Stop recording";
>diff --git a/Source/WebInspectorUI/UserInterface/Base/Main.js b/Source/WebInspectorUI/UserInterface/Base/Main.js
>index 251cef7c43434448ba6c972ea27faa0cca7884b6..81b06dd502e8ae5793ca26cc1745f07fc2eb3e6a 100644
>--- a/Source/WebInspectorUI/UserInterface/Base/Main.js
>+++ b/Source/WebInspectorUI/UserInterface/Base/Main.js
>@@ -114,12 +114,12 @@ WI.loaded = function()
>         WI.memoryManager = new WI.MemoryManager,
>         WI.applicationCacheManager = new WI.ApplicationCacheManager,
>         WI.timelineManager = new WI.TimelineManager,
>+        WI.auditManager = new WI.AuditManager,
>         WI.debuggerManager = new WI.DebuggerManager,
>         WI.layerTreeManager = new WI.LayerTreeManager,
>         WI.workerManager = new WI.WorkerManager,
>         WI.domDebuggerManager = new WI.DOMDebuggerManager,
>         WI.canvasManager = new WI.CanvasManager,
>-        WI.auditManager = new WI.AuditManager,
>     ];
> 
>     // Register for events.
>diff --git a/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js b/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js
>index e994556128d000e794398582e8222413271b4284..8562816b1d9718883dac32d85b492132f1c18a68 100644
>--- a/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js
>+++ b/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js
>@@ -41,6 +41,9 @@ WI.DebuggerManager = class DebuggerManager extends WI.Object
>         WI.timelineManager.addEventListener(WI.TimelineManager.Event.CapturingWillStart, this._timelineCapturingWillStart, this);
>         WI.timelineManager.addEventListener(WI.TimelineManager.Event.CapturingStopped, this._timelineCapturingStopped, this);
> 
>+        WI.auditManager.addEventListener(WI.AuditManager.Event.TestScheduled, this._handleAuditManagerTestScheduled, this);
>+        WI.auditManager.addEventListener(WI.AuditManager.Event.TestCompleted, this._handleAuditManagerTestCompleted, this);
>+
>         WI.targetManager.addEventListener(WI.TargetManager.Event.TargetRemoved, this._targetRemoved, this);
> 
>         WI.settings.pauseForInternalScripts.addEventListener(WI.Setting.Event.Changed, this._pauseForInternalScriptsDidChange, this);
>@@ -93,6 +96,7 @@ WI.DebuggerManager = class DebuggerManager extends WI.Object
>             this._breakpointsEnabledSetting.value = this._temporarilyDisabledBreakpointsRestoreSetting.value;
>             this._temporarilyDisabledBreakpointsRestoreSetting.value = null;
>         }
>+        this._temporarilyDisableBreakpointsRequestCount = 0;
> 
>         this._ignoreBreakpointDisplayLocationDidChangeEvent = false;
> 
>@@ -1110,10 +1114,14 @@ WI.DebuggerManager = class DebuggerManager extends WI.Object
> 
>     _startDisablingBreakpointsTemporarily()
>     {
>+        if (++this._temporarilyDisableBreakpointsRequestCount > 1)
>+            return;
>+
>         console.assert(!this.breakpointsDisabledTemporarily, "Already temporarily disabling breakpoints.");
>         if (this.breakpointsDisabledTemporarily)
>             return;
> 
>+
>         this._temporarilyDisabledBreakpointsRestoreSetting.value = this._breakpointsEnabledSetting.value;
> 
>         this.breakpointsEnabled = false;
>@@ -1121,6 +1129,10 @@ WI.DebuggerManager = class DebuggerManager extends WI.Object
> 
>     _stopDisablingBreakpointsTemporarily()
>     {
>+        this._temporarilyDisableBreakpointsRequestCount = Math.max(0, this._temporarilyDisableBreakpointsRequestCount - 1);
>+        if (this._temporarilyDisableBreakpointsRequestCount > 0)
>+            return;
>+
>         console.assert(this.breakpointsDisabledTemporarily, "Was not temporarily disabling breakpoints.");
>         if (!this.breakpointsDisabledTemporarily)
>             return;
>@@ -1144,6 +1156,19 @@ WI.DebuggerManager = class DebuggerManager extends WI.Object
>         this._stopDisablingBreakpointsTemporarily();
>     }
> 
>+    _handleAuditManagerTestScheduled(event)
>+    {
>+        this._startDisablingBreakpointsTemporarily();
>+
>+        if (this.paused)
>+            this.resume();
>+    }
>+
>+    _handleAuditManagerTestCompleted(event)
>+    {
>+        this._stopDisablingBreakpointsTemporarily();
>+    }
>+
>     _targetRemoved(event)
>     {
>         let wasPaused = this.paused;
>diff --git a/Source/WebInspectorUI/UserInterface/Test/Test.js b/Source/WebInspectorUI/UserInterface/Test/Test.js
>index f8adde814a31199222b807081ba3dacaeb27b5de..195fbc21222239dda5037848fbf2435249ed7907 100644
>--- a/Source/WebInspectorUI/UserInterface/Test/Test.js
>+++ b/Source/WebInspectorUI/UserInterface/Test/Test.js
>@@ -57,12 +57,12 @@ WI.loaded = function()
>         WI.heapManager = new WI.HeapManager,
>         WI.memoryManager = new WI.MemoryManager,
>         WI.timelineManager = new WI.TimelineManager,
>+        WI.auditManager = new WI.AuditManager,
>         WI.debuggerManager = new WI.DebuggerManager,
>         WI.layerTreeManager = new WI.LayerTreeManager,
>         WI.workerManager = new WI.WorkerManager,
>         WI.domDebuggerManager = new WI.DOMDebuggerManager,
>         WI.canvasManager = new WI.CanvasManager,
>-        WI.auditManager = new WI.AuditManager,
>     ];
> 
>     // Register for events.
>diff --git a/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.css b/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.css
>index f7448e136c2805125773e06c339fd9969b640fda..48f53ed51e90cdc1b9c2df515d8e4b2b43630eec 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.css
>+++ b/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.css
>@@ -62,6 +62,10 @@
>     background-color: hsl(50, 100%, 94%);
> }
> 
>+.sidebar > .panel.navigation.debugger .warning-banner + .warning-banner {
>+    display: none;
>+}
>+
> .sidebar > .panel.navigation.debugger .tree-outline.single-thread > .item.thread {
>     display: none;
> }
>diff --git a/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js b/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js
>index 90ab86d2af47e623491f0914596d4bbe66b49e67..e7b3db92dd2b839406c7870a7531580caede3a17 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js
>@@ -58,15 +58,25 @@ WI.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WI.NavigationSideba
>         WI.timelineManager.addEventListener(WI.TimelineManager.Event.CapturingWillStart, this._timelineCapturingWillStart, this);
>         WI.timelineManager.addEventListener(WI.TimelineManager.Event.CapturingStopped, this._timelineCapturingStopped, this);
> 
>+        WI.auditManager.addEventListener(WI.AuditManager.Event.TestScheduled, this._handleAuditManagerTestScheduled, this);
>+        WI.auditManager.addEventListener(WI.AuditManager.Event.TestCompleted, this._handleAuditManagerTestCompleted, this);
>+
>         WI.targetManager.addEventListener(WI.TargetManager.Event.TargetAdded, this._targetAdded, this);
>         WI.targetManager.addEventListener(WI.TargetManager.Event.TargetRemoved, this._targetRemoved, this);
> 
>         this._timelineRecordingWarningElement = document.createElement("div");
>         this._timelineRecordingWarningElement.classList.add("warning-banner");
>-        this._timelineRecordingWarningElement.append(WI.UIString("Debugger disabled during Timeline recording"), " ");
>-        let stopRecordingLink = this._timelineRecordingWarningElement.appendChild(document.createElement("a"));
>-        stopRecordingLink.textContent = WI.UIString("Stop recording");
>-        stopRecordingLink.addEventListener("click", () => { WI.timelineManager.stopCapturing(); });
>+        this._timelineRecordingWarningElement.append(WI.UIString("Debugger disabled during Timeline recording"), document.createElement("br"));
>+        let timelineStopRecordingLink = this._timelineRecordingWarningElement.appendChild(document.createElement("a"));
>+        timelineStopRecordingLink.textContent = WI.UIString("Stop recording");
>+        timelineStopRecordingLink.addEventListener("click", () => { WI.timelineManager.stopCapturing(); });
>+
>+        this._auditTestWarningElement = document.createElement("div");
>+        this._auditTestWarningElement.classList.add("warning-banner");
>+        this._auditTestWarningElement.append(WI.UIString("Debugger disabled during Audit"), document.createElement("br"));
>+        let auditStopRecordingLink = this._auditTestWarningElement.appendChild(document.createElement("a"));
>+        auditStopRecordingLink.textContent = WI.UIString("Stop Audit");
>+        auditStopRecordingLink.addEventListener("click", () => { WI.auditManager.stop(); });
> 
>         this._breakpointsDisabledWarningElement = document.createElement("div");
>         this._breakpointsDisabledWarningElement.classList.add("warning-banner");
>@@ -622,8 +632,7 @@ WI.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WI.NavigationSideba
> 
>     _timelineCapturingWillStart(event)
>     {
>-        this._debuggerBreakpointsButtonItem.enabled = false;
>-        this._debuggerPauseResumeButtonItem.enabled = false;
>+        this._updateTemporarilyDisabledBreakpointsButtons();
> 
>         this.contentView.element.insertBefore(this._timelineRecordingWarningElement, this.contentView.element.firstChild);
>         this._updateBreakpointsDisabledBanner();
>@@ -631,19 +640,42 @@ WI.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WI.NavigationSideba
> 
>     _timelineCapturingStopped(event)
>     {
>-        this._debuggerBreakpointsButtonItem.enabled = true;
>-        this._debuggerPauseResumeButtonItem.enabled = true;
>+        this._updateTemporarilyDisabledBreakpointsButtons();
> 
>         this._timelineRecordingWarningElement.remove();
>         this._updateBreakpointsDisabledBanner();
>     }
> 
>+    _handleAuditManagerTestScheduled(event)
>+    {
>+        this._updateTemporarilyDisabledBreakpointsButtons();
>+
>+        this.contentView.element.insertBefore(this._auditTestWarningElement, this.contentView.element.firstChild);
>+        this._updateBreakpointsDisabledBanner();
>+    }
>+
>+    _handleAuditManagerTestCompleted(event)
>+    {
>+        this._updateTemporarilyDisabledBreakpointsButtons();
>+
>+        this._auditTestWarningElement.remove();
>+        this._updateBreakpointsDisabledBanner();
>+    }
>+
>+    _updateTemporarilyDisabledBreakpointsButtons()
>+    {
>+        let breakpointsDisabledTemporarily = WI.debuggerManager.breakpointsDisabledTemporarily;
>+        this._debuggerBreakpointsButtonItem.enabled = !breakpointsDisabledTemporarily;
>+        this._debuggerPauseResumeButtonItem.enabled = !breakpointsDisabledTemporarily;
>+    }
>+
>     _updateBreakpointsDisabledBanner()
>     {
>         let breakpointsEnabled = WI.debuggerManager.breakpointsEnabled;
>         let timelineWarningShowing = !!this._timelineRecordingWarningElement.parentElement;
>+        let auditWarningShowing = !!this._auditTestWarningElement.parentElement;
> 
>-        if (!breakpointsEnabled && !timelineWarningShowing)
>+        if (!breakpointsEnabled && !timelineWarningShowing && !auditWarningShowing)
>             this.contentView.element.insertBefore(this._breakpointsDisabledWarningElement, this.contentView.element.firstChild);
>         else
>             this._breakpointsDisabledWarningElement.remove();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 162bd0b0ecf3d1a0ce3d273a5c8aa28e41c98f71..529f21c33d5832478e5dbc2e38358808f3296fa3 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-01-04  Devin Rousso  <drousso@apple.com>
>+
>+        Web Inspector: Audit: disable breakpoints when running Audit
>+        https://bugs.webkit.org/show_bug.cgi?id=193158
>+        <rdar://problem/47057083>
>+
>+        Reviewed by Joseph Pecoraro.
>+
>+        * inspector/audit/basic.html:
>+        * inspector/audit/basic-expected.txt:
>+
>+        * inspector/audit/resources/audit-utilities.js:
>+        (TestPage.registerInitializer.InspectorTest.Audit.addTest):
>+
> 2019-01-04  Jer Noble  <jer.noble@apple.com>
> 
>         Web Content process main thread blocked beneath ImageDecoderAVFObjC::readSamples for many seconds on imgur.com
>diff --git a/LayoutTests/inspector/audit/basic-expected.txt b/LayoutTests/inspector/audit/basic-expected.txt
>index b1a054918319423245a03eaf2281839d820f24d8..79a2965155c126b61f522d2520f329818fc8a8f6 100644
>--- a/LayoutTests/inspector/audit/basic-expected.txt
>+++ b/LayoutTests/inspector/audit/basic-expected.txt
>@@ -126,3 +126,8 @@ PASS: Result should be "error".
>   errors:
>    - rejected
> 
>+-- Running test case: Audit.Basic.Debugger
>+Testing `debugger`...
>+PASS: Should not pause on the debugger statement.
>+PASS: Result should be "pass".
>+
>diff --git a/LayoutTests/inspector/audit/basic.html b/LayoutTests/inspector/audit/basic.html
>index f31ebf44de18066a9f20c8c48c4c4e9cff4bf86c..921a7410734328f3606c476d9c486c8eaa758dcb 100644
>--- a/LayoutTests/inspector/audit/basic.html
>+++ b/LayoutTests/inspector/audit/basic.html
>@@ -45,6 +45,43 @@ function test()
>     InspectorTest.Audit.addPromiseTest("Audit.Basic.Error.Promise.Resolved", `setTimeout(resolve, 0)`, WI.AuditTestCaseResult.Level.Error);
>     InspectorTest.Audit.addPromiseTest("Audit.Basic.Error.Promise.Rejected", `setTimeout(reject, 0, "rejected")`, WI.AuditTestCaseResult.Level.Error);
> 
>+    suite.addTestCase({
>+        name: "Audit.Basic.Debugger",
>+        description: "Test that a debugger statement doesn't pause when running an audit.",
>+        test(resolve, reject) {
>+            const level = "pass";
>+            let audit = new WI.AuditTestCase("Audit.Basic.Debugger", `function() { debugger; return "${level}"; }`);
>+
>+            let paused = false;
>+            let pausedListener = WI.debuggerManager.singleFireEventListener(WI.DebuggerManager.Event.Paused, (event) => {
>+                paused = true;
>+
>+                WI.debuggerManager.resume();
>+            });
>+
>+            WI.auditManager.awaitEvent(WI.AuditManager.Event.TestCompleted).then((event) => {
>+                InspectorTest.expectFalse(paused, "Should not pause on the debugger statement.");
>+
>+                let results = WI.auditManager.results.lastValue;
>+                InspectorTest.assert(results.length === 1, "There should be 1 result.");
>+
>+                let result = results[0];
>+                InspectorTest.assert(result instanceof WI.AuditTestCaseResult, "Result should be a WI.AuditTestCaseResult.");
>+                if (!result)
>+                    return;
>+
>+                InspectorTest.expectEqual(result.level, level, `Result should be "${level}".`);
>+
>+                WI.debuggerManager.removeEventListener(WI.DebuggerManager.Event.Paused, pausedListener);
>+            });
>+
>+            InspectorTest.log("Testing `debugger`...");
>+
>+            WI.auditManager.start([audit])
>+            .then(resolve, reject);
>+        },
>+    });
>+
>     suite.runTestCasesAndFinish();
> }
> </script>
>diff --git a/LayoutTests/inspector/audit/resources/audit-utilities.js b/LayoutTests/inspector/audit/resources/audit-utilities.js
>index 10f114c82e00597205a57cfb5e5bffe4ad218fdd..8279464ef511057f0ef3eb0c773b0b21eb342b15 100644
>--- a/LayoutTests/inspector/audit/resources/audit-utilities.js
>+++ b/LayoutTests/inspector/audit/resources/audit-utilities.js
>@@ -58,8 +58,7 @@ TestPage.registerInitializer(() => {
>                         logArray("domAttributes", data.domAttributes);
>                     if (data.errors)
>                         logArray("errors", data.errors);
>-                })
>-                .then(resolve, reject);
>+                });
> 
>                 InspectorTest.log("Testing" + (logs.beforeStart || "") + "...");
>

Actions: View | Formatted Diff | Diff

Attachments on bug 193158: 358367 | 358391