[webkit-changes] [198774] trunk/Source/WebInspectorUI

Mon, 28 Mar 2016 20:28:21 -0700

Title: [198774] trunk/Source/WebInspectorUI
Revision
198774
Author
commit-qu...@webkit.org
Date
2016-03-28 20:28:10 -0700 (Mon, 28 Mar 2016)

Log Message

Web Inspector: Ensure maximum accuracy while profiling
https://bugs.webkit.org/show_bug.cgi?id=155809
<rdar://problem/25325035>

Patch by Joseph Pecoraro <pecor...@apple.com> on 2016-03-28
Reviewed by Timothy Hatcher.

* Localizations/en.lproj/localizedStrings.js:
New strings.

* UserInterface/Controllers/DebuggerManager.js:
(WebInspector.DebuggerManager):
When starting the inspector, if it was previously closed while
breakpoints were temporarily disabled, restore the correct
breakpoints enabled state.

(WebInspector.DebuggerManager.prototype.set breakpointsEnabled):
Warn if we ever try to enable breakpoints during timeline recordings.

(WebInspector.DebuggerManager.prototype.get breakpointsDisabledTemporarily):
(WebInspector.DebuggerManager.prototype.startDisablingBreakpointsTemporarily):
(WebInspector.DebuggerManager.prototype.stopDisablingBreakpointsTemporarily):
Method to start/stop temporarily disabling breakpoints.

(WebInspector.DebuggerManager.prototype._breakpointDisabledStateDidChange):
(WebInspector.DebuggerManager.prototype._setBreakpoint):
When temporarily disabling breakpoints avoid the convenience behavior of
enabling all breakpoints when enabling or setting a single breakpoint.

* UserInterface/Controllers/TimelineManager.js:
(WebInspector.TimelineManager.prototype.startCapturing):
Emit a will start capturing event to do work before enabling instruments.

* UserInterface/Views/DebuggerSidebarPanel.css:
(.sidebar > .panel.navigation.debugger .timeline-recording-warning):
(.sidebar > .panel.navigation.debugger .timeline-recording-warning > a):
Styles for a warning section in the Debugger Sidebar when the Debugger
is temporarily disabled due to a Timeline recording.

* UserInterface/Views/DebuggerSidebarPanel.js:
(WebInspector.DebuggerSidebarPanel.prototype._timelineRecordingWillStart):
(WebInspector.DebuggerSidebarPanel.prototype._timelineRecordingStopped):
Modify the Debugger state and UI before and after a Timeline recording.

Modified Paths

Diff

Modified: trunk/Source/WebInspectorUI/ChangeLog (198773 => 198774)


