Fix but that ignored startRenderer in WebContentsImpl.java

[Android] Remove startRender from evaluateJavaScript

Always start a renderer if one does not already exist.
The check is cheap so no need for a parameter to control it.

BUG=

Committed: https://crrev.com/7bf3d6dcbb9ab6aecd8998cfcdc0b565de4b45e1
Cr-Commit-Position: refs/heads/master@{#294985}

[boliu, 2014-09-13 00:22:30]

boliu@chromium.org changed reviewers:
+ michaelbai@chromium.org, tedchoc@chromium.org

[boliu, 2014-09-13 00:22:34]

This was a typo in https://codereview.chromium.org/481803004/

Ted for review.

This went into m38. Michael, you implemented this, right? Do you think this is
severe enough to warrant merging to m38?

[boliu, 2014-09-13 00:23:22]

On 2014/09/13 00:22:34, boliu wrote:
> This was a typo in https://codereview.chromium.org/481803004/

Ehh wrong link. It was typo in https://codereview.chromium.org/464393002/

> 
> Ted for review.
> 
> This went into m38. Michael, you implemented this, right? Do you think this is
> severe enough to warrant merging to m38?

[Ted C, 2014-09-13 00:40:45]

On 2014/09/13 00:23:22, boliu wrote:
> On 2014/09/13 00:22:34, boliu wrote:
> > This was a typo in https://codereview.chromium.org/481803004/
> 
> Ehh wrong link. It was typo in https://codereview.chromium.org/464393002/
> 
> > 
> > Ted for review.
> > 
> > This went into m38. Michael, you implemented this, right? Do you think this
is
> > severe enough to warrant merging to m38?

lgtm -- This looks like a little oversight from a refactoring (by akv @samsung).
I think we should try to merge this as it looks like the majority of callers
what the other behavior.

[Ted C, 2014-09-13 00:41:08]

On 2014/09/13 00:40:45, Ted C wrote:
> On 2014/09/13 00:23:22, boliu wrote:
> > On 2014/09/13 00:22:34, boliu wrote:
> > > This was a typo in https://codereview.chromium.org/481803004/
> > 
> > Ehh wrong link. It was typo in https://codereview.chromium.org/464393002/
> > 
> > > 
> > > Ted for review.
> > > 
> > > This went into m38. Michael, you implemented this, right? Do you think
this
> is
> > > severe enough to warrant merging to m38?
> 
> lgtm -- This looks like a little oversight from a refactoring (by akv
@samsung).
> I think we should try to merge this as it looks like the majority of callers
> what the other behavior.

I would file a bug and update the commit message though if we want to merge
this.

[boliu, 2014-09-13 00:48:40]

On 2014/09/13 00:41:08, Ted C wrote:
> On 2014/09/13 00:40:45, Ted C wrote:
> > On 2014/09/13 00:23:22, boliu wrote:
> > > On 2014/09/13 00:22:34, boliu wrote:
> > > > This was a typo in https://codereview.chromium.org/481803004/
> > > 
> > > Ehh wrong link. It was typo in https://codereview.chromium.org/464393002/
> > > 
> > > > 
> > > > Ted for review.
> > > > 
> > > > This went into m38. Michael, you implemented this, right? Do you think
> this
> > is
> > > > severe enough to warrant merging to m38?
> > 
> > lgtm -- This looks like a little oversight from a refactoring (by akv
> @samsung).
> > I think we should try to merge this as it looks like the majority of callers
> > what the other behavior.
> 
> I would file a bug and update the commit message though if we want to merge
> this.

Oh I thought this was only called by webview.

Looking at the implementation, passing true isn't anymore expensive, so I'm
wondering why do we even have this parameter. Why not just do the initialzation
work if needed and remove the parameter?

Michael wrote this I think. Thoughts?

[AKVT, 2014-09-13 03:12:44]

On 2014/09/13 00:48:40, boliu wrote:
> On 2014/09/13 00:41:08, Ted C wrote:
> > On 2014/09/13 00:40:45, Ted C wrote:
> > > On 2014/09/13 00:23:22, boliu wrote:
> > > > On 2014/09/13 00:22:34, boliu wrote:
> > > > > This was a typo in https://codereview.chromium.org/481803004/
> > > > 
> > > > Ehh wrong link. It was typo in
https://codereview.chromium.org/464393002/
> > > > 
> > > > > 
> > > > > Ted for review.
> > > > > 
> > > > > This went into m38. Michael, you implemented this, right? Do you think
> > this
> > > is
> > > > > severe enough to warrant merging to m38?
> > > 
> > > lgtm -- This looks like a little oversight from a refactoring (by akv
> > @samsung).
> > > I think we should try to merge this as it looks like the majority of
callers
> > > what the other behavior.
> > 
> > I would file a bug and update the commit message though if we want to merge
> > this.
> 
> Oh I thought this was only called by webview.
> 
> Looking at the implementation, passing true isn't anymore expensive, so I'm
> wondering why do we even have this parameter. Why not just do the
initialzation
> work if needed and remove the parameter?
> 
> Michael wrote this I think. Thoughts?

@boliu & Ted - Sorry it was a typo. I was intended to pass statrRenderer by
checking the flow inside WebContentsImpl, but missed in the end. All calls from
ContentViewCore which uses evaluateJavaScript() is passing false and which uses
evaluateJavaScriptEvenIfNotYetNavigated() is passing true. So we need to pass
the variable instead of true. Thnaks

[michaelbai, 2014-09-15 20:37:19]

lgtm

[michaelbai, 2014-09-15 20:47:33]

On 2014/09/15 20:37:19, michaelbai wrote:
> lgtm

IIRC, this is used to support WebView.evaluateJavascript() before renderer
started (WebView.loadUrl())

[boliu, 2014-09-15 20:54:06]

On 2014/09/15 20:47:33, michaelbai wrote:
> On 2014/09/15 20:37:19, michaelbai wrote:
> > lgtm
> 
> IIRC, this is used to support WebView.evaluateJavascript() before renderer
> started (WebView.loadUrl())

Right, but can we make that the default and remove this parameter?

All it does is this:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/we...

Looks pretty safe to call at any point imo?

[michaelbai, 2014-09-15 23:26:05]

On 2014/09/15 20:54:06, boliu wrote:
> On 2014/09/15 20:47:33, michaelbai wrote:
> > On 2014/09/15 20:37:19, michaelbai wrote:
> > > lgtm
> > 
> > IIRC, this is used to support WebView.evaluateJavascript() before renderer
> > started (WebView.loadUrl())
> 
> Right, but can we make that the default and remove this parameter?
> 
> All it does is this:
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/we...
> 
> Looks pretty safe to call at any point imo?


I don't know if there is other issue to remove the parameter, it seemes that
parameter could be removed, but you'd better full test it.

[boliu, 2014-09-16 00:16:34]

PTAL

Removed the paramter. Webview tests are already passing. I don't see why this
will break anything.

[boliu, 2014-09-16 00:18:02]

Also means nothing to merge to m38. Nothing was broken :D

[Ted C, 2014-09-16 00:39:58]

slgtm

[boliu, 2014-09-16 00:41:43]

The CQ bit was checked by boliu@chromium.org

[boliu, 2014-09-16 00:42:22]

On 2014/09/16 00:39:58, Ted C wrote:
> slgtm

Heh, you need the actual word with spaces around it now. I'll carry over your
previous approval

[commit-bot: I haz the power, 2014-09-16 00:48:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/570783002/20001

[commit-bot: I haz the power, 2014-09-16 02:28:01]

Committed patchset #2 (id:20001) as bbae57f9690db0dfcf99bc57da4e417ca4f900f6

[commit-bot: I haz the power, 2014-09-16 02:30:26]

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