Bug 55822 – Web Inspector: move breakpoints filtering to debugger model

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: Web Inspector (Deprecated)

[Pavel Podivilov]

Web Inspector: move breakpoints filtering to debugger model.

Breakpoints are currently restored by debugger model, so it's the best place to filter out breakpoints that don't have corresponding scripts.

[Pavel Podivilov]

Created attachment 84860 [details]
Patch.

[Pavel Feldman]

Comment on attachment 84860 [details]
Patch.

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

> Source/WebCore/inspector/front-end/DebuggerModel.js:333
> +                this.dispatchEventToListeners(WebInspector.DebuggerModel.Events.BreakpointAdded, breakpoint);

What happens if breakpoint is not resolved or is resolved on a different line? Should it get BreakpointAdded event? Do we have an "unresolved" breakpoint state?

[Pavel Podivilov]

Created attachment 85157 [details]
Patch.

[Pavel Podivilov]

(In reply to comment #2)
> (From update of attachment 84860 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=84860&action=review
> 
> > Source/WebCore/inspector/front-end/DebuggerModel.js:333
> > +                this.dispatchEventToListeners(WebInspector.DebuggerModel.Events.BreakpointAdded, breakpoint);
> 
> What happens if breakpoint is not resolved or is resolved on a different line? Should it get BreakpointAdded event? Do we have an "unresolved" breakpoint state?

Debugger model breakpoint has "locations" property, breakpoint is considered "unresolved" when breakpoint.locations array is empty.

Right after the navigation all breakpoints are hidden. BreakpointAdded event is dispatched in "parsed-script-source" event handler for breakpoints with matching url. BreakpointResolved is dispatched for already revealed breakpoints when new location is added to breakpoint.locations array.

[Pavel Feldman]

> Right after the navigation all breakpoints are hidden. BreakpointAdded event is dispatched in "parsed-script-source" event handler for breakpoints with matching url. BreakpointResolved is dispatched for already revealed breakpoints when new location is added to breakpoint.locations array.

Why do we distinguish between "added" breakpoints and "resolved" breakpoints? Why do add and hide all the breakpoints upon navigation?

[Pavel Podivilov]

(In reply to comment #5)
> > Right after the navigation all breakpoints are hidden. BreakpointAdded event is dispatched in "parsed-script-source" event handler for breakpoints with matching url. BreakpointResolved is dispatched for already revealed breakpoints when new location is added to breakpoint.locations array.
> 
> Why do we distinguish between "added" breakpoints and "resolved" breakpoints? Why do add and hide all the breakpoints upon navigation?

BreakpointAdded event is dispatched for all breakpoints with url that matches any script compiled in current debugging session (including unresolved breakpoints).
BreakpointResolved may be dispatched after BreakpointAdded when breakpoint is actually set by VM.
We hide and add breakpoints upon navigation to avoid UI cluttering.

[Pavel Feldman]

> BreakpointAdded event is dispatched for all breakpoints with url that matches any script compiled in current debugging session (including unresolved breakpoints).
> BreakpointResolved may be dispatched after BreakpointAdded when breakpoint is actually set by VM.
> We hide and add breakpoints upon navigation to avoid UI cluttering.

Do we show unresolved breakpoints though?

[Pavel Podivilov]

(In reply to comment #7)
> > BreakpointAdded event is dispatched for all breakpoints with url that matches any script compiled in current debugging session (including unresolved breakpoints).
> > BreakpointResolved may be dispatched after BreakpointAdded when breakpoint is actually set by VM.
> > We hide and add breakpoints upon navigation to avoid UI cluttering.
> 
> Do we show unresolved breakpoints though?

Yes. Breakpoint is shown by BreakpointAdded handler in UI. Later it may be moved on BreakpointResolved event.

[Pavel Feldman]

Comment on attachment 85157 [details]
Patch.

It is not clear why we need this "_hidden" state. Is there a notion of "hidden" breakpoint on the JSC / V8 level? DebuggerModel is just their mirror.

[Pavel Podivilov]

Hidden should be a property of presentation breakpoints.

*** This bug has been marked as a duplicate of bug 55823 ***