--- trunk/Source/WebInspectorUI/ChangeLog	2016-03-29 02:03:22 UTC (rev 198773)
+++ trunk/Source/WebInspectorUI/ChangeLog	2016-03-29 03:28:10 UTC (rev 198774)
@@ -1,3 +1,48 @@
+2016-03-28  Joseph Pecoraro  <pecor...@apple.com>
+
+        Web Inspector: Ensure maximum accuracy while profiling
+        https://bugs.webkit.org/show_bug.cgi?id=155809
+        <rdar://problem/25325035>
+
+        Reviewed by Timothy Hatcher.
+
+        * Localizations/en.lproj/localizedStrings.js:
+        New strings.
+
+        * UserInterface/Controllers/DebuggerManager.js:
+        (WebInspector.DebuggerManager):
+        When starting the inspector, if it was previously closed while
+        breakpoints were temporarily disabled, restore the correct
+        breakpoints enabled state.
+
+        (WebInspector.DebuggerManager.prototype.set breakpointsEnabled):
+        Warn if we ever try to enable breakpoints during timeline recordings.
+
+        (WebInspector.DebuggerManager.prototype.get breakpointsDisabledTemporarily):
+        (WebInspector.DebuggerManager.prototype.startDisablingBreakpointsTemporarily):
+        (WebInspector.DebuggerManager.prototype.stopDisablingBreakpointsTemporarily):
+        Method to start/stop temporarily disabling breakpoints.
+
+        (WebInspector.DebuggerManager.prototype._breakpointDisabledStateDidChange):
+        (WebInspector.DebuggerManager.prototype._setBreakpoint):
+        When temporarily disabling breakpoints avoid the convenience behavior of
+        enabling all breakpoints when enabling or setting a single breakpoint.
+
+        * UserInterface/Controllers/TimelineManager.js:
+        (WebInspector.TimelineManager.prototype.startCapturing):
+        Emit a will start capturing event to do work before enabling instruments.
+
+        * UserInterface/Views/DebuggerSidebarPanel.css:
+        (.sidebar > .panel.navigation.debugger .timeline-recording-warning):
+        (.sidebar > .panel.navigation.debugger .timeline-recording-warning > a):
+        Styles for a warning section in the Debugger Sidebar when the Debugger
+        is temporarily disabled due to a Timeline recording.
+
+        * UserInterface/Views/DebuggerSidebarPanel.js:
+        (WebInspector.DebuggerSidebarPanel.prototype._timelineRecordingWillStart):
+        (WebInspector.DebuggerSidebarPanel.prototype._timelineRecordingStopped):
+        Modify the Debugger state and UI before and after a Timeline recording.
+
 2016-03-28  Nikita Vasilyev  <nvasil...@apple.com>
 
         Web Inspector: Use font-variant-numeric: tabular-nums instead of -apple-system-monospaced-numbers

Modified: trunk/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js (198773 => 198774)


--- trunk/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js	2016-03-29 02:03:22 UTC (rev 198773)
+++ trunk/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js	2016-03-29 03:28:10 UTC (rev 198774)
@@ -215,6 +215,7 @@
 localizedStrings["Debugger"] = "Debugger";
 localizedStrings["Debugger Paused"] = "Debugger Paused";
 localizedStrings["Debugger Statement"] = "Debugger Statement";
+localizedStrings["Debugger is disabled during a Timeline recording."] = "Debugger is disabled during a Timeline recording.";
 localizedStrings["Decoded"] = "Decoded";
 localizedStrings["Decoration"] = "Decoration";
 localizedStrings["Default"] = "Default";
@@ -683,6 +684,7 @@
 localizedStrings["Step over (%s or %s)"] = "Step over (%s or %s)";
 localizedStrings["Stop Recording"] = "Stop Recording";
 localizedStrings["Stop recording (%s)"] = "Stop recording (%s)";
+localizedStrings["Stop recording."] = "Stop recording.";
 localizedStrings["Storage"] = "Storage";
 localizedStrings["Style"] = "Style";
 localizedStrings["Style Attribute"] = "Style Attribute";

Modified: trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js (198773 => 198774)


--- trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js	2016-03-29 02:03:22 UTC (rev 198773)
+++ trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js	2016-03-29 03:28:10 UTC (rev 198774)
@@ -71,6 +71,14 @@
         this._breakpointsSetting = new WebInspector.Setting("breakpoints", []);
         this._breakpointsEnabledSetting = new WebInspector.Setting("breakpoints-enabled", true);
 
+        // Restore the correct breakpoints enabled setting if Web Inspector had
+        // previously been left in a state where breakpoints were temporarily disabled.
+        this._temporarilyDisabledBreakpointsRestoreSetting = new WebInspector.Setting("temporarily-disabled-breakpoints-restore", null);
+        if (this._temporarilyDisabledBreakpointsRestoreSetting.value !== null) {
+            this._breakpointsEnabledSetting.value = this._temporarilyDisabledBreakpointsRestoreSetting.value;
+            this._temporarilyDisabledBreakpointsRestoreSetting.value = null;
+        }
+
         if (window.DebuggerAgent)
             DebuggerAgent.setBreakpointsActive(this._breakpointsEnabledSetting.value);
 
