[ios] Protect JS messaging system from window.webkit object overriding.

[ios] Protect JS messaging system from overriding window.webkit object.

Any webpage can override window.webkit object (intentionally or by
accident). Since window.webkit is used for JS->Native messaging it should
be protected from overriding.

Before sending a message, delete |window.webkit| to restore the original
implementation.

BUG=611023
Committed: https://crrev.com/3eccc98ae84d9f47151d329b13fe2527f3f649ae
Cr-Commit-Position: refs/heads/master@{#393351}

[Eugene But (OOO till 7-30), 2016-05-11 21:27:06]

eugenebut@chromium.org changed reviewers:
+ jyquinn@chromium.org

[Jackie Quinn, 2016-05-11 22:05:01]

https://codereview.chromium.org/1973683002/diff/20001/ios/web/web_state/js/re...
File ios/web/web_state/js/resources/message.js (right):

https://codereview.chromium.org/1973683002/diff/20001/ios/web/web_state/js/re...
ios/web/web_state/js/resources/message.js:19: // Retain message handlers object
in case if a web page decide to override
Grammar nit: "in case a web page overrides"

https://codereview.chromium.org/1973683002/diff/20001/ios/web/web_state/js/re...
ios/web/web_state/js/resources/message.js:127: var messageHandlers =
window.webkit.messageHandlers ||
What happens if it gets overridden and there is a window.webkit.messageHandlers
object? Should we be using the override here, or should we always use the ivar?

[Eugene But (OOO till 7-30), 2016-05-11 22:34:55]

Description was changed from

==========
[ios] Protect JS messaging system from window.webkit object overriding.

Any webpage can override window.webkit object (intentionally or by
accident). Since window.webkit.messageHandlers is used for JS->Native
messaging it should be protected from overriding.

Original window.webkit.messageHandlers object is retained in ivar by
web page script.

BUG=611023
==========

to

==========
[ios] Protect JS messaging system from overriding window.webkit object.

Any webpage can override window.webkit object (intentionally or by
accident). Since window.webkit is used for JS->Native messaging it should
be protected from overriding.

Before sending a message, delete |window.webkit| to restore the original
implementation.

BUG=611023
==========

[Eugene But (OOO till 7-30), 2016-05-12 13:59:15]

PTAL

https://codereview.chromium.org/1973683002/diff/20001/ios/web/web_state/js/re...
File ios/web/web_state/js/resources/message.js (right):

https://codereview.chromium.org/1973683002/diff/20001/ios/web/web_state/js/re...
ios/web/web_state/js/resources/message.js:19: // Retain message handlers object
in case if a web page decide to override
On 2016/05/11 22:05:01, Jackie Quinn wrote:
> Grammar nit: "in case a web page overrides"

Acknowledged.

https://codereview.chromium.org/1973683002/diff/20001/ios/web/web_state/js/re...
ios/web/web_state/js/resources/message.js:127: var messageHandlers =
window.webkit.messageHandlers ||
On 2016/05/11 22:05:01, Jackie Quinn wrote:
> What happens if it gets overridden and there is a
window.webkit.messageHandlers
> object? Should we be using the override here, or should we always use the
ivar?
deleting |window.webkit| addresses all kinds of issues :)

[Jackie Quinn, 2016-05-12 17:33:58]

lgtm with additional commenting request

https://codereview.chromium.org/1973683002/diff/60001/ios/web/web_state/js/re...
File ios/web/web_state/js/resources/message.js (right):

https://codereview.chromium.org/1973683002/diff/60001/ios/web/web_state/js/re...
ios/web/web_state/js/resources/message.js:119: // If |window.webkit| has been
overridden deleting the object restores
Well that's nifty! Probably still good to have a comment in there explaining why
we need to use the original rather than a potential override.

[Eugene But (OOO till 7-30), 2016-05-12 18:44:41]

Thanks!

https://codereview.chromium.org/1973683002/diff/60001/ios/web/web_state/js/re...
File ios/web/web_state/js/resources/message.js (right):

https://codereview.chromium.org/1973683002/diff/60001/ios/web/web_state/js/re...
ios/web/web_state/js/resources/message.js:119: // If |window.webkit| has been
overridden deleting the object restores
On 2016/05/12 17:33:58, Jackie Quinn wrote:
> Well that's nifty! Probably still good to have a comment in there explaining
why
> we need to use the original rather than a potential override.
I did my best changing the comment :)

[Eugene But (OOO till 7-30), 2016-05-12 19:45:36]

The CQ bit was checked by eugenebut@chromium.org

[Eugene But (OOO till 7-30), 2016-05-12 19:45:37]

The patchset sent to the CQ was uploaded after l-g-t-m from jyquinn@chromium.org
Link to the patchset: https://codereview.chromium.org/1973683002/#ps80001
(title: "Updated comments")

[commit-bot: I haz the power, 2016-05-12 19:45:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1973683002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1973683002/80001

[commit-bot: I haz the power, 2016-05-12 21:01:19]

Description was changed from

==========
[ios] Protect JS messaging system from overriding window.webkit object.

Any webpage can override window.webkit object (intentionally or by
accident). Since window.webkit is used for JS->Native messaging it should
be protected from overriding.

Before sending a message, delete |window.webkit| to restore the original
implementation.

BUG=611023
==========

to

==========
[ios] Protect JS messaging system from overriding window.webkit object.

Any webpage can override window.webkit object (intentionally or by
accident). Since window.webkit is used for JS->Native messaging it should
be protected from overriding.

Before sending a message, delete |window.webkit| to restore the original
implementation.

BUG=611023
==========

[commit-bot: I haz the power, 2016-05-12 21:01:20]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-12 21:02:27]

Description was changed from

==========
[ios] Protect JS messaging system from overriding window.webkit object.

Any webpage can override window.webkit object (intentionally or by
accident). Since window.webkit is used for JS->Native messaging it should
be protected from overriding.

Before sending a message, delete |window.webkit| to restore the original
implementation.

BUG=611023
==========

to

==========
[ios] Protect JS messaging system from overriding window.webkit object.

Any webpage can override window.webkit object (intentionally or by
accident). Since window.webkit is used for JS->Native messaging it should
be protected from overriding.

Before sending a message, delete |window.webkit| to restore the original
implementation.

BUG=611023

Committed: https://crrev.com/3eccc98ae84d9f47151d329b13fe2527f3f649ae
Cr-Commit-Position: refs/heads/master@{#393351}
==========

[commit-bot: I haz the power, 2016-05-12 21:02:28]

Patchset 5 (id:??) landed as
https://crrev.com/3eccc98ae84d9f47151d329b13fe2527f3f649ae
Cr-Commit-Position: refs/heads/master@{#393351}