Implement chrome.extension.connectExternal and fix various API inconsistencies.

Implement chrome.extension.connectExternal and fix various API inconsistencies.

BUG=23583
BUG=17910
TEST=no

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=28565

[Aaron Boodman, 2009-10-08 18:09:57]

lgtm

Would prefer the test as an api test, though.

http://codereview.chromium.org/262016/diff/33/34
File chrome/browser/extensions/extension_browsertests_misc.cc (right):

http://codereview.chromium.org/262016/diff/33/34#newcode514
Line 514: // Tests that message passing between extensions and content scripts
works.
Would it be possible to implement this as an api test instead? I'd like to keep
this file from growing even larger.

http://codereview.chromium.org/262016/diff/33/47
File chrome/renderer/resources/renderer_extension_bindings.js (right):

http://codereview.chromium.org/262016/diff/33/47#newcode63
Line 63: var connectEvent = (sourceExtensionId == chromeHidden.extensionId ?
This is an important line. Maybe add a comment?

http://codereview.chromium.org/262016/diff/33/47#newcode71
Line 71: // TODO(mpcomplete): port.tab is obsolete. remove backwards compat at
1.0.
Should we have a convention for marking backward compat code, so that we can
grep for it later?

Some ideas:

// TODO(OLD_API)
// TODO(EXTENSIONS_DEPRECATED)
...

http://codereview.chromium.org/262016/diff/33/47#newcode120
Line 120: // TODO(mpcomplete): chrome.self is deprecated.  Remove it at 1.0.
Also I don't know that "1.0" is necessarily the time when we want to remove
these. I'd rather just mark them deprecated and punt on when to remove.

http://codereview.chromium.org/262016/diff/33/47#newcode130
Line 130: chrome.extension.connect = function(targetId_opt, connectInfo_opt) {
None of our other APIs currently support truly optional arguments. When
something is marked optional, it still has to be passed as |null|. So that makes
this API inconsistent.

That said, I think I prefer the aesthetics here better. I just think it might be
slightly confusing that "optional" means different things in different places.

[Matt Perry, 2009-10-08 21:15:30]

http://codereview.chromium.org/262016/diff/33/34
File chrome/browser/extensions/extension_browsertests_misc.cc (right):

http://codereview.chromium.org/262016/diff/33/34#newcode514
Line 514: // Tests that message passing between extensions and content scripts
works.
On 2009/10/08 18:09:57, Aaron Boodman wrote:
> Would it be possible to implement this as an api test instead? I'd like to
keep
> this file from growing even larger.

Done.

http://codereview.chromium.org/262016/diff/33/47
File chrome/renderer/resources/renderer_extension_bindings.js (right):

http://codereview.chromium.org/262016/diff/33/47#newcode63
Line 63: var connectEvent = (sourceExtensionId == chromeHidden.extensionId ?
On 2009/10/08 18:09:57, Aaron Boodman wrote:
> This is an important line. Maybe add a comment?

Done.

http://codereview.chromium.org/262016/diff/33/47#newcode71
Line 71: // TODO(mpcomplete): port.tab is obsolete. remove backwards compat at
1.0.
On 2009/10/08 18:09:57, Aaron Boodman wrote:
> Should we have a convention for marking backward compat code, so that we can
> grep for it later?
> 
> Some ideas:
> 
> // TODO(OLD_API)
> // TODO(EXTENSIONS_DEPRECATED)
> ...

Done.