<webview>: BrowserPlugin methods are not defined immediately after the shadow object is appended to…

<webview>: BrowserPlugin methods are not defined immediately after the shadow object is appended to the shadow root

BUG=169038
Test=manually


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=175897

[sadrul, 2013-01-09 18:51:23]

https://codereview.chromium.org/11820029/diff/1/chrome/renderer/resources/ext...
File chrome/renderer/resources/extensions/web_view.js (right):

https://codereview.chromium.org/11820029/diff/1/chrome/renderer/resources/ext...
chrome/renderer/resources/extensions/web_view.js:62: // the shadow object is
appended to the shadow root.
Interesting. Is this because of a race, or is that always the case (i.e. the
methods not being available at this point)?

[Fady Samuel, 2013-01-09 19:18:41]

https://codereview.chromium.org/11820029/diff/1/chrome/renderer/resources/ext...
File chrome/renderer/resources/extensions/web_view.js (right):

https://codereview.chromium.org/11820029/diff/1/chrome/renderer/resources/ext...
chrome/renderer/resources/extensions/web_view.js:62: // the shadow object is
appended to the shadow root.
On 2013/01/09 18:51:24, sadrul wrote:
> Interesting. Is this because of a race, or is that always the case (i.e. the
> methods not being available at this point)?

It works some of the time, so this is a race. This code is generally more robust
because it avoids accessing the innards of objectNode_ until the call is
actually made.

[sadrul, 2013-01-09 19:30:30]

LGTM

You might want to double check with someone with JS readability.

[Fady Samuel, 2013-01-09 19:36:36]

+arv for javascript style.

[arv (Not doing code reviews), 2013-01-09 19:40:01]

LGTM

https://codereview.chromium.org/11820029/diff/1/chrome/renderer/resources/ext...
File chrome/renderer/resources/extensions/web_view.js (right):

https://codereview.chromium.org/11820029/diff/1/chrome/renderer/resources/ext...
chrome/renderer/resources/extensions/web_view.js:64: var self = this;
Move self out of inner function?

var self = this;
WEB_VIEW_API_METHODS.forEach(function(apiMethod) {
  node[apiMethod] = function(var_args) {
    return self.objectNode_[apiMethod].apply(self.objectNode_, arguments);
  };
});

[Fady Samuel, 2013-01-09 19:49:25]

CQ'ing now.

https://codereview.chromium.org/11820029/diff/1/chrome/renderer/resources/ext...
File chrome/renderer/resources/extensions/web_view.js (right):

https://codereview.chromium.org/11820029/diff/1/chrome/renderer/resources/ext...
chrome/renderer/resources/extensions/web_view.js:64: var self = this;
On 2013/01/09 19:40:01, arv wrote:
> Move self out of inner function?
> 
> var self = this;
> WEB_VIEW_API_METHODS.forEach(function(apiMethod) {
>   node[apiMethod] = function(var_args) {
>     return self.objectNode_[apiMethod].apply(self.objectNode_, arguments);
>   };
> });

Done.

[commit-bot: I haz the power, 2013-01-09 19:54:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fsamuel@chromium.org/11820029/5001

[commit-bot: I haz the power, 2013-01-09 21:56:48]

Change committed as 175897