Bug 135517 – Web Inspector: breakpoint resolved state should not depend on all breakpoints being enabled

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Brian Burg]

The breakpoint model consults WebInspector.debuggerManager.breakpointsEnabled, which prevents this invariant from being true.

Breakpoint.resolved should not be used as a crutch for styling breakpoint widgets.

[Radar WebKit Bug Importer]

<rdar://problem/17887540>

[Brian Burg]

Even with fixing breakpoint.resolved digging into DebuggerManager, this is stil not true.

There are several places where we set breakpoint.resolved = true|false temporarily, but don't detach or reattach a sourceCode to the location.

[Brian Burg]

Created attachment 236153 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 236153 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=236153&action=review

r=me assuming those lines just need to be swapped. If not, then this probably needs another patch.

> Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js:310
> +            breakpoint.sourceCodeLocation.sourceCode = sourceCodeLocation.sourceCode;
> +            var sourceCodeLocation = this._sourceCodeLocationFromPayload(location);

Err, something seems aloof here. Did the lines get swapped or is "sourceCodeLocation" coming from somewhere else in line 309 making line 310 unnecessary?

> Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js:389
> +        for (breakpoint of breakpoints)

Needs "var breakpoint" or you'll leak a global.

[Brian Burg]

Comment on attachment 236153 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=236153&action=review

>> Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js:310
>> +            var sourceCodeLocation = this._sourceCodeLocationFromPayload(location);
> 
> Err, something seems aloof here. Did the lines get swapped or is "sourceCodeLocation" coming from somewhere else in line 309 making line 310 unnecessary?

oops, you are right. It used to be hoisted outside the if.

>> Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js:389
>> +        for (breakpoint of breakpoints)
> 
> Needs "var breakpoint" or you'll leak a global.

I <3 JavaScript. Maybe.

[Brian Burg]

Committed r172204: <http://trac.webkit.org/changeset/172204>