Web Inspector: Ensure maximum accuracy while profiling
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Mar 2016 03:28:10 +0000 (03:28 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Mar 2016 03:28:10 +0000 (03:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=155809
<rdar://problem/25325035>

Patch by Joseph Pecoraro <pecoraro@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.

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

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js
Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js
Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js
Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.css
Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js

index 28e94e7..8409988 100644 (file)
@@ -1,3 +1,48 @@
+2016-03-28  Joseph Pecoraro  <pecoraro@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  <nvasilyev@apple.com>
 
         Web Inspector: Use font-variant-numeric: tabular-nums instead of -apple-system-monospaced-numbers
index e912d11..42b4b42 100644 (file)
Binary files a/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js and b/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js differ
index dcf068c..6f79d08 100644 (file)
@@ -71,6 +71,14 @@ WebInspector.DebuggerManager = class DebuggerManager extends WebInspector.Object
         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 @@ WebInspector.DebuggerManager = class DebuggerManager extends WebInspector.Object
         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 @@ WebInspector.DebuggerManager = class DebuggerManager extends WebInspector.Object
         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 @@ WebInspector.DebuggerManager = class DebuggerManager extends WebInspector.Object
         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 @@ WebInspector.DebuggerManager = class DebuggerManager extends WebInspector.Object
 
         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 @@ WebInspector.DebuggerManager = class DebuggerManager extends WebInspector.Object
         }
 
         if (breakpoint === this._allUncaughtExceptionsBreakpoint) {
-            if (!breakpoint.disabled)
+            if (!breakpoint.disabled && !this.breakpointsDisabledTemporarily)
                 this.breakpointsEnabled = true;
             this._allUncaughtExceptionsBreakpointEnabledSetting.value = !breakpoint.disabled;
             this._updateBreakOnExceptionsState();
index fa01312..6d55904 100644 (file)
@@ -158,6 +158,8 @@ WebInspector.TimelineManager = class TimelineManager extends WebInspector.Object
         if (!this._activeRecording || shouldCreateRecording)
             this._loadNewRecording();
 
+        this.dispatchEventToListeners(WebInspector.TimelineManager.Event.CapturingWillStart);
+
         this._activeRecording.start();
     }
 
@@ -866,6 +868,7 @@ WebInspector.TimelineManager = class TimelineManager extends WebInspector.Object
 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"
 };
index 1b2c081..f6dfebd 100644 (file)
@@ -23,7 +23,6 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-
 .sidebar > .panel.navigation.debugger > :matches(.content, .empty-content-placeholder) {
     top: 29px;
 }
     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;
 }
index 1a4e2f5..b7f630d 100644 (file)
@@ -46,6 +46,16 @@ WebInspector.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WebInspec
         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 @@ WebInspector.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WebInspec
         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);