Webview attributes overhaul.

Webview attributes overhaul.

**********

This is an initial working reimplementation of webview attributes that will allow for many improvements to how webview handles attributes (and with much less code repetition!) in future CLs. In this CL:

- Created a new web_view_attributes.js file to contain the implementation of webview attributes.

- Reimplemented all webview attributes as objects that share common properties through inheritance, and can be operated on more consistently (this is important for automating the handling of attributes in later CLs).

- Constants are now used for attribute names consistently throughout webview.

Committed: https://crrev.com/884a5b43cba6a19b8d067fed99dc2906ed0ef656
Cr-Commit-Position: refs/heads/master@{#300980}

[paulmeyer, 2014-10-22 21:35:39]

paulmeyer@chromium.org changed reviewers:
+ fsamuel@chromium.org, rockot@chromium.org

[paulmeyer, 2014-10-22 21:35:40]

+fsamuel@ for guest_view/

+rockot@ for dispatcher.cc and extensions_renderer_resources.grd

[Fady Samuel, 2014-10-22 21:48:27]

https://codereview.chromium.org/663483004/diff/1/extensions/renderer/resource...
File extensions/renderer/resources/guest_view/web_view.js (right):

https://codereview.chromium.org/663483004/diff/1/extensions/renderer/resource...
extensions/renderer/resources/guest_view/web_view.js:19: var
BROWSER_PLUGIN_ATTRIBUTE_INTERNALINSTANCEID = 'internalinstanceid'
Move to WebViewConstants?

https://codereview.chromium.org/663483004/diff/1/extensions/renderer/resource...
extensions/renderer/resources/guest_view/web_view.js:256: // TODO(paulmeyer) is
this right?
Yes, we're not looking specifically for any value of autosize. Just that
autosize exists.

https://codereview.chromium.org/663483004/diff/1/extensions/renderer/resource...
extensions/renderer/resources/guest_view/web_view.js:481: // TODO(paulmeyer)
this looks wacky.
Please ask Istiaque about this. I'm not sure why the code looks like this.

https://codereview.chromium.org/663483004/diff/1/extensions/renderer/resource...
extensions/renderer/resources/guest_view/web_view.js:569: 'maxheight':
parseInt(this.attributes[WebViewConstants.ATTRIBUTE_MAXHEIGHT].
I feel like the key names should probably also match the attribute names, but
you can probably leave that for another CL.

[paulmeyer, 2014-10-23 21:32:20]

Patchset #2 (id:20001) has been deleted

[paulmeyer, 2014-10-23 21:39:56]

ptal

https://codereview.chromium.org/663483004/diff/1/extensions/renderer/resource...
File extensions/renderer/resources/guest_view/web_view.js (right):

https://codereview.chromium.org/663483004/diff/1/extensions/renderer/resource...
extensions/renderer/resources/guest_view/web_view.js:19: var
BROWSER_PLUGIN_ATTRIBUTE_INTERNALINSTANCEID = 'internalinstanceid'
On 2014/10/22 21:48:26, Fady Samuel wrote:
> Move to WebViewConstants?

Done.

https://codereview.chromium.org/663483004/diff/1/extensions/renderer/resource...
extensions/renderer/resources/guest_view/web_view.js:256: // TODO(paulmeyer) is
this right?
On 2014/10/22 21:48:26, Fady Samuel wrote:
> Yes, we're not looking specifically for any value of autosize. Just that
> autosize exists.

Acknowledged.

https://codereview.chromium.org/663483004/diff/1/extensions/renderer/resource...
extensions/renderer/resources/guest_view/web_view.js:481: // TODO(paulmeyer)
this looks wacky.
On 2014/10/22 21:48:26, Fady Samuel wrote:
> Please ask Istiaque about this. I'm not sure why the code looks like this.

This will get cleaned up when I unify the storage/access of attributes in a
later patch.

https://codereview.chromium.org/663483004/diff/1/extensions/renderer/resource...
extensions/renderer/resources/guest_view/web_view.js:569: 'maxheight':
parseInt(this.attributes[WebViewConstants.ATTRIBUTE_MAXHEIGHT].
On 2014/10/22 21:48:26, Fady Samuel wrote:
> I feel like the key names should probably also match the attribute names, but
> you can probably leave that for another CL.

I agree, and hopefully there will be a nice concise way of doing this once the
attribute objects are further along (later CL).

[Ken Rockot(use gerrit already), 2014-10-23 21:42:47]

general extensions bits lgtm

[Fady Samuel, 2014-10-23 21:50:14]

lgtm

[paulmeyer, 2014-10-23 21:58:28]

The CQ bit was checked by paulmeyer@chromium.org

[commit-bot: I haz the power, 2014-10-23 21:59:50]

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

[commit-bot: I haz the power, 2014-10-23 23:35:34]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2014-10-23 23:36:23]

Patchset 2 (id:??) landed as
https://crrev.com/884a5b43cba6a19b8d067fed99dc2906ed0ef656
Cr-Commit-Position: refs/heads/master@{#300980}