[ios] Remove the openJavascript method from Tab.

[ios] Remove the openJavascript method from Tab.

Moves the implementation up into BrowserViewController, which is the only
caller.  BVC now processes the input JS lightly and then calls
WebState::ExecuteJavascript() directly.

BUG=681867
Review-Url: https://codereview.chromium.org/2646003002
Cr-Commit-Position: refs/heads/master@{#445068}
Committed: https://chromium.googlesource.com/chromium/src/+/746baec6650e544f81bc8a7def3b13da13fbef51

[rohitrao (ping after 24h), 2017-01-19 21:55:30]

The CQ bit was checked by rohitrao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 21:56:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[rohitrao (ping after 24h), 2017-01-19 21:59:09]

rohitrao@chromium.org changed reviewers:
+ eugenebut@google.com

[rohitrao (ping after 24h), 2017-01-19 21:59:09]

Can this code live somewhere other than BVC?  If we can get rid of the
percentEncoding sanitization, the BVC code basically becomes a straight
pass-through to WebState.

https://codereview.chromium.org/2646003002/diff/1/ios/chrome/browser/ui/brows...
File ios/chrome/browser/ui/browser_view_controller.mm (right):

https://codereview.chromium.org/2646003002/diff/1/ios/chrome/browser/ui/brows...
ios/chrome/browser/ui/browser_view_controller.mm:3791: javascript = [javascript
stringByRemovingPercentEncoding];
Is this sanitization important?

https://codereview.chromium.org/2646003002/diff/1/ios/chrome/browser/ui/brows...
ios/chrome/browser/ui/browser_view_controller.mm:3794:
web_state->ExecuteJavaScript(base::SysNSStringToUTF16(javascript));
This is a little dumb because we go from UTF8 -> NSString -> UTF16 -> NSString
=)  I can probably remove one of those conversions.

[commit-bot: I haz the power, 2017-01-19 22:06:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 22:06:32]

Dry run: This issue passed the CQ dry run.

[Eugene But (OOO till 7-30), 2017-01-19 22:20:33]

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

[Eugene But (OOO till 7-30), 2017-01-19 22:20:34]

lgtm

https://codereview.chromium.org/2646003002/diff/1/ios/chrome/browser/ui/brows...
File ios/chrome/browser/ui/browser_view_controller.mm (right):

https://codereview.chromium.org/2646003002/diff/1/ios/chrome/browser/ui/brows...
ios/chrome/browser/ui/browser_view_controller.mm:3791: javascript = [javascript
stringByRemovingPercentEncoding];
On 2017/01/19 21:59:09, rohitrao wrote:
> Is this sanitization important?
I don't know.

https://codereview.chromium.org/2646003002/diff/1/ios/chrome/browser/ui/brows...
ios/chrome/browser/ui/browser_view_controller.mm:3792: web::WebState* web_state
= [[_model currentTab] webState];
s/web_state/webState

https://codereview.chromium.org/2646003002/diff/1/ios/chrome/browser/ui/brows...
ios/chrome/browser/ui/browser_view_controller.mm:3794:
web_state->ExecuteJavaScript(base::SysNSStringToUTF16(javascript));
On 2017/01/19 21:59:09, rohitrao wrote:
> This is a little dumb because we go from UTF8 -> NSString -> UTF16 -> NSString
> =)  I can probably remove one of those conversions.
This is user supplied JS. I don't think they can type so much code to cause a
performance problem :)

[rohitrao (ping after 24h), 2017-01-20 16:11:25]

The CQ bit was checked by rohitrao@chromium.org

[rohitrao (ping after 24h), 2017-01-20 16:11:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
eugenebut@chromium.org
Link to the patchset: https://codereview.chromium.org/2646003002/#ps20001
(title: "Review.")

[commit-bot: I haz the power, 2017-01-20 16:11:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[rohitrao (ping after 24h), 2017-01-20 16:12:17]

https://codereview.chromium.org/2646003002/diff/1/ios/chrome/browser/ui/brows...
File ios/chrome/browser/ui/browser_view_controller.mm (right):

https://codereview.chromium.org/2646003002/diff/1/ios/chrome/browser/ui/brows...
ios/chrome/browser/ui/browser_view_controller.mm:3792: web::WebState* web_state
= [[_model currentTab] webState];
On 2017/01/19 22:20:34, Eugene But wrote:
> s/web_state/webState

Done.

https://codereview.chromium.org/2646003002/diff/1/ios/chrome/browser/ui/brows...
ios/chrome/browser/ui/browser_view_controller.mm:3794:
web_state->ExecuteJavaScript(base::SysNSStringToUTF16(javascript));
On 2017/01/19 22:20:33, Eugene But wrote:
> On 2017/01/19 21:59:09, rohitrao wrote:
> > This is a little dumb because we go from UTF8 -> NSString -> UTF16 ->
NSString
> > =)  I can probably remove one of those conversions.
> This is user supplied JS. I don't think they can type so much code to cause a
> performance problem :)

This is not actually true.  The JS supplied here comes from links in the page,
so they can probably be arbitrarily large.

[commit-bot: I haz the power, 2017-01-20 16:20:50]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484928685913780,
"parent_rev": "ea7ba6105cd287f37966e1bee2818d88aecba1d3", "commit_rev":
"746baec6650e544f81bc8a7def3b13da13fbef51"}

[commit-bot: I haz the power, 2017-01-20 16:21:24]

Description was changed from

==========
[ios] Remove the openJavascript method from Tab.

Moves the implementation up into BrowserViewController, which is the only
caller.  BVC now processes the input JS lightly and then calls
WebState::ExecuteJavascript() directly.

BUG=681867
==========

to

==========
[ios] Remove the openJavascript method from Tab.

Moves the implementation up into BrowserViewController, which is the only
caller.  BVC now processes the input JS lightly and then calls
WebState::ExecuteJavascript() directly.

BUG=681867

Review-Url: https://codereview.chromium.org/2646003002
Cr-Commit-Position: refs/heads/master@{#445068}
Committed:
https://chromium.googlesource.com/chromium/src/+/746baec6650e544f81bc8a7def3b...
==========

[commit-bot: I haz the power, 2017-01-20 16:21:25]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/746baec6650e544f81bc8a7def3b...

[Eugene But (OOO till 7-30), 2017-01-20 18:05:18]

https://codereview.chromium.org/2646003002/diff/1/ios/chrome/browser/ui/brows...
File ios/chrome/browser/ui/browser_view_controller.mm (right):

https://codereview.chromium.org/2646003002/diff/1/ios/chrome/browser/ui/brows...
ios/chrome/browser/ui/browser_view_controller.mm:3794:
web_state->ExecuteJavaScript(base::SysNSStringToUTF16(javascript));
On 2017/01/20 16:12:17, rohitrao wrote:
> On 2017/01/19 22:20:33, Eugene But wrote:
> > On 2017/01/19 21:59:09, rohitrao wrote:
> > > This is a little dumb because we go from UTF8 -> NSString -> UTF16 ->
> NSString
> > > =)  I can probably remove one of those conversions.
> > This is user supplied JS. I don't think they can type so much code to cause
a
> > performance problem :)
> 
> This is not actually true.  The JS supplied here comes from links in the page,
> so they can probably be arbitrarily large.
So maybe we should change WebState::ExecuteJavaScript to take NSString? I
created this method with sting16 signature to match content API, but in this
case it looks like diverging is reasonable.