Changeset 172204 in webkit


Ignore:
Timestamp:
Aug 6, 2014 11:23:14 PM (10 years ago)
Author:
Brian Burg
Message:

Web Inspector: breakpoint resolved state should not depend on all breakpoints being enabled
https://bugs.webkit.org/show_bug.cgi?id=135517

Reviewed by Joseph Pecoraro.

Previously, Breakpoint.resolved returned false if all breakpoints were disabled, even if
the breakpoint had an associated SourceCode. This was a weird hack to make it easier to
style breakpoint widgets. This made it hard for other code to deal with resolved
breakpoints that were also disabled, or SourceCodeLocations that resolve and unresolve.
This patch removes that consideration and fixes style update code to manually check if all
breakpoints are being suppressed.

The code now enforces that a Breakpoint must have a SourceCode before it can be resolved.
(As a performance optimization when loading the initial frame tree, we sometimes we give
Breakpoints a SourceCode before the debugger officially says that the breakpoint has been
resolved. Thus, it's possible to be unresolved with a SourceCode, but not vice-versa.)

This patch also adds a few guards where we assumed a SourceCodeLocation had a SourceCode.

  • UserInterface/Controllers/DebuggerManager.js:

(WebInspector.DebuggerManager.prototype.set breakpointsEnabled): Remove spurious
ResolvedStateDidChange events.

(WebInspector.DebuggerManager.prototype.breakpointResolved): Set the breakpoint's SourceCode
if it has not been set already by DebuggerManager.associateBreakpointsWithSourceCode.

  • UserInterface/Models/Breakpoint.js:

(WebInspector.Breakpoint.prototype.get resolved):
(WebInspector.Breakpoint.prototype.set resolved.isSpecialBreakpoint):
(WebInspector.Breakpoint.prototype.set resolved): Add an assertion.

  • UserInterface/Models/SourceCodeLocation.js: Add guards for SourceCode.

(WebInspector.SourceCodeLocation.prototype.populateLiveDisplayLocationTooltip):

  • UserInterface/Views/BreakpointTreeElement.js: Account for DebuggerManager.breakpointsEnabled.

(WebInspector.BreakpointTreeElement):
(WebInspector.BreakpointTreeElement.prototype._updateStatus):

  • UserInterface/Views/ProbeSetDetailsSection.js:

(WebInspector.ProbeSetDetailsSection.prototype._updateLinkElement): Loosen the assertion.

  • UserInterface/Views/SourceCodeTextEditor.js: Account for DebuggerManager.breakpointsEnabled.

(WebInspector.SourceCodeTextEditor):
(WebInspector.SourceCodeTextEditor.prototype.close):
(WebInspector.SourceCodeTextEditor.prototype._breakpointStatusDidChange):
(WebInspector.SourceCodeTextEditor.prototype._breakpointsEnabledDidChange):
(WebInspector.SourceCodeTextEditor.prototype._updateBreakpointStatus):

  • UserInterface/Views/TextEditor.js: Account for DebuggerManager.breakpointsEnabled.
Location:
trunk/Source/WebInspectorUI
Files:
8 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebInspectorUI/ChangeLog

    r172203 r172204  
     12014-08-06  Brian J. Burg  <burg@cs.washington.edu>
     2
     3        Web Inspector: breakpoint resolved state should not depend on all breakpoints being enabled
     4        https://bugs.webkit.org/show_bug.cgi?id=135517
     5
     6        Reviewed by Joseph Pecoraro.
     7
     8        Previously, Breakpoint.resolved returned false if all breakpoints were disabled, even if
     9        the breakpoint had an associated SourceCode. This was a weird hack to make it easier to
     10        style breakpoint widgets. This made it hard for other code to deal with resolved
     11        breakpoints that were also disabled, or SourceCodeLocations that resolve and unresolve.
     12        This patch removes that consideration and fixes style update code to manually check if all
     13        breakpoints are being suppressed.
     14
     15        The code now enforces that a Breakpoint must have a SourceCode before it can be resolved.
     16        (As a performance optimization when loading the initial frame tree, we sometimes we give
     17        Breakpoints a SourceCode before the debugger officially says that the breakpoint has been
     18        resolved. Thus, it's possible to be unresolved with a SourceCode, but not vice-versa.)
     19
     20        This patch also adds a few guards where we assumed a SourceCodeLocation had a SourceCode.
     21
     22        * UserInterface/Controllers/DebuggerManager.js:
     23        (WebInspector.DebuggerManager.prototype.set breakpointsEnabled): Remove spurious
     24        ResolvedStateDidChange events.
     25
     26        (WebInspector.DebuggerManager.prototype.breakpointResolved): Set the breakpoint's SourceCode
     27        if it has not been set already by DebuggerManager.associateBreakpointsWithSourceCode.
     28
     29        * UserInterface/Models/Breakpoint.js:
     30        (WebInspector.Breakpoint.prototype.get resolved):
     31        (WebInspector.Breakpoint.prototype.set resolved.isSpecialBreakpoint):
     32        (WebInspector.Breakpoint.prototype.set resolved): Add an assertion.
     33        * UserInterface/Models/SourceCodeLocation.js: Add guards for !SourceCode.
     34        (WebInspector.SourceCodeLocation.prototype.populateLiveDisplayLocationTooltip):
     35        * UserInterface/Views/BreakpointTreeElement.js: Account for DebuggerManager.breakpointsEnabled.
     36        (WebInspector.BreakpointTreeElement):
     37        (WebInspector.BreakpointTreeElement.prototype._updateStatus):
     38        * UserInterface/Views/ProbeSetDetailsSection.js:
     39        (WebInspector.ProbeSetDetailsSection.prototype._updateLinkElement): Loosen the assertion.
     40        * UserInterface/Views/SourceCodeTextEditor.js: Account for DebuggerManager.breakpointsEnabled.
     41        (WebInspector.SourceCodeTextEditor):
     42        (WebInspector.SourceCodeTextEditor.prototype.close):
     43        (WebInspector.SourceCodeTextEditor.prototype._breakpointStatusDidChange):
     44        (WebInspector.SourceCodeTextEditor.prototype._breakpointsEnabledDidChange):
     45        (WebInspector.SourceCodeTextEditor.prototype._updateBreakpointStatus):
     46        * UserInterface/Views/TextEditor.js: Account for DebuggerManager.breakpointsEnabled.
     47
    1482014-08-06  Brian J. Burg  <burg@cs.washington.edu>
    249
  • trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js

    r171784 r172204  
    109109        this.dispatchEventToListeners(WebInspector.DebuggerManager.Event.BreakpointsEnabledDidChange);
    110110
    111         this._allExceptionsBreakpoint.dispatchEventToListeners(WebInspector.Breakpoint.Event.ResolvedStateDidChange);
    112         this._allUncaughtExceptionsBreakpoint.dispatchEventToListeners(WebInspector.Breakpoint.Event.ResolvedStateDidChange);
    113 
    114         for (var i = 0; i < this._breakpoints.length; ++i)
    115             this._breakpoints[i].dispatchEventToListeners(WebInspector.Breakpoint.Event.ResolvedStateDidChange);
    116 
    117111        DebuggerAgent.setBreakpointsActive(enabled);
    118112
     
    311305
    312306        console.assert(breakpoint.identifier === breakpointIdentifier);
     307
     308        if (!breakpoint.sourceCodeLocation.sourceCode) {
     309            var sourceCodeLocation = this._sourceCodeLocationFromPayload(location);
     310            breakpoint.sourceCodeLocation.sourceCode = sourceCodeLocation.sourceCode;
     311        }
    313312
    314313        breakpoint.resolved = true;
  • trunk/Source/WebInspectorUI/UserInterface/Models/Breakpoint.js

    r164543 r172204  
    118118    get resolved()
    119119    {
    120         return this._resolved && WebInspector.debuggerManager.breakpointsEnabled;
     120        return this._resolved;
    121121    },
    122122
     
    125125        if (this._resolved === resolved)
    126126            return;
     127
     128        function isSpecialBreakpoint()
     129        {
     130            return this._sourceCodeLocation.isEqual(new WebInspector.SourceCodeLocation(null, Infinity, Infinity));
     131        }
     132
     133        console.assert(!resolved || this._sourceCodeLocation.sourceCode || isSpecialBreakpoint.call(this), "Breakpoints must have a SourceCode to be resolved.", this);
    127134
    128135        this._resolved = resolved || false;
  • trunk/Source/WebInspectorUI/UserInterface/Models/SourceCodeLocation.js

    r171790 r172204  
    227227
    228228        this.addEventListener(WebInspector.SourceCodeLocation.Event.DisplayLocationChanged, function(event) {
    229             element.title = prefix + this.tooltipString();
     229            if (this.sourceCode)
     230                element.title = prefix + this.tooltipString();
    230231        }, this);
    231232    },
     
    259260
    260261        this.addEventListener(WebInspector.SourceCodeLocation.Event.DisplayLocationChanged, function(event) {
    261             updateDisplayString.call(this, currentDisplay, true);
     262            if (this.sourceCode)
     263                updateDisplayString.call(this, currentDisplay, true);
    262264        }, this);
    263265
  • trunk/Source/WebInspectorUI/UserInterface/Views/BreakpointTreeElement.js

    r164543 r172204  
    4141    this._listeners.register(breakpoint, WebInspector.Breakpoint.Event.AutoContinueDidChange, this._updateStatus);
    4242    this._listeners.register(breakpoint, WebInspector.Breakpoint.Event.ResolvedStateDidChange, this._updateStatus);
     43    this._listeners.register(WebInspector.debuggerManager, WebInspector.DebuggerManager.Event.BreakpointsEnabledDidChange, this._updateStatus);
    4344
    4445    this._listeners.register(WebInspector.probeManager, WebInspector.ProbeManager.Event.ProbeSetAdded, this._probeSetAdded);
     
    169170            this._statusImageElement.classList.remove(WebInspector.BreakpointTreeElement.StatusImageAutoContinueStyleClassName);
    170171
    171         if (this._breakpoint.resolved)
     172        if (this._breakpoint.resolved && WebInspector.debuggerManager.breakpointsEnabled)
    172173            this._statusImageElement.classList.add(WebInspector.BreakpointTreeElement.StatusImageResolvedStyleClassName);
    173174        else
  • trunk/Source/WebInspectorUI/UserInterface/Views/ProbeSetDetailsSection.js

    r171819 r172204  
    9292        var breakpoint = this._probeSet.breakpoint;
    9393        var titleElement = null;
    94         if (breakpoint.resolved)
     94        if (breakpoint.sourceCodeLocation.sourceCode)
    9595            titleElement = WebInspector.createSourceCodeLocationLink(breakpoint.sourceCodeLocation);
    9696        else {
    9797            // Fallback for when we can't create a live source link.
    98             console.assert(!breakpoint.sourceCodeLocation.sourceCode);
     98            console.assert(!breakpoint.resolved);
    9999
    100100            var location = breakpoint.sourceCodeLocation;
  • trunk/Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js

    r171784 r172204  
    4343
    4444    if (this._supportsDebugging) {
    45         WebInspector.Breakpoint.addEventListener(WebInspector.Breakpoint.Event.DisabledStateDidChange, this._updateBreakpointStatus, this);
    46         WebInspector.Breakpoint.addEventListener(WebInspector.Breakpoint.Event.AutoContinueDidChange, this._updateBreakpointStatus, this);
    47         WebInspector.Breakpoint.addEventListener(WebInspector.Breakpoint.Event.ResolvedStateDidChange, this._updateBreakpointStatus, this);
     45        WebInspector.Breakpoint.addEventListener(WebInspector.Breakpoint.Event.DisabledStateDidChange, this._breakpointStatusDidChange, this);
     46        WebInspector.Breakpoint.addEventListener(WebInspector.Breakpoint.Event.AutoContinueDidChange, this._breakpointStatusDidChange, this);
     47        WebInspector.Breakpoint.addEventListener(WebInspector.Breakpoint.Event.ResolvedStateDidChange, this._breakpointStatusDidChange, this);
    4848        WebInspector.Breakpoint.addEventListener(WebInspector.Breakpoint.Event.LocationDidChange, this._updateBreakpointLocation, this);
    4949
     50        WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.BreakpointsEnabledDidChange, this._breakpointsEnabledDidChange, this);
    5051        WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.BreakpointAdded, this._breakpointAdded, this);
    5152        WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.BreakpointRemoved, this._breakpointRemoved, this);
     
    115116    {
    116117        if (this._supportsDebugging) {
    117             WebInspector.Breakpoint.removeEventListener(WebInspector.Breakpoint.Event.DisabledStateDidChange, this._updateBreakpointStatus, this);
    118             WebInspector.Breakpoint.removeEventListener(WebInspector.Breakpoint.Event.AutoContinueDidChange, this._updateBreakpointStatus, this);
    119             WebInspector.Breakpoint.removeEventListener(WebInspector.Breakpoint.Event.ResolvedStateDidChange, this._updateBreakpointStatus, this);
     118            WebInspector.Breakpoint.removeEventListener(WebInspector.Breakpoint.Event.DisabledStateDidChange, this._breakpointStatusDidChange, this);
     119            WebInspector.Breakpoint.removeEventListener(WebInspector.Breakpoint.Event.AutoContinueDidChange, this._breakpointStatusDidChange, this);
     120            WebInspector.Breakpoint.removeEventListener(WebInspector.Breakpoint.Event.ResolvedStateDidChange, this._breakpointStatusDidChange, this);
    120121            WebInspector.Breakpoint.removeEventListener(WebInspector.Breakpoint.Event.LocationDidChange, this._updateBreakpointLocation, this);
    121122
     123            WebInspector.debuggerManager.removeEventListener(WebInspector.DebuggerManager.Event.BreakpointsEnabledDidChange, this._breakpointsEnabledDidChange, this);
    122124            WebInspector.debuggerManager.removeEventListener(WebInspector.DebuggerManager.Event.BreakpointAdded, this._breakpointAdded, this);
    123125            WebInspector.debuggerManager.removeEventListener(WebInspector.DebuggerManager.Event.BreakpointRemoved, this._breakpointRemoved, this);
     
    375377    },
    376378
    377     _updateBreakpointStatus: function(event)
     379    _breakpointStatusDidChange: function(event)
     380    {
     381        this._updateBreakpointStatus(event.target);
     382    },
     383
     384    _breakpointsEnabledDidChange: function()
    378385    {
    379386        console.assert(this._supportsDebugging);
    380387
     388        var breakpoints = WebInspector.debuggerManager.breakpointsForSourceCode(this._sourceCode);
     389        for (var breakpoint of breakpoints)
     390            this._updateBreakpointStatus(breakpoint);
     391    },
     392
     393    _updateBreakpointStatus: function(breakpoint)
     394    {
     395        console.assert(this._supportsDebugging);
     396
    381397        if (!this._contentPopulated)
    382398            return;
    383399
    384         var breakpoint = event.target;
    385400        if (!this._matchesBreakpoint(breakpoint))
    386401            return;
  • trunk/Source/WebInspectorUI/UserInterface/Views/TextEditor.js

    r164846 r172204  
    933933        }
    934934
     935        allResolved = allResolved && WebInspector.debuggerManager.breakpointsEnabled;
     936
    935937        function updateStyles()
    936938        {
Note: See TracChangeset for help on using the changeset viewer.