@@ -100,6 +108,10 @@
         if (this._breakpointsEnabledSetting.value === enabled)
             return;
 
+        console.assert(!(enabled && this.breakpointsDisabledTemporarily), "Should not enable breakpoints when we are temporarily disabling breakpoints.");
+        if (enabled && this.breakpointsDisabledTemporarily)
+            return;
+
         this._breakpointsEnabledSetting.value = enabled;
 
         this.dispatchEventToListeners(WebInspector.DebuggerManager.Event.BreakpointsEnabledDidChange);
@@ -335,6 +347,34 @@
         return knownScripts;
     }
 
+    get breakpointsDisabledTemporarily()
+    {
+        return this._temporarilyDisabledBreakpointsRestoreSetting.value !== null;
+    }
+
+    startDisablingBreakpointsTemporarily()
+    {
+        console.assert(this._temporarilyDisabledBreakpointsRestoreSetting.value === null, "Already temporarily disabling breakpoints.");
+        if (this._temporarilyDisabledBreakpointsRestoreSetting.value !== null)
+            return;
+
+        this._temporarilyDisabledBreakpointsRestoreSetting.value = this._breakpointsEnabledSetting.value;
+
+        this.breakpointsEnabled = false;
+    }
+
+    stopDisablingBreakpointsTemporarily()
+    {
+        console.assert(this._temporarilyDisabledBreakpointsRestoreSetting.value !== null, "Was not temporarily disabling breakpoints.");
+        if (this._temporarilyDisabledBreakpointsRestoreSetting.value === null)
+            return;
+
+        let restoreState = this._temporarilyDisabledBreakpointsRestoreSetting.value;
+        this._temporarilyDisabledBreakpointsRestoreSetting.value = null;
+
+        this.breakpointsEnabled = restoreState;
+    }
+
     addBreakpoint(breakpoint, skipEventDispatch, shouldSpeculativelyResolve)
     {
         console.assert(breakpoint instanceof WebInspector.Breakpoint, "Bad argument to DebuggerManger.addBreakpoint: ", breakpoint);
@@ -687,7 +727,7 @@
         if (breakpoint.identifier || breakpoint.disabled)
             return;
 
-        if (!this._restoringBreakpoints) {
+        if (!this._restoringBreakpoints && !this.breakpointsDisabledTemporarily) {
             // Enable breakpoints since a breakpoint is being set. This eliminates
             // a multi-step process for the user that can be confusing.
             this.breakpointsEnabled = true;
@@ -800,7 +840,7 @@
 
         let breakpoint = event.target;
         if (breakpoint === this._allExceptionsBreakpoint) {
-            if (!breakpoint.disabled)
+            if (!breakpoint.disabled && !this.breakpointsDisabledTemporarily)
                 this.breakpointsEnabled = true;
             this._allExceptionsBreakpointEnabledSetting.value = !breakpoint.disabled;
             this._updateBreakOnExceptionsState();
@@ -808,7 +848,7 @@
         }
 
         if (breakpoint === this._allUncaughtExceptionsBreakpoint) {
-            if (!breakpoint.disabled)
+            if (!breakpoint.disabled && !this.breakpointsDisabledTemporarily)
                 this.breakpointsEnabled = true;
             this._allUncaughtExceptionsBreakpointEnabledSetting.value = !breakpoint.disabled;
             this._updateBreakOnExceptionsState();

Modified: trunk/Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js (198773 => 198774)


--- trunk/Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js	2016-03-29 02:03:22 UTC (rev 198773)
+++ trunk/Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js	2016-03-29 03:28:10 UTC (rev 198774)
@@ -158,6 +158,8 @@
         if (!this._activeRecording || shouldCreateRecording)
             this._loadNewRecording();
 
+        this.dispatchEventToListeners(WebInspector.TimelineManager.Event.CapturingWillStart);
+
         this._activeRecording.start();
     }
 
@@ -866,6 +868,7 @@
 WebInspector.TimelineManager.Event = {
     RecordingCreated: "timeline-manager-recording-created",
     RecordingLoaded: "timeline-manager-recording-loaded",
+    CapturingWillStart: "timeline-manager-capturing-will-start",
     CapturingStarted: "timeline-manager-capturing-started",
     CapturingStopped: "timeline-manager-capturing-stopped"
 };

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.css (198773 => 198774)


--- trunk/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.css	2016-03-29 02:03:22 UTC (rev 198773)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.css	2016-03-29 03:28:10 UTC (rev 198774)
@@ -23,7 +23,6 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-
 .sidebar > .panel.navigation.debugger > :matches(.content, .empty-content-placeholder) {
     top: 29px;
 }
@@ -35,6 +34,22 @@
     right: 0;
 }
 
+.sidebar > .panel.navigation.debugger .timeline-recording-warning {
+    text-align: center;
+    font-size: 11px;
+
+    padding: 11px 6px;
+    margin-bottom: 6px;
+
+    border-bottom: 1px solid var(--border-color);
+    background-color: hsl(50, 100%, 94%);
+}
+
+.sidebar > .panel.navigation.debugger .timeline-recording-warning > a {
+    text-decoration: underline;
+    cursor: pointer;
+}
+
 .sidebar > .panel.navigation.debugger .details-section {
     font-size: 11px;
 }
@@ -47,7 +62,6 @@
     display: none;
 }
 
-
 .sidebar > .panel.navigation.debugger .details-section > .content > .group {
     display: block;
 }

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js (198773 => 198774)


--- trunk/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js	2016-03-29 02:03:22 UTC (rev 198773)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js	2016-03-29 03:28:10 UTC (rev 198774)
@@ -46,6 +46,16 @@
         WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.ActiveCallFrameDidChange, this._debuggerActiveCallFrameDidChange, this);
         WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.WaitingToPause, this._debuggerWaitingToPause, this);
 
