Expose Browser Plugin API in <browser> Tag Shim.

Expose Browser Plugin API in <browser> Tag Shim.

I'm a bit concerned about add{/remove}EventListener method overriding here though, because this overrides the default add{/remove}EventListener for the tag. I'm not sure if this is desired or not.

BUG=154304
TEST=Tested with calling <browser>.getProcessId().
Also tested with 'loadStart' event callback.


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

[sadrul, 2012-10-12 22:03:49]

drive-by

http://codereview.chromium.org/11139005/diff/1/chrome/renderer/resources/exte...
File chrome/renderer/resources/extensions/browser_tag.js (right):

http://codereview.chromium.org/11139005/diff/1/chrome/renderer/resources/exte...
chrome/renderer/resources/extensions/browser_tag.js:59: return
objectNode[apiMethod].apply(objectNode, arguments);
node[apiMethod] = objectNode[apiMethod].bind(objectNode);?

[Mihai Parparita -not on Chrome, 2012-10-12 22:19:41]

(another drive-by)

https://chromiumcodereview.appspot.com/11139005/diff/1/chrome/renderer/resour...
File chrome/renderer/resources/extensions/browser_tag.js (right):

https://chromiumcodereview.appspot.com/11139005/diff/1/chrome/renderer/resour...
chrome/renderer/resources/extensions/browser_tag.js:59: return
objectNode[apiMethod].apply(objectNode, arguments);
On 2012/10/12 22:03:49, sadrul wrote:
> node[apiMethod] = objectNode[apiMethod].bind(objectNode);?

 +1

https://chromiumcodereview.appspot.com/11139005/diff/1/chrome/renderer/resour...
chrome/renderer/resources/extensions/browser_tag.js:61: });
You just pass in this as an argument to forEach, and then "this.objectNode_"
will do the right thing inside (this is what the loop on line 75 does).

[Mihai Parparita -not on Chrome, 2012-10-12 22:21:13]

Also, it seems like it should be possible to add at least basic tests (for the
presence of these methods) to
http://code.google.com/searchframe#OAMlx_jo-ck/src/chrome/test/data/extension...

[lazyboy, 2012-10-12 23:21:16]

Thanks for the review!
Added test for checking if the api functions exist in the <browser> element.
PTAL.

https://chromiumcodereview.appspot.com/11139005/diff/1/chrome/renderer/resour...
File chrome/renderer/resources/extensions/browser_tag.js (right):

https://chromiumcodereview.appspot.com/11139005/diff/1/chrome/renderer/resour...
chrome/renderer/resources/extensions/browser_tag.js:59: return
objectNode[apiMethod].apply(objectNode, arguments);
On 2012/10/12 22:19:41, Mihai Parparita wrote:
> On 2012/10/12 22:03:49, sadrul wrote:
> > node[apiMethod] = objectNode[apiMethod].bind(objectNode);?
> 
>  +1

Cool, thanks.

https://chromiumcodereview.appspot.com/11139005/diff/1/chrome/renderer/resour...
chrome/renderer/resources/extensions/browser_tag.js:61: });
On 2012/10/12 22:19:41, Mihai Parparita wrote:
> You just pass in this as an argument to forEach, and then "this.objectNode_"
> will do the right thing inside (this is what the loop on line 75 does).
Yes, now doing this;
I didn't do it b/c I was accessing the object in the inner function of this.

[Mihai Parparita -not on Chrome, 2012-10-13 00:13:53]

LGTM

https://chromiumcodereview.appspot.com/11139005/diff/5002/chrome/renderer/res...
File chrome/renderer/resources/extensions/browser_tag.js (right):

https://chromiumcodereview.appspot.com/11139005/diff/5002/chrome/renderer/res...
chrome/renderer/resources/extensions/browser_tag.js:55: var objectNode =
this.objectNode_;
This is no longer needed.

[lazyboy, 2012-10-13 02:44:07]

Thanks for the review!

https://chromiumcodereview.appspot.com/11139005/diff/5002/chrome/renderer/res...
File chrome/renderer/resources/extensions/browser_tag.js (right):

https://chromiumcodereview.appspot.com/11139005/diff/5002/chrome/renderer/res...
chrome/renderer/resources/extensions/browser_tag.js:55: var objectNode =
this.objectNode_;
On 2012/10/13 00:13:53, Mihai Parparita wrote:
> This is no longer needed.

Removed.

[commit-bot: I haz the power, 2012-10-13 02:45:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/lazyboy@chromium.org/11139005/9001

[commit-bot: I haz the power, 2012-10-13 06:07:02]

Change committed as 161734