Bug 51738 – Web Inspector: refactoring: encapsulate lazy initialization of SourceFrame

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

[Pavel Podivilov]

Web Inspector: refactoring: encapsulate lazy initialization of SourceFrame

[Pavel Podivilov]

Created attachment 77667 [details]
Patch.

[Pavel Podivilov]

Comment on attachment 77667 [details]
Patch.

Extract content loading logic from SourceView and ScriptView to ContentProvider implementations.
Pass ContentProvider in SourceFrame constructor to allow SourceFrame manage it's lazy initialization.

[Pavel Feldman]

Comment on attachment 77667 [details]
Patch.

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

Could you please split this into smaller patches? It is really hard to follow the changes here and I feel there are bugs introduced.

> WebCore/inspector/front-end/ScriptView.js:34
> +    this.sourceFrame.addScript(script.sourceID, script.startingLine);

I did not know SourceFrame is sourceID aware. It probably should not be.

> WebCore/inspector/front-end/SourceFrame.js:869
> +WebInspector.ContentProvider = function()

SourceFrameContentProvider ?

> WebCore/inspector/front-end/SourceFrame.js:914
> +WebInspector.ResourceContentProvider = function(resource)

I thought the whole point of content provider was to isolate SourceFrame from Script / Resource. Now I see explicit resource usage in SourceFrame.js. This seems wrong.

[Pavel Podivilov]

Created attachment 77685 [details]
Patch.

[Pavel Podivilov]

Comment on attachment 77685 [details]
Patch.

All comments addressed.

[Pavel Feldman]

Comment on attachment 77685 [details]
Patch.

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

> WebCore/inspector/front-end/SourceFrame.js:835
> +        var sourceIDToStartingLineMap = this._contentProvider.sourceIDToStartingLineMap();

You will be able to use short for syntax if you make properties on Array's prototype non-enumerable:
Object.defineProperty(Array.prototype, "remove", { value: function() { ... }}); By default it'll create non-writable, non-enumerable, non-configurable property.

> WebCore/inspector/front-end/SourceFrame.js:874
> +    sourceIDToStartingLineMap: function()

This is a very strange content provider. I'd suggest to provide Scripts array here: you can then get both ids and starting lines from the scripts.

[Pavel Podivilov]

Created attachment 77716 [details]
Comments addressed.

[Pavel Podivilov]

Created attachment 78525 [details]
Rebase.

[Pavel Podivilov]

Committed r75937: <http://trac.webkit.org/changeset/75937>