+        WebInspector.timelineManager.addEventListener(WebInspector.TimelineManager.Event.CapturingWillStart, this._timelineRecordingWillStart, this);
+        WebInspector.timelineManager.addEventListener(WebInspector.TimelineManager.Event.CapturingStopped, this._timelineRecordingStopped, this);        
+
+        this._timelineRecordingWarningElement = document.createElement("div");
+        this._timelineRecordingWarningElement.classList.add("timeline-recording-warning");
+        this._timelineRecordingWarningElement.append(WebInspector.UIString("Debugger is disabled during a Timeline recording."), " ");
+        let stopRecordingLink = this._timelineRecordingWarningElement.appendChild(document.createElement("a"));
+        stopRecordingLink.textContent = WebInspector.UIString("Stop recording.");
+        stopRecordingLink.addEventListener("click", () => { WebInspector.timelineManager.stopCapturing(); });
+
         this._navigationBar = new WebInspector.NavigationBar;
         this.addSubview(this._navigationBar);
 
@@ -387,6 +397,29 @@
         this._addIssuesForSourceCode(resource);
     }
 
+    _timelineRecordingWillStart(event)
+    {
+        WebInspector.debuggerManager.startDisablingBreakpointsTemporarily();
+
+        if (WebInspector.debuggerManager.paused)
+            WebInspector.debuggerManager.resume();
+
+        this._debuggerBreakpointsButtonItem.enabled = false;
+        this._debuggerPauseResumeButtonItem.enabled = false;
+
+        this.contentView.element.insertBefore(this._timelineRecordingWarningElement, this.contentView.element.firstChild);
+    }
+
+    _timelineRecordingStopped(event)
+    {
+        WebInspector.debuggerManager.stopDisablingBreakpointsTemporarily();
+
+        this._debuggerBreakpointsButtonItem.enabled = true;
+        this._debuggerPauseResumeButtonItem.enabled = true;
+
+        this._timelineRecordingWarningElement.remove();
+    }
+
     _scriptAdded(event)
     {
         this._addScript(event.data.script);

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to