[Webkit-unassigned] [Bug 40425] Web Inspector: provide API for content scripts to interact with the inspector

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=40425


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #62568|review?                     |review+
               Flag|                            |




--- Comment #9 from Pavel Feldman <pfeldman at chromium.org>  2010-07-27 06:00:27 PST ---
(From update of attachment 62568)
WebCore/inspector/front-end/ExtensionAPI.js:34
 +  function EventSink(type)
Add comment claiming that it is not externally visible.

WebCore/inspector/front-end/ExtensionAPI.js:66
 +          for (listener in this._listeners)
Never fire while iterating over collection.

WebCore/inspector/front-end/ExtensionAPI.js:69
 +  };
no need for ;

WebCore/inspector/front-end/ExtensionAPI.js:92
 +      getAll: function(callback)
Lets start using JSDoc here.

WebCore/inspector/front-end/ExtensionAPI.js:123
 +      get: function(id)
I am not sure we need this accessor.

WebCore/inspector/front-end/ExtensionAPI.js:146
 +          extensionServer.sendRequest({ command: "createPanel", id: id, label: label, url: expandURL(pageURL), icon: expandURL(iconURL) }, callback && bind(callbackWrapper, this));
i'd start formatting these vertically.

WebCore/inspector/front-end/ExtensionAPI.js:174
 +      this.onSearch = new EventSink("panel-search-" + id);
Lets save onSearch until v5? :)

WebCore/inspector/front-end/ExtensionAPI.js:159
 +          var id = "extension-sidebar-" + extensionServer.nextObjectId;
make this a function

WebCore/inspector/front-end/ExtensionAPI.js:193
 +      setExpansion: function(state)
setExpanded(expanded)

WebCore/inspector/front-end/ExtensionPanel.js:33
 +      this.toolbarItemLabel = label;
Lets pull this property up and add it into the Panel's constructor.

WebCore/inspector/front-end/ExtensionPanel.js:36
 +      this.attach(); // Extension panel has to be attached so that extension can see it in DOM tree
You only need to call WebInspector.addPanelToolbarIcon here.

WebCore/inspector/front-end/ExtensionServer.js:125
 +          WebInspector.addPanelToolbarIcon(toolbarElement, panel, lastToolbarItem);
Ok.

WebCore/inspector/front-end/ExtensionServer.js:126
 +          WebInspector.panels[id] = panel;
This can kill all our code.

WebCore/inspector/front-end/ExtensionServer.js:134
 +              return this._status("E_NOTFOUND", message.panel);
Nice magic strings!

WebCore/inspector/front-end/ExtensionServer.js:226
 +          InspectorExtensionRegistry.getExtensions();
getExtensionsAsync

WebCore/inspector/front-end/ExtensionServer.js:235
 +      _addExtension: function(extension)
inline ?

WebCore/inspector/front-end/InjectedScript.js:246
 +          if (value != null && typeof value !== "string")
I'd do try / catch JSON.stringify.

WebCore/inspector/front-end/inspector.js:1208
 +          this.extensionServer.notifyObjectCreated("resource", WebInspector.extensionServer.convertResource(resource));
here and below, only use constants for notifyObjectCreated and postEvent. Or do postResetEvent.

WebCore/inspector/front-end/inspector.js:1606
 +  WebInspector.addExtensions = function(extensions)
You can define this method in ExtensionServer.js and made server's one private to the unit.

General comment on tests: you should add a test for extension-facing API that would enumerate and whitelist all the accessible properties (see window.* tests).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.