Bug 81669 – Web Inspector: Extract WebInspector.UIBreakpoint from WebInspector.Breakpoint.

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Vsevolod Vlasov]

Currently persisted breakpoint can potentially be used in zero to many uiSourceCodes.
Splitting WebInspector.Breakpoint into WebInspector.Breakpoint and WebInspector.UIBreakpoint and moving some logic to new objects will make code simpler and cleaner.

[Vsevolod Vlasov]

Created attachment 132838 [details]
Patch

[Pavel Feldman]

I guess Pavel P. should take a look first.

[Vsevolod Vlasov]

Created attachment 133015 [details]
Patch

[Pavel Podivilov]

Comment on attachment 133015 [details]
Patch

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

Could you please extract DPM-related part into a separate change?

> Source/WebCore/inspector/front-end/BreakpointManager.js:47
> +    this._uiBreakpointsByUILocation = {};

Please use UISourceCode as key type.

> Source/WebCore/inspector/front-end/BreakpointManager.js:-61
> -        if (!this._breakpoint(breakpoint.uiSourceCodeId, breakpoint.lineNumber))

It checks for duplicates, please keep it as is.

> Source/WebCore/inspector/front-end/BreakpointManager.js:92
> +        var breakpoints = this._breakpoints(uiSourceCode.id);

Could you iterate over uiBreakpointsByUILocation map instead?

> Source/WebCore/inspector/front-end/BreakpointManager.js:184
>      _addBreakpointToUI: function(breakpoint)

Please inline this method.

> Source/WebCore/inspector/front-end/BreakpointManager.js:220
> +        if (this._breakpoint(breakpoint.id, lineNumber))

breakpoint.id should be breakpoint.uiSourceCodeId

> Source/WebCore/inspector/front-end/BreakpointManager.js:434
> +        for (var id in this._uiBreakpointsByUILocation) {

You should just clear this map.

> Source/WebCore/inspector/front-end/BreakpointManager.js:489
> +    createUIBreakpoint: function(uiSourceCode)

Please make private.

> Source/WebCore/inspector/front-end/BreakpointManager.js:524
> +    return new WebInspector.Breakpoint(uiSourceCode.id, lineNumber, condition, enabled, !!uiSourceCode.url);

Please inline.

> Source/WebCore/inspector/front-end/BreakpointManager.js:550
> +    someFunction: function()

?

[Vsevolod Vlasov]

Created attachment 133076 [details]
Patch

[Pavel Podivilov]

Comment on attachment 133076 [details]
Patch

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

looks good.

> Source/WebCore/inspector/front-end/BreakpointManager.js:45
> +     * @type {Object.<WebInspector.UISourceCode, Object.<string,WebInspector.UIBreakpoint>>}

Please fix the annotation.

[Vsevolod Vlasov]

Committed r111595: <http://trac.webkit.org/changeset/111595>

[Kristóf Kosztyó]

Hi

The expected for the inspector/debugger/breakpoint-manager.html was wrong for every port except the chromium, so I rebaseline it at r111666. http://trac.webkit.org/changeset/111666