Bug 72388 – Web Inspector: [Extensions API] drop ExtensionSidebarPane.onUpdated, use callbacks instead

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

[Andrey Kosyakov]

- fire ExtensionSidebarPane.on{Hidden,Shown} for non-iframe content (experssions/objects);
- drop ExtensionsSidebarPane.onUpdated, provide callback for setObject()/setExpression() instead;
- split LayoutTests/inspector/extensions.html into extensions-panel.html & extensions-sidebar.html.

[Andrey Kosyakov]

Created attachment 115167 [details]
Patch

[Pavel Feldman]

Comment on attachment 115167 [details]
Patch

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

> Source/WebCore/inspector/front-end/ExtensionPanel.js:84
> +WebInspector.ExtensionNotifierView = function(id)

You should probably make this a superclass of the ExtensionView.

> Source/WebCore/inspector/front-end/ExtensionPanel.js:183
> +    setObject: function(object, title, callback)

Drive-by: you should start annotating your code.

> Source/WebCore/inspector/front-end/ExtensionPanel.js:215
> +    _makeObjectPropertiesView: function()

_createObjectPropertiesView

> Source/WebCore/inspector/front-end/ExtensionPanel.js:231
> +            callback("operation cancelled");

What does "operation cancelled" mean for the client? Annotating this callback would give me a clue.

> Source/WebCore/inspector/front-end/ExtensionServer.js:234
> +            var result = error ? this._status.E_FAILED(error) : this._status.OK();

aha. so non-empty is error string.

> Source/WebCore/inspector/front-end/View.js:173
> +    detach: function(force)

force -> overrideHideOnDetach

[Andrey Kosyakov]

Committed r100433: <http://trac.webkit.org/changeset/100433>

[Andrey Kosyakov]

(In reply to comment #2)

> > Source/WebCore/inspector/front-end/ExtensionPanel.js:84
> > +WebInspector.ExtensionNotifierView = function(id)
> 
> You should probably make this a superclass of the ExtensionView.

I've started with doing exactly this, then realized they won't even share a single method implementation due to checks for _frameIndex being present in willHide()/willShow().


> > Source/WebCore/inspector/front-end/ExtensionPanel.js:183
> > +    setObject: function(object, title, callback)
> 
> Drive-by: you should start annotating your code.

Done for entire ExtensionsPanel.js

> > Source/WebCore/inspector/front-end/ExtensionPanel.js:215
> > +    _makeObjectPropertiesView: function()
> 
> _createObjectPropertiesView

Done.

> > Source/WebCore/inspector/front-end/ExtensionPanel.js:231
> > +            callback("operation cancelled");
> 
> What does "operation cancelled" mean for the client? Annotating this callback would give me a clue.

Unfortunately it won't -- we can't specify parameter names in type annotation of callbacks.

> > Source/WebCore/inspector/front-end/View.js:173
> > +    detach: function(force)
> 
> force -> overrideHideOnDetach

Done.