extensions chrome.self.getViews()

extensions chrome.self.getViews():

extension toolstrips and background pages can now retrieve a list of the other "views" (window objects) in their extension and script them directly

R=mpComplete

Committed: http://src.chromium.org/viewvc/16222

[Matt Perry, 2009-05-15 21:50:48]

Good stuff!  Couple comments.

http://codereview.chromium.org/115419/diff/1/2
File chrome/renderer/extensions/extension_process_bindings.cc (right):

http://codereview.chromium.org/115419/diff/1/2#newcode76
Line 76: return (*Singleton<ExtensionViewContexts>::get())[extension_id];
careful: Singleton works globally by type.  If someone else happened to want a
Singleton of a map of string to list of contexts, you'd step on each other's
singleton.  And typedef doesn't make the type unique.

See event_bindings.cc for an example of how I get around this.

http://codereview.chromium.org/115419/diff/1/2#newcode82
Line 82: GURL url = renderview->webview()->GetMainFrame()->GetURL();
do we need to verify that the scheme is chrome-extension?

http://codereview.chromium.org/115419/diff/1/2#newcode85
Line 85: v8::Persistent<v8::Context>::New(v8::Context::GetCurrent());
nit: indent += 2 (indent 4 when wrapping)

http://codereview.chromium.org/115419/diff/1/3
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/115419/diff/1/3#newcode1540
Line 1540: frame->ExecuteScript(WebScriptSource(WebString::fromUTF8(
nit: I would move the ExecuteScript call into a static method in
extension_process_bindings.cc that you call from here, so that the ugliness is
encapsulated all in that file.

http://codereview.chromium.org/115419/diff/1/5
File chrome/renderer/resources/extension_process_bindings.js (right):

http://codereview.chromium.org/115419/diff/1/5#newcode509
Line 509: chrome.self._register = function() {
style guide says register_ for private vars.

http://codereview.chromium.org/115419/diff/1/5#newcode511
Line 511: window.addEventListener('unload', function() {
UnregisterExtension(extensionId); },
line length

[rafaelw, 2009-05-15 22:32:25]

http://codereview.chromium.org/115419/diff/1/2
File chrome/renderer/extensions/extension_process_bindings.cc (right):

http://codereview.chromium.org/115419/diff/1/2#newcode76
Line 76: return (*Singleton<ExtensionViewContexts>::get())[extension_id];
On 2009/05/15 21:50:49, Matt Perry wrote:
> careful: Singleton works globally by type.  If someone else happened to want a
> Singleton of a map of string to list of contexts, you'd step on each other's
> singleton.  And typedef doesn't make the type unique.
> 
> See event_bindings.cc for an example of how I get around this.

Done.

http://codereview.chromium.org/115419/diff/1/2#newcode82
Line 82: GURL url = renderview->webview()->GetMainFrame()->GetURL();
On 2009/05/15 21:50:49, Matt Perry wrote:
> do we need to verify that the scheme is chrome-extension?

Seems like a good idea. I don't think I should legitimately ever happen, so I've
made it a DCHECK.

http://codereview.chromium.org/115419/diff/1/2#newcode85
Line 85: v8::Persistent<v8::Context>::New(v8::Context::GetCurrent());
On 2009/05/15 21:50:49, Matt Perry wrote:
> nit: indent += 2 (indent 4 when wrapping)

Done.

http://codereview.chromium.org/115419/diff/1/3
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/115419/diff/1/3#newcode1540
Line 1540: frame->ExecuteScript(WebScriptSource(WebString::fromUTF8(
On 2009/05/15 21:50:49, Matt Perry wrote:
> nit: I would move the ExecuteScript call into a static method in
> extension_process_bindings.cc that you call from here, so that the ugliness is
> encapsulated all in that file.

Done.

http://codereview.chromium.org/115419/diff/1/5
File chrome/renderer/resources/extension_process_bindings.js (right):

http://codereview.chromium.org/115419/diff/1/5#newcode509
Line 509: chrome.self._register = function() {
On 2009/05/15 21:50:49, Matt Perry wrote:
> style guide says register_ for private vars.

Done.

http://codereview.chromium.org/115419/diff/1/5#newcode511
Line 511: window.addEventListener('unload', function() {
UnregisterExtension(extensionId); },
On 2009/05/15 21:50:49, Matt Perry wrote:
> line length

Done.

[Matt Perry, 2009-05-15 22:38:51]

lgtm