Carry over user-agent override when opening new windows

Carry over user-agent override when opening new windows

RenderFrameImpl will try to set user-agen override based on the last
request on the frame, but when a new window is opened there is only a
blank last request where override has not been set.

This patch will set the user-agent override on the initial blank request
on a new frame based on the setting of opener.

BUG=599036
R=esprehn@chromium.org
R=avi@chromium.org

[Avi (use Gerrit), 2016-03-30 16:11:57]

This looks reasonable, but I'm not knowledgeable enough about the internals of
Blink. LGTM as a stamp, but get esprehn to approve too.

[Charlie Reis, 2016-03-30 19:16:12]

creis@chromium.org changed reviewers:
+ mnaganov@chromium.org

[Charlie Reis, 2016-03-30 19:16:12]

[Adding mnaganov, who I think is familiar with user agent override on Android.]

[mnaganov (inactive), 2016-03-30 23:53:32]

Looks reasonable, let's check that the tests pass.

[esprehn, 2016-03-31 05:09:10]

esprehn@chromium.org changed reviewers:
+ dcheng@chromium.org

[esprehn, 2016-03-31 05:09:11]

Honestly I have no idea what this DataSource code does. This seems kind of gross
to have to manually do this... why isn't the setting persisted in the same way
alert blocking and other things that are "per tab" are?

I'd prefer dcheng@ review this, I'll rubber stamp it if we can't find a decent
reviewer though since it does seem okayish.

[dcheng, 2016-03-31 05:24:18]

Also, test? =)

https://codereview.chromium.org/1841403002/diff/1/content/renderer/render_fra...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/1841403002/diff/1/content/renderer/render_fra...
content/renderer/render_frame_impl.cc:2629: blink::WebView* webview =
render_view_->webview();
This doesn't get called at all when creating a new window:
RenderViewImpl::createView() and RenderFrameImpl::CreateMainFrame() is where
that all happens.

[allan.jensen, 2016-04-04 10:58:16]

On 2016/03/31 at 05:24:18, dcheng wrote:
> Also, test? =)
> 
>
https://codereview.chromium.org/1841403002/diff/1/content/renderer/render_fra...
> File content/renderer/render_frame_impl.cc (right):
> 
>
https://codereview.chromium.org/1841403002/diff/1/content/renderer/render_fra...
> content/renderer/render_frame_impl.cc:2629: blink::WebView* webview =
render_view_->webview();
> This doesn't get called at all when creating a new window:
RenderViewImpl::createView() and RenderFrameImpl::CreateMainFrame() is where
that all happens.

The code-paths for creating a new window using a normal link and using a form
submit are different.

[allan.jensen, 2016-04-04 11:39:05]

Are there any documentation on how to write tests for chromium?

The patch was developed using the test attached to
https://bugreports.qt.io/browse/QTBUG-50876 ,
https://bugreports.qt.io/secure/attachment/54068/index.html

And hitting submit using CTRL+click.

[dcheng, 2016-04-04 19:26:25]

On 2016/04/04 at 10:58:16, allan.jensen wrote:
> On 2016/03/31 at 05:24:18, dcheng wrote:
> > Also, test? =)
> > 
> >
https://codereview.chromium.org/1841403002/diff/1/content/renderer/render_fra...
> > File content/renderer/render_frame_impl.cc (right):
> > 
> >
https://codereview.chromium.org/1841403002/diff/1/content/renderer/render_fra...
> > content/renderer/render_frame_impl.cc:2629: blink::WebView* webview =
render_view_->webview();
> > This doesn't get called at all when creating a new window:
RenderViewImpl::createView() and RenderFrameImpl::CreateMainFrame() is where
that all happens.
> 
> The code-paths for creating a new window using a normal link and using a form
submit are different.

They both go through ChromeClient::createWindow. This was all cleaned up a month
ago: https://codereview.chromium.org/1685003002/
This is never called when creating a new window now.

(Also, I don't see why there should be a distinction between a new window
created by a link vs a form submission)

[dcheng, 2016-04-04 19:33:22]

On 2016/04/04 at 11:39:05, allan.jensen wrote:
> Are there any documentation on how to write tests for chromium?
> 
> The patch was developed using the test attached to
https://bugreports.qt.io/browse/QTBUG-50876 ,
https://bugreports.qt.io/secure/attachment/54068/index.html
> 
> And hitting submit using CTRL+click.

As for testing, you could exercise this with a layout test or browser test: I
wouldn't write this as a unit test since we specifically need to test behavior
in //content.

Layout test:
Easy to compare expected results (just dump navigator.userAgent) but harder to
set up the test (you need to add plumbing for setting the user agent override
from the layout test runner: see
https://code.google.com/p/chromium/codesearch#chromium/src/components/test_ru...)

Browser test:
Easy to set the user agent override but checking the expected result is a bit
harder (the usual hack is to set the string to check as the title of the page,
and then use TitleWatcher from
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/tes...)