Extract the element implementation logic to function mods in <webview>.

Extract the element implementation logic to function mods in <webview>.
  function mods end with "$".

"element" inside <webview> is currently the plugin <object> element, i.e.
  the browser plugin. This CL makes it possible to provide a different
  implementation for the element. This CL also provides a stub
  using <iframe>. This will be used by the <webview> using OOPIF
  architecture CL: https://chromiumcodereview.appspot.com/972313002/

BUG=330264
Test=None, no visible changes.

Committed: https://crrev.com/df345d7ae7e0a524eaab0466573f7aad68c86a88
Cr-Commit-Position: refs/heads/master@{#333592}

[lazyboy, 2015-06-05 23:34:43]

lazyboy@chromium.org changed reviewers:
+ fsamuel@chromium.org

[lazyboy, 2015-06-05 23:34:43]

+Fady for initial review.

[Fady Samuel, 2015-06-05 23:45:53]

Mostly nits. Please pass this along to Paul. I'm happy with it if he is.

https://codereview.chromium.org/1165773004/diff/40001/extensions/renderer/res...
File extensions/renderer/resources/guest_view/guest_view_container.js (right):

https://codereview.chromium.org/1165773004/diff/40001/extensions/renderer/res...
extensions/renderer/resources/guest_view/guest_view_container.js:25:
privates(this).browserPluginElement = this.createInternalElement$();
rename browserPluginElement to internalElement all over.

https://codereview.chromium.org/1165773004/diff/40001/extensions/renderer/res...
File extensions/renderer/resources/guest_view/guest_view_iframe.js (right):

https://codereview.chromium.org/1165773004/diff/40001/extensions/renderer/res...
extensions/renderer/resources/guest_view/guest_view_iframe.js:17: var
internalElement = privates(view).browserPluginElement;
Renane browserPluginElement to internalElement.

https://codereview.chromium.org/1165773004/diff/40001/extensions/renderer/res...
extensions/renderer/resources/guest_view/guest_view_iframe.js:73:
window.console.log('GuestViewImpl.prototype.createImpl$');
Avoid spew.

[lazyboy, 2015-06-05 23:51:35]

lazyboy@chromium.org changed reviewers:
+ paulmeyer@chromium.org

[lazyboy, 2015-06-05 23:51:35]

Cool, addressed nits, adding Paul for review.

[lazyboy, 2015-06-05 23:52:38]

https://chromiumcodereview.appspot.com/1165773004/diff/40001/extensions/rende...
File extensions/renderer/resources/guest_view/guest_view_container.js (right):

https://chromiumcodereview.appspot.com/1165773004/diff/40001/extensions/rende...
extensions/renderer/resources/guest_view/guest_view_container.js:25:
privates(this).browserPluginElement = this.createInternalElement$();
On 2015/06/05 23:45:53, Fady Samuel wrote:
> rename browserPluginElement to internalElement all over.

Done.

https://chromiumcodereview.appspot.com/1165773004/diff/40001/extensions/rende...
File extensions/renderer/resources/guest_view/guest_view_iframe.js (right):

https://chromiumcodereview.appspot.com/1165773004/diff/40001/extensions/rende...
extensions/renderer/resources/guest_view/guest_view_iframe.js:17: var
internalElement = privates(view).browserPluginElement;
On 2015/06/05 23:45:53, Fady Samuel wrote:
> Renane browserPluginElement to internalElement.

Done.

https://chromiumcodereview.appspot.com/1165773004/diff/40001/extensions/rende...
extensions/renderer/resources/guest_view/guest_view_iframe.js:73:
window.console.log('GuestViewImpl.prototype.createImpl$');
On 2015/06/05 23:45:53, Fady Samuel wrote:
> Avoid spew.

Done.

[paulmeyer, 2015-06-08 13:37:38]

Nice work! LGTM, with a couple nits in one comment.

https://codereview.chromium.org/1165773004/diff/80001/extensions/renderer/res...
File extensions/renderer/resources/guest_view/web_view/web_view_iframe.js
(right):

https://codereview.chromium.org/1165773004/diff/80001/extensions/renderer/res...
extensions/renderer/resources/guest_view/web_view/web_view_iframe.js:5: // This
module implements provides OOPIF overrides for WebView (<webview>).
nit: choose one of 'implements' or 'provides'.

Also, in other files you refer to "--site-per-process overrides" instead of
"OOPIF overrides". You may want to change these for consistency. I'm fine with
either.

[Fady Samuel, 2015-06-08 14:06:23]

rubberstamp lgtm

[Fady Samuel, 2015-06-09 03:16:32]

Going back through this I found a bunch of nits.

https://codereview.chromium.org/1165773004/diff/100001/extensions/renderer/re...
File extensions/renderer/resources/guest_view/guest_view_container.js (left):

https://codereview.chromium.org/1165773004/diff/100001/extensions/renderer/re...
extensions/renderer/resources/guest_view/guest_view_container.js:125:
GuestViewContainer.prototype.handleBrowserPluginAttributeMutation =
nit: handleInternalElementAttributeMutation.

Change all references to browserPlugin to instead say internalElement.

https://codereview.chromium.org/1165773004/diff/100001/extensions/renderer/re...
File extensions/renderer/resources/guest_view/guest_view_container.js (right):

https://codereview.chromium.org/1165773004/diff/100001/extensions/renderer/re...
extensions/renderer/resources/guest_view/guest_view_container.js:125:
GuestViewContainer.prototype.onInternalInstanceID = function(
onInternalInstanceId

https://codereview.chromium.org/1165773004/diff/100001/extensions/renderer/re...
File extensions/renderer/resources/guest_view/web_view/web_view_iframe.js
(right):

https://codereview.chromium.org/1165773004/diff/100001/extensions/renderer/re...
extensions/renderer/resources/guest_view/web_view/web_view_iframe.js:26: var
generatedID = IdGenerator.GetNextId();
nit: generatedId

https://codereview.chromium.org/1165773004/diff/100001/extensions/renderer/re...
extensions/renderer/resources/guest_view/web_view/web_view_iframe.js:27: // We
do have a plugin to set our instance id, so do it here.
I wouldn't mention plugin here at all. Simply say that we generate an instance
Id for the iframe.

https://codereview.chromium.org/1165773004/diff/100001/extensions/renderer/re...
extensions/renderer/resources/guest_view/web_view/web_view_iframe.js:28:
this.onInternalInstanceID(generatedID);
nit: Id

[lazyboy, 2015-06-09 18:48:23]

https://chromiumcodereview.appspot.com/1165773004/diff/80001/extensions/rende...
File extensions/renderer/resources/guest_view/web_view/web_view_iframe.js
(right):

https://chromiumcodereview.appspot.com/1165773004/diff/80001/extensions/rende...
extensions/renderer/resources/guest_view/web_view/web_view_iframe.js:5: // This
module implements provides OOPIF overrides for WebView (<webview>).
On 2015/06/08 13:37:38, Paul Meyer wrote:
> nit: choose one of 'implements' or 'provides'.
Done.
> 
> Also, in other files you refer to "--site-per-process overrides" instead of
> "OOPIF overrides". You may want to change these for consistency. I'm fine with
> either.

Changed to --site-per-process overrides.
Done.

https://chromiumcodereview.appspot.com/1165773004/diff/100001/extensions/rend...
File extensions/renderer/resources/guest_view/guest_view_container.js (left):

https://chromiumcodereview.appspot.com/1165773004/diff/100001/extensions/rend...
extensions/renderer/resources/guest_view/guest_view_container.js:125:
GuestViewContainer.prototype.handleBrowserPluginAttributeMutation =
On 2015/06/09 03:16:31, Fady Samuel wrote:
> nit: handleInternalElementAttributeMutation.
> 
> Change all references to browserPlugin to instead say internalElement.

Done mostly except one local variable, where it helps to know that we are
creating a browser plugin <object>. This is the inside the default
createInternalElement$

https://chromiumcodereview.appspot.com/1165773004/diff/100001/extensions/rend...
File extensions/renderer/resources/guest_view/guest_view_container.js (right):

https://chromiumcodereview.appspot.com/1165773004/diff/100001/extensions/rend...
extensions/renderer/resources/guest_view/guest_view_container.js:125:
GuestViewContainer.prototype.onInternalInstanceID = function(
On 2015/06/09 03:16:31, Fady Samuel wrote:
> onInternalInstanceId

Done.

https://chromiumcodereview.appspot.com/1165773004/diff/100001/extensions/rend...
File extensions/renderer/resources/guest_view/web_view/web_view_iframe.js
(right):

https://chromiumcodereview.appspot.com/1165773004/diff/100001/extensions/rend...
extensions/renderer/resources/guest_view/web_view/web_view_iframe.js:26: var
generatedID = IdGenerator.GetNextId();
On 2015/06/09 03:16:32, Fady Samuel wrote:
> nit: generatedId

Done.

https://chromiumcodereview.appspot.com/1165773004/diff/100001/extensions/rend...
extensions/renderer/resources/guest_view/web_view/web_view_iframe.js:27: // We
do have a plugin to set our instance id, so do it here.
On 2015/06/09 03:16:32, Fady Samuel wrote:
> I wouldn't mention plugin here at all. Simply say that we generate an instance
> Id for the iframe.

Done.

https://chromiumcodereview.appspot.com/1165773004/diff/100001/extensions/rend...
extensions/renderer/resources/guest_view/web_view/web_view_iframe.js:28:
this.onInternalInstanceID(generatedID);
On 2015/06/09 03:16:32, Fady Samuel wrote:
> nit: Id

Done.

[lazyboy, 2015-06-09 19:31:32]

lazyboy@chromium.org changed reviewers:
+ rockot@chromium.org

[lazyboy, 2015-06-09 19:31:33]

rockot@ for extensions/*, specifically:
extensions/extensions.gypi
extensions/renderer/dispatcher.cc
extensions/renderer/resources/extensions_renderer_resources.grd

[Ken Rockot(use gerrit already), 2015-06-09 19:35:34]

On 2015/06/09 19:31:33, lazyboy wrote:
> rockot@ for extensions/*, specifically:
> extensions/extensions.gypi
> extensions/renderer/dispatcher.cc
> extensions/renderer/resources/extensions_renderer_resources.grd

these files lgtm

[Ken Rockot(use gerrit already), 2015-06-09 19:35:44]

oops, with nit

https://codereview.chromium.org/1165773004/diff/120001/extensions/renderer/re...
File extensions/renderer/resources/extensions_renderer_resources.grd (right):

https://codereview.chromium.org/1165773004/diff/120001/extensions/renderer/re...
extensions/renderer/resources/extensions_renderer_resources.grd:35: <include
name="IDR_GUEST_VIEW_IFRAME_CONTAINER_JS"
file="guest_view/guest_view_iframe_container.js" type="BINDATA" />
nit: alphabetical ordering seems off here? (shouldn't GUEST_VIEW_IFRAME_JS and
GUEST_VIEW_IFRAME_CONTAINER_JS come after GUEST_VIEW_EVENTS_JS?)

[lazyboy, 2015-06-09 19:41:27]

https://codereview.chromium.org/1165773004/diff/120001/extensions/renderer/re...
File extensions/renderer/resources/extensions_renderer_resources.grd (right):

https://codereview.chromium.org/1165773004/diff/120001/extensions/renderer/re...
extensions/renderer/resources/extensions_renderer_resources.grd:35: <include
name="IDR_GUEST_VIEW_IFRAME_CONTAINER_JS"
file="guest_view/guest_view_iframe_container.js" type="BINDATA" />
On 2015/06/09 19:35:44, Ken Rockot wrote:
> nit: alphabetical ordering seems off here? (shouldn't GUEST_VIEW_IFRAME_JS and
> GUEST_VIEW_IFRAME_CONTAINER_JS come after GUEST_VIEW_EVENTS_JS?)

Sorted them.
Done.

[lazyboy, 2015-06-09 21:48:16]

The CQ bit was checked by lazyboy@chromium.org

[lazyboy, 2015-06-09 21:48:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
fsamuel@chromium.org, paulmeyer@chromium.org, rockot@chromium.org
Link to the patchset:
https://chromiumcodereview.appspot.com/1165773004/#ps160001 (title: "sync
@tott")

[commit-bot: I haz the power, 2015-06-09 21:49:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1165773004/160001

[commit-bot: I haz the power, 2015-06-09 22:18:43]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-06-09 22:20:28]

Patchset 9 (id:??) landed as
https://crrev.com/df345d7ae7e0a524eaab0466573f7aad68c86a88
Cr-Commit-Position: refs/heads/master@{#333592}