Add a WebContents getter callback in ResourceRequestInfo

Add a WebContents getter callback in ResourceRequestInfo

This CL adds a method to ResourceRequestInfo that returns a callback that can
be used on the UI thread to get the WebContents associated with the request.It
is used in the SSLErrorHandler code, instead of using a RenderProcessHost ID
and a RenderFrameHost ID directly .This allows SSL error interstitials to work
with PlzNavigate enabled.

BUG = 504347

Committed: https://crrev.com/0d32d6d30fec43e2ad159193c65ace6636537b23
Cr-Commit-Position: refs/heads/master@{#361314}

[clamy, 2015-11-19 13:38:40]

Description was changed from

==========
Add a WebContents getter callback in ResourceRequestInfo

This CL adds a method to ResourceRequestInfo that returns a callback that can
be used on the UI thread to get the WebContents associated with the request.It
is used in the SSLErrorHandler code, instead of using a RenderProcessHost ID
and a RenderFrameHost ID directly .This allows SSL error interstitials to work
with PlzNavigate enabled.

BUG = 504347
==========

to

==========
Add a WebContents getter callback in ResourceRequestInfo

This CL adds a method to ResourceRequestInfo that returns a callback that can
be used on the UI thread to get the WebContents associated with the request.It
is used in the SSLErrorHandler code, instead of using a RenderProcessHost ID
and a RenderFrameHost ID directly .This allows SSL error interstitials to work
with PlzNavigate enabled.

BUG = 504347
==========

[clamy, 2015-11-19 13:38:40]

clamy@chromium.org changed reviewers:
+ davidben@chromium.org, nasko@chromium.org

[clamy, 2015-11-19 13:39:41]

@nasko, davidben: PTAL. This is the patch for having a callback in
ResourceRequestInfo. I'm also considering defining the callback type in
WebContents - wdyt?

[davidben, 2015-11-19 17:46:13]

Yeesh, this SSL code is a mess.

https://codereview.chromium.org/1459473003/diff/20001/chrome/browser/chrome_c...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/1459473003/diff/20001/chrome/browser/chrome_c...
chrome/browser/chrome_content_browser_client.cc:1961: web_contents_getter,
Rather than passing in this callback, it's probably better to pass in a
WebContents* and, in the case of no WebContents, have //content internally
switch to DENY if there is none. I'm not sure the NOTREACHED() in line 1983 is
valid anyway because IO stuff is destroyed a thread hop after UI stuff.

(Actually, I would even have put line 1971 inside //content, but it looks like
//android_webview screwed up. So we're probably stuck with that one for now.
Hrmf.)

Edit: Actually, I'm pretty sure it's impossible for there to be no WebContents
here. See comments in content/browser/ssl.

https://codereview.chromium.org/1459473003/diff/20001/chrome/browser/chrome_c...
File chrome/browser/chrome_content_browser_client.h (right):

https://codereview.chromium.org/1459473003/diff/20001/chrome/browser/chrome_c...
chrome/browser/chrome_content_browser_client.h:180: void AllowCertificateError(
You'll also need to change the other ContentBrowserClient implementations.

https://codereview.chromium.org/1459473003/diff/20001/content/browser/loader/...
File content/browser/loader/resource_request_info_impl.cc (right):

https://codereview.chromium.org/1459473003/diff/20001/content/browser/loader/...
content/browser/loader/resource_request_info_impl.cc:200:
ResourceRequestInfoImpl::GetWebContentsForRequest() {
This probably wants to comment to explain why there are two cases.

https://codereview.chromium.org/1459473003/diff/20001/content/browser/ssl/ssl...
File content/browser/ssl/ssl_error_handler.h (right):

https://codereview.chromium.org/1459473003/diff/20001/content/browser/ssl/ssl...
content/browser/ssl/ssl_error_handler.h:108: // Construct on the IO thread.
Really we should make this class UI thread only. (That might be better as a
follow-up? I can do that after this lands perhaps.)

In the meantime, perhaps if WebContentsGetterOnUI were moved to a parameter of
Dispatch? I think you don't even need to store it beyond the duration of the
PostTask.

(See ssl_policy.cc comment.)

https://codereview.chromium.org/1459473003/diff/20001/content/browser/ssl/ssl...
File content/browser/ssl/ssl_policy.cc (right):

https://codereview.chromium.org/1459473003/diff/20001/content/browser/ssl/ssl...
content/browser/ssl/ssl_policy.cc:235: handler->web_contents_getter(),
Wow, this code is all kinds of silly. This gets called from
SSLPolicy::OnCertError, which gets called from
SSLCertErrorHandler::OnDispatched... which is calling something off of
manager_... which is a per-WebContents object that has a pointer back to the
WebContents through the NavigationControllerImpl!

Not going to ask you to refactor it all here, but probably you can get the
WebContents itself without having to go through the callback.

https://codereview.chromium.org/1459473003/diff/20001/content/public/browser/...
File content/public/browser/resource_request_info.h (right):

https://codereview.chromium.org/1459473003/diff/20001/content/public/browser/...
content/public/browser/resource_request_info.h:9: #include "base/callback.h"
callback_forward.h

https://codereview.chromium.org/1459473003/diff/20001/content/public/browser/...
content/public/browser/resource_request_info.h:57: // is asscoaited with, or
nullptr if it does not exit. The callback should
asscoaited -> associated
exit -> exist

Probably also add:

 // Note: not all resource requests will be owned by a WebContents. For example,
requests made by a ServiceWorker.

(Aside: Really, this ought to be "is owned by", I think, but it might be too
early for that? Downloads still return WebContentses right now and I'd like to
disconnect that, once the ResourceLoader becomes more cleaned up.)

https://codereview.chromium.org/1459473003/diff/20001/content/public/browser/...
content/public/browser/resource_request_info.h:59: using WebContentsGetterOnUI =
base::Callback<WebContents*(void)>;
base::Callback<WebContents*(void)> isn't much longer. I think I might advocate
just using that, especially since it shouldn't appear in all that many type
signatures. I don't have strong preferences though.

https://codereview.chromium.org/1459473003/diff/20001/content/public/browser/...
content/public/browser/resource_request_info.h:65: // The child process unique
ID of the requestor.
// Use GetWebContentsForRequest() instead.
(Or is this too early?)

https://codereview.chromium.org/1459473003/diff/20001/content/public/browser/...
content/public/browser/resource_request_info.h:69: // or like-thing in the
renderer that the request gets routed to).
// Use GetWebContentsForRequest() instead.
(Or is this too early?)

https://codereview.chromium.org/1459473003/diff/20001/content/public/browser/...
content/public/browser/resource_request_info.h:82: // just use GetRouteID above.
// Use GetWebContentsForRequest() instead.
(Or is this too early?)

[davidben, 2015-11-19 17:46:47]

On 2015/11/19 17:46:13, davidben (slow) wrote:
> Yeesh, this SSL code is a mess.

(Not your CL making it a mess. I mean it's currently a mess.)

[davidben, 2015-11-19 17:50:25]

https://codereview.chromium.org/1459473003/diff/20001/content/browser/ssl/ssl...
File content/browser/ssl/ssl_policy.cc (right):

https://codereview.chromium.org/1459473003/diff/20001/content/browser/ssl/ssl...
content/browser/ssl/ssl_policy.cc:235: handler->web_contents_getter(),
Oh! And even SSLPolicy is a per-WebContents object! Though it doesn't have a
pointer back up. Yeah, this is all totally silly. I'm not sure why there is an
SSLPolicy / SSLManager split, TBH.

[clamy, 2015-11-20 13:50:43]

Thanks!

https://codereview.chromium.org/1459473003/diff/20001/chrome/browser/chrome_c...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/1459473003/diff/20001/chrome/browser/chrome_c...
chrome/browser/chrome_content_browser_client.cc:1961: web_contents_getter,
On 2015/11/19 17:46:13, davidben (slow) wrote:
> Rather than passing in this callback, it's probably better to pass in a
> WebContents* and, in the case of no WebContents, have //content internally
> switch to DENY if there is none. I'm not sure the NOTREACHED() in line 1983 is
> valid anyway because IO stuff is destroyed a thread hop after UI stuff.
> 
> (Actually, I would even have put line 1971 inside //content, but it looks like
> //android_webview screwed up. So we're probably stuck with that one for now.
> Hrmf.)
> 
> Edit: Actually, I'm pretty sure it's impossible for there to be no WebContents
> here. See comments in content/browser/ssl.

Since it's impossible not to have a WebContents here, I went with a DCHECK of
the WebContents.

https://codereview.chromium.org/1459473003/diff/20001/chrome/browser/chrome_c...
File chrome/browser/chrome_content_browser_client.h (right):

https://codereview.chromium.org/1459473003/diff/20001/chrome/browser/chrome_c...
chrome/browser/chrome_content_browser_client.h:180: void AllowCertificateError(
On 2015/11/19 17:46:13, davidben (slow) wrote:
> You'll also need to change the other ContentBrowserClient implementations.

Done on those I found in CodeSearch.

https://codereview.chromium.org/1459473003/diff/20001/content/browser/loader/...
File content/browser/loader/resource_request_info_impl.cc (right):

https://codereview.chromium.org/1459473003/diff/20001/content/browser/loader/...
content/browser/loader/resource_request_info_impl.cc:200:
ResourceRequestInfoImpl::GetWebContentsForRequest() {
On 2015/11/19 17:46:13, davidben (slow) wrote:
> This probably wants to comment to explain why there are two cases.

Done.

https://codereview.chromium.org/1459473003/diff/20001/content/browser/ssl/ssl...
File content/browser/ssl/ssl_error_handler.h (right):

https://codereview.chromium.org/1459473003/diff/20001/content/browser/ssl/ssl...
content/browser/ssl/ssl_error_handler.h:108: // Construct on the IO thread.
On 2015/11/19 17:46:13, davidben (slow) wrote:
> Really we should make this class UI thread only. (That might be better as a
> follow-up? I can do that after this lands perhaps.)
> 
> In the meantime, perhaps if WebContentsGetterOnUI were moved to a parameter of
> Dispatch? I think you don't even need to store it beyond the duration of the
> PostTask.
> 
> (See ssl_policy.cc comment.)

Done.

https://codereview.chromium.org/1459473003/diff/20001/content/browser/ssl/ssl...
File content/browser/ssl/ssl_policy.cc (right):

https://codereview.chromium.org/1459473003/diff/20001/content/browser/ssl/ssl...
content/browser/ssl/ssl_policy.cc:235: handler->web_contents_getter(),
On 2015/11/19 17:50:25, davidben (slow) wrote:
> Oh! And even SSLPolicy is a per-WebContents object! Though it doesn't have a
> pointer back up. Yeah, this is all totally silly. I'm not sure why there is an
> SSLPolicy / SSLManager split, TBH.

Done. I now expose the manager from the handler, and we can get the WebContents
through the NavigationController.

https://codereview.chromium.org/1459473003/diff/20001/content/public/browser/...
File content/public/browser/resource_request_info.h (right):

https://codereview.chromium.org/1459473003/diff/20001/content/public/browser/...
content/public/browser/resource_request_info.h:9: #include "base/callback.h"
On 2015/11/19 17:46:13, davidben (slow) wrote:
> callback_forward.h

Done.

https://codereview.chromium.org/1459473003/diff/20001/content/public/browser/...
content/public/browser/resource_request_info.h:57: // is asscoaited with, or
nullptr if it does not exit. The callback should
On 2015/11/19 17:46:13, davidben (slow) wrote:
> asscoaited -> associated
> exit -> exist
> 
> Probably also add:
> 
>  // Note: not all resource requests will be owned by a WebContents. For
example,
> requests made by a ServiceWorker.
> 
> (Aside: Really, this ought to be "is owned by", I think, but it might be too
> early for that? Downloads still return WebContentses right now and I'd like to
> disconnect that, once the ResourceLoader becomes more cleaned up.)

Done.

https://codereview.chromium.org/1459473003/diff/20001/content/public/browser/...
content/public/browser/resource_request_info.h:59: using WebContentsGetterOnUI =
base::Callback<WebContents*(void)>;
On 2015/11/19 17:46:13, davidben (slow) wrote:
> base::Callback<WebContents*(void)> isn't much longer. I think I might advocate
> just using that, especially since it shouldn't appear in all that many type
> signatures. I don't have strong preferences though.

Done.

https://codereview.chromium.org/1459473003/diff/20001/content/public/browser/...
content/public/browser/resource_request_info.h:65: // The child process unique
ID of the requestor.
On 2015/11/19 17:46:13, davidben (slow) wrote:
> // Use GetWebContentsForRequest() instead.
> (Or is this too early?)

Done.

https://codereview.chromium.org/1459473003/diff/20001/content/public/browser/...
content/public/browser/resource_request_info.h:69: // or like-thing in the
renderer that the request gets routed to).
On 2015/11/19 17:46:13, davidben (slow) wrote:
> // Use GetWebContentsForRequest() instead.
> (Or is this too early?)

Done. I don't think it's too early, most people are going to cast it to a
WebContents using the exact same code the callback uses internally.

https://codereview.chromium.org/1459473003/diff/20001/content/public/browser/...
content/public/browser/resource_request_info.h:82: // just use GetRouteID above.
On 2015/11/19 17:46:13, davidben (slow) wrote:
> // Use GetWebContentsForRequest() instead.
> (Or is this too early?)

Done.

[davidben, 2015-11-20 22:19:54]

lgtm. This actually isn't used outside of //content anymore, but it will be
later, so I don't have strong feelings as to whether you unexport it now and
re-export it afterwards or leave it as-is.

https://codereview.chromium.org/1459473003/diff/40001/content/browser/loader/...
File content/browser/loader/resource_request_info_impl.cc (right):

https://codereview.chromium.org/1459473003/diff/40001/content/browser/loader/...
content/browser/loader/resource_request_info_impl.cc:212: // In other cases, use
the RPH ID + RFH ID to get the WebContents.
Nit: This comments and above switch between using the full class name and
abbreviations. (I'd probably just spell it out as it isn't much longer.

https://codereview.chromium.org/1459473003/diff/40001/content/public/browser/...
File content/public/browser/resource_request_info.h (right):

https://codereview.chromium.org/1459473003/diff/40001/content/public/browser/...
content/public/browser/resource_request_info.h:11: #include
"content/public/browser/web_contents.h"
I think you can just forward-declare this.

[clamy, 2015-11-23 16:43:03]

clamy@chromium.org changed reviewers:
+ halliwell@chromium.org, mnaganov@chromium.org

[clamy, 2015-11-23 16:44:09]

@davidben: thanks! As per content/public rules, I unexposed the method from the
public interface.
@mnnaganov: PTAL at the changes in android_webview/
@halliwell: PTAL at the changes in chromecast/

https://codereview.chromium.org/1459473003/diff/40001/content/browser/loader/...
File content/browser/loader/resource_request_info_impl.cc (right):

https://codereview.chromium.org/1459473003/diff/40001/content/browser/loader/...
content/browser/loader/resource_request_info_impl.cc:212: // In other cases, use
the RPH ID + RFH ID to get the WebContents.
On 2015/11/20 22:19:53, davidben (slow) wrote:
> Nit: This comments and above switch between using the full class name and
> abbreviations. (I'd probably just spell it out as it isn't much longer.

Done.

https://codereview.chromium.org/1459473003/diff/40001/content/public/browser/...
File content/public/browser/resource_request_info.h (right):

https://codereview.chromium.org/1459473003/diff/40001/content/public/browser/...
content/public/browser/resource_request_info.h:11: #include
"content/public/browser/web_contents.h"
On 2015/11/20 22:19:53, davidben (slow) wrote:
> I think you can just forward-declare this.

Done.

https://codereview.chromium.org/1459473003/diff/60001/content/browser/ssl/ssl...
File content/browser/ssl/ssl_manager.h (right):

https://codereview.chromium.org/1459473003/diff/60001/content/browser/ssl/ssl...
content/browser/ssl/ssl_manager.h:62: static void
OnSSLCertificateSubresourceError(
This is called by WebSocketHost, which happens to live in
content/browser/renderer_host, where DEPS rules prevent us from including
web_contents.h/web_contents_impl.h and properly instantiating the callback.

[mnaganov (inactive), 2015-11-23 16:47:42]

android_webview/ LGTM

https://codereview.chromium.org/1459473003/diff/60001/android_webview/browser...
File android_webview/browser/aw_content_browser_client.cc (right):

https://codereview.chromium.org/1459473003/diff/60001/android_webview/browser...
android_webview/browser/aw_content_browser_client.cc:370:
AwContentsClientBridgeBase::FromWebContents(web_contents);
Thanks! Looks like we can now remove AwContentsClientBridgeBase::FromID, as this
was the only usage. I'll do that after you land the change.

[nasko, 2015-11-23 17:00:24]

Some drive-by nits.

https://codereview.chromium.org/1459473003/diff/60001/content/browser/loader/...
File content/browser/loader/resource_request_info_impl.cc (right):

https://codereview.chromium.org/1459473003/diff/60001/content/browser/loader/...
content/browser/loader/resource_request_info_impl.cc:217: NOTREACHED();
Since this is a macro and it isn't evident whether it produces any output or not
in Release build, I would put explicit {} to avoid the return being conditional
if the macro doesn't expand to a meaningful expression.

https://codereview.chromium.org/1459473003/diff/60001/content/browser/loader/...
File content/browser/loader/resource_request_info_impl.h (right):

https://codereview.chromium.org/1459473003/diff/60001/content/browser/loader/...
content/browser/loader/resource_request_info_impl.h:98: // is associated with,
or nullptr if it does not exist. The callback should
nit: The request itself can be from a context that isn't associated with
WebContents, so wording that "it does not exist" isn't quite correct.

https://codereview.chromium.org/1459473003/diff/60001/content/browser/loader/...
content/browser/loader/resource_request_info_impl.h:100: // Note: not all
resource requests will be owned by a WebContents. For
nit: Capitalize Not.

https://codereview.chromium.org/1459473003/diff/60001/content/browser/ssl/ssl...
File content/browser/ssl/ssl_error_handler.h (right):

https://codereview.chromium.org/1459473003/diff/60001/content/browser/ssl/ssl...
content/browser/ssl/ssl_error_handler.h:101: // Should only be accessed on the
UI thread.
Why not DCHECK the thread?

https://codereview.chromium.org/1459473003/diff/60001/content/browser/ssl/ssl...
File content/browser/ssl/ssl_manager.cc (right):

https://codereview.chromium.org/1459473003/diff/60001/content/browser/ssl/ssl...
content/browser/ssl/ssl_manager.cc:47: WebContents* GetWebContentsFromRFHID(int
render_process_id,
We have this method in two places. Can't we have it common somewhere? Maybe even
as a static method on WebContentsImpl?

[halliwell, 2015-11-23 17:22:05]

On 2015/11/23 17:00:24, nasko wrote:
> Some drive-by nits.
> 
>
https://codereview.chromium.org/1459473003/diff/60001/content/browser/loader/...
> File content/browser/loader/resource_request_info_impl.cc (right):
> 
>
https://codereview.chromium.org/1459473003/diff/60001/content/browser/loader/...
> content/browser/loader/resource_request_info_impl.cc:217: NOTREACHED();
> Since this is a macro and it isn't evident whether it produces any output or
not
> in Release build, I would put explicit {} to avoid the return being
conditional
> if the macro doesn't expand to a meaningful expression.
> 
>
https://codereview.chromium.org/1459473003/diff/60001/content/browser/loader/...
> File content/browser/loader/resource_request_info_impl.h (right):
> 
>
https://codereview.chromium.org/1459473003/diff/60001/content/browser/loader/...
> content/browser/loader/resource_request_info_impl.h:98: // is associated with,
> or nullptr if it does not exist. The callback should
> nit: The request itself can be from a context that isn't associated with
> WebContents, so wording that "it does not exist" isn't quite correct.
> 
>
https://codereview.chromium.org/1459473003/diff/60001/content/browser/loader/...
> content/browser/loader/resource_request_info_impl.h:100: // Note: not all
> resource requests will be owned by a WebContents. For
> nit: Capitalize Not.
> 
>
https://codereview.chromium.org/1459473003/diff/60001/content/browser/ssl/ssl...
> File content/browser/ssl/ssl_error_handler.h (right):
> 
>
https://codereview.chromium.org/1459473003/diff/60001/content/browser/ssl/ssl...
> content/browser/ssl/ssl_error_handler.h:101: // Should only be accessed on the
> UI thread.
> Why not DCHECK the thread?
> 
>
https://codereview.chromium.org/1459473003/diff/60001/content/browser/ssl/ssl...
> File content/browser/ssl/ssl_manager.cc (right):
> 
>
https://codereview.chromium.org/1459473003/diff/60001/content/browser/ssl/ssl...
> content/browser/ssl/ssl_manager.cc:47: WebContents*
GetWebContentsFromRFHID(int
> render_process_id,
> We have this method in two places. Can't we have it common somewhere? Maybe
even
> as a static method on WebContentsImpl?

chromecast/ lgtm

[clamy, 2015-11-23 17:56:54]

Thanks!

https://codereview.chromium.org/1459473003/diff/60001/content/browser/loader/...
File content/browser/loader/resource_request_info_impl.cc (right):

https://codereview.chromium.org/1459473003/diff/60001/content/browser/loader/...
content/browser/loader/resource_request_info_impl.cc:217: NOTREACHED();
On 2015/11/23 17:00:24, nasko wrote:
> Since this is a macro and it isn't evident whether it produces any output or
not
> in Release build, I would put explicit {} to avoid the return being
conditional
> if the macro doesn't expand to a meaningful expression.

Done.

https://codereview.chromium.org/1459473003/diff/60001/content/browser/loader/...
File content/browser/loader/resource_request_info_impl.h (right):

https://codereview.chromium.org/1459473003/diff/60001/content/browser/loader/...
content/browser/loader/resource_request_info_impl.h:98: // is associated with,
or nullptr if it does not exist. The callback should
On 2015/11/23 17:00:24, nasko wrote:
> nit: The request itself can be from a context that isn't associated with
> WebContents, so wording that "it does not exist" isn't quite correct.

Done.

https://codereview.chromium.org/1459473003/diff/60001/content/browser/loader/...
content/browser/loader/resource_request_info_impl.h:100: // Note: not all
resource requests will be owned by a WebContents. For
On 2015/11/23 17:00:24, nasko wrote:
> nit: Capitalize Not.

Done.

https://codereview.chromium.org/1459473003/diff/60001/content/browser/ssl/ssl...
File content/browser/ssl/ssl_error_handler.h (right):

https://codereview.chromium.org/1459473003/diff/60001/content/browser/ssl/ssl...
content/browser/ssl/ssl_error_handler.h:101: // Should only be accessed on the
UI thread.
On 2015/11/23 17:00:24, nasko wrote:
> Why not DCHECK the thread?

Done. I moved the implementation to the .cc file, since I'm not sure of the cost
of the DCHECK.

https://codereview.chromium.org/1459473003/diff/60001/content/browser/ssl/ssl...
File content/browser/ssl/ssl_manager.cc (right):

https://codereview.chromium.org/1459473003/diff/60001/content/browser/ssl/ssl...
content/browser/ssl/ssl_manager.cc:47: WebContents* GetWebContentsFromRFHID(int
render_process_id,
On 2015/11/23 17:00:24, nasko wrote:
> We have this method in two places. Can't we have it common somewhere? Maybe
even
> as a static method on WebContentsImpl?

Done.

[nasko, 2015-11-23 18:08:26]

https://codereview.chromium.org/1459473003/diff/80001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.h (right):

https://codereview.chromium.org/1459473003/diff/80001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.h:116: static WebContents*
FromRPHAndRFHIDs(int render_process_host_id,
How about FromRenderFrameHostId or FromFrameRoutingId or
FromRenderFrameHostRoutingId? It is always bound to the RenderProcessHost, so
adding RPH to the name doesn't add anything meaningful.

[clamy, 2015-11-23 18:28:08]

Thanks!

https://codereview.chromium.org/1459473003/diff/80001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.h (right):

https://codereview.chromium.org/1459473003/diff/80001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.h:116: static WebContents*
FromRPHAndRFHIDs(int render_process_host_id,
On 2015/11/23 18:08:26, nasko wrote:
> How about FromRenderFrameHostId or FromFrameRoutingId or
> FromRenderFrameHostRoutingId? It is always bound to the RenderProcessHost, so
> adding RPH to the name doesn't add anything meaningful.

Done.

[nasko, 2015-11-23 18:29:01]

LGTM

[clamy, 2015-11-23 18:35:20]

The CQ bit was checked by clamy@chromium.org

[clamy, 2015-11-23 18:35:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
davidben@chromium.org, mnaganov@chromium.org, halliwell@chromium.org
Link to the patchset: https://codereview.chromium.org/1459473003/#ps100001
(title: "Addressed Nasko's comment")

[commit-bot: I haz the power, 2015-11-23 18:35:59]

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

[commit-bot: I haz the power, 2015-11-23 20:30:38]

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

[commit-bot: I haz the power, 2015-11-23 20:30:39]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[clamy, 2015-11-24 10:38:33]

The CQ bit was checked by clamy@chromium.org

[commit-bot: I haz the power, 2015-11-24 10:39:15]

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

[commit-bot: I haz the power, 2015-11-24 11:16:33]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-11-24 11:17:26]

Patchset 6 (id:??) landed as
https://crrev.com/0d32d6d30fec43e2ad159193c65ace6636537b23
Cr-Commit-Position: refs/heads/master@{#361314}