PlzNavigate: Support ResourceTiming API

PlzNavigate: Support ResourceTiming API

With browser-side-navigation, there are no trace of previous navigation
responses in the redirect chain. This CL forward these informations to
blink for making the Resource Timing API working.

BUG=641912
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/d563574ba6b1beec7cf5fcdb994ffb3cc49ec64d
Cr-Commit-Position: refs/heads/master@{#422797}

[arthursonzogni, 2016-09-06 08:42:20]

Description was changed from

==========
PlzNavigate: Support ResourceTiming API

With browser-side-navigation, there are no trace of previous navigation
responses in the redirect chain. This CL forward these informations to
blink for making the Resource Timing API working.

BUG=641912
==========

to

==========
PlzNavigate: Support ResourceTiming API

With browser-side-navigation, there are no trace of previous navigation
responses in the redirect chain. This CL forward these informations to
blink for making the Resource Timing API working.

BUG=641912
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[arthursonzogni, 2016-09-06 09:09:10]

arthursonzogni@chromium.org changed reviewers:
+ jochen@chromium.org, nasko@chromium.org

[arthursonzogni, 2016-09-06 09:09:10]

Hi jochen@ and nasko@,

I make the Resource Timing API working with PlzNavigate. Basically, I forward
all the ResourceResponses acquired during redirections to blink. They are used
by blink for their URLs, their headers and some of their timing attributes.

jochen@: please, can you review the 'blink' part ?
nasko@: please, can you take a look at the 'content' part ?

Thanks in advance!
Arthur

[arthursonzogni, 2016-09-06 10:00:48]

https://codereview.chromium.org/2316573002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/2316573002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:648: long long
encodedDataLength = 0;
Oops, I didn't see that I forgot to transmit the encodedDataLength. I will
submit a new patch for this.

[arthursonzogni, 2016-09-06 14:59:53]

I fixed the problem with encoded_data_length.

I will add an owner of media/ and CC ricea@ once the CL looks good to everybody.

[jochen (gone - plz use gerrit), 2016-09-06 15:03:15]

jochen@chromium.org changed reviewers:
+ japhet@chromium.org

[jochen (gone - plz use gerrit), 2016-09-06 15:03:16]

Nate is a better review for the blink side of this.

[arthursonzogni, 2016-09-06 15:17:36]

Hi Nate,

Could you please take a look at the blink part ?

Thanks!

[Nate Chapin, 2016-09-06 22:03:42]

https://codereview.chromium.org/2316573002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/network/ResourceRequest.h (right):

https://codereview.chromium.org/2316573002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/network/ResourceRequest.h:247: const
std::vector<WebURLResponse>& previousResponses() const { return m_responses; }
Conceptually, it seems like previousResponses() should be a property of
ResourceResponse instead of ResourceRequest. Is there a reason that's not
viable?

https://codereview.chromium.org/2316573002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/network/ResourceRequest.h:299:
std::vector<WebURLResponse> m_responses;
Here and elsewhere in this file: why WebURLResponse instead of ResourceResponse?
It's a bit strange for the layering that the core/ request class is holding the
web/ response wrapper instead core/ response class.

https://codereview.chromium.org/2316573002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/network/ResourceRequest.h:343:
std::vector<std::unique_ptr<CrossThreadResourceResponseData>> m_responses;
m_responses should only ever be populated for a main resource request, right?
CrossThreadResourceRequestData is only used for requests that can be made from
workers, which does not include main resources. Perhaps instead of including
m_responses in CrossThreadResourceRequestData, we can simply assert that we
never call ResourceRequest::copyData() when m_responses is non-empty?

[arthursonzogni, 2016-09-08 13:01:08]

arthursonzogni@chromium.org changed reviewers:
- jochen@chromium.org

[arthursonzogni, 2016-09-08 13:01:09]

Hi Nate,
Thanks for you comments!

I moved the redirect ResourceResponse into the final ResourceResponse. It looks
a little less weird.

Thanks,
Arthur

https://codereview.chromium.org/2316573002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/network/ResourceRequest.h (right):

https://codereview.chromium.org/2316573002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/network/ResourceRequest.h:247: const
std::vector<WebURLResponse>& previousResponses() const { return m_responses; }
On 2016/09/06 22:03:42, Nate Chapin wrote:
> Conceptually, it seems like previousResponses() should be a property of
> ResourceResponse instead of ResourceRequest. Is there a reason that's not
> viable?

I like the idea. There was no reason except it was easier. I will try it on the
next patch.

https://codereview.chromium.org/2316573002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/network/ResourceRequest.h:299:
std::vector<WebURLResponse> m_responses;
On 2016/09/06 22:03:42, Nate Chapin wrote:
> Here and elsewhere in this file: why WebURLResponse instead of
ResourceResponse?
> It's a bit strange for the layering that the core/ request class is holding
the
> web/ response wrapper instead core/ response class.

I will take care of doing this properly.

https://codereview.chromium.org/2316573002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/network/ResourceRequest.h:343:
std::vector<std::unique_ptr<CrossThreadResourceResponseData>> m_responses;
On 2016/09/06 22:03:42, Nate Chapin wrote:
> m_responses should only ever be populated for a main resource request, right?
> CrossThreadResourceRequestData is only used for requests that can be made from
> workers, which does not include main resources. Perhaps instead of including
> m_responses in CrossThreadResourceRequestData, we can simply assert that we
> never call ResourceRequest::copyData() when m_responses is non-empty?

You are right.
I was not aware of the use of this class.

[Nate Chapin, 2016-09-08 18:41:14]

blink parts are looking pretty good. Just a few smaller things.

https://codereview.chromium.org/2316573002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/2316573002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:618: double startTime =
monotonicallyIncreasingTime();
Any reason not to use:

double startTime = resource->resourceRequest().navigationStartTime() ?
resource->resourceRequest().navigationStartTime() :
monotonicallyIncreasingTime();

?

https://codereview.chromium.org/2316573002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/network/ResourceRequest.h (right):

https://codereview.chromium.org/2316573002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/network/ResourceRequest.h:93: void
setInitialURL(const KURL&);
Can we pull the initialURL off ResourceResponse, since it has the full redirect
chain? Less data duplication that way.

https://codereview.chromium.org/2316573002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/network/ResourceResponse.h (right):

https://codereview.chromium.org/2316573002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/network/ResourceResponse.h:306: const
std::vector<ResourceResponse>& redirectResponses() const { return
m_redirectResponses; }
WTF::Vector instead of std::vector in blink. Legacy!

[arthursonzogni, 2016-09-09 12:45:33]

Thank you Nate.

My last patch solves your comments.

https://codereview.chromium.org/2316573002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/2316573002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:618: double startTime =
monotonicallyIncreasingTime();
On 2016/09/08 18:41:13, Nate Chapin wrote:
> Any reason not to use:
> 
> double startTime = resource->resourceRequest().navigationStartTime() ?
> resource->resourceRequest().navigationStartTime() :
> monotonicallyIncreasingTime();
> 
> ?
There is no particular reasons. I will update it.

https://codereview.chromium.org/2316573002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/network/ResourceRequest.h (right):

https://codereview.chromium.org/2316573002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/network/ResourceRequest.h:93: void
setInitialURL(const KURL&);
On 2016/09/08 18:41:13, Nate Chapin wrote:
> Can we pull the initialURL off ResourceResponse, since it has the full
redirect
> chain? Less data duplication that way.

That is certainly one reason why I liked having the redirect responses inside
the request.

Currently, I use initialURL() inside PerformanceResourceTiming's constructor. I
certainly can add some method/arguments to PerformanceResourceTiming in order to
pass the initial url from the resource response.

My main concern is that I have some futur CLs that fix tests by simply use
request.initialURL() instead of request.url(). And there is no ResourceResponse
in the neighborhood.

I think I will remove initialURL() from ResourceRequest for the moment.

https://codereview.chromium.org/2316573002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/network/ResourceResponse.h (right):

https://codereview.chromium.org/2316573002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/network/ResourceResponse.h:306: const
std::vector<ResourceResponse>& redirectResponses() const { return
m_redirectResponses; }
On 2016/09/08 18:41:13, Nate Chapin wrote:
> WTF::Vector instead of std::vector in blink. Legacy!

Done.

[Nate Chapin, 2016-09-09 16:27:14]

blink LGTM

[nasko, 2016-09-09 21:17:34]

Hey Camille,
Would you be able to review this CL? I have a few others and some other issues
that have come up this/next week.
Thanks in advance!
Nasko

[clamy, 2016-09-12 15:46:32]

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

[clamy, 2016-09-12 15:46:33]

Thanks! The code in content/ looks mostly good, a few comments below.

https://codereview.chromium.org/2316573002/diff/60001/content/common/navigati...
File content/common/navigation_params.h (right):

https://codereview.chromium.org/2316573002/diff/60001/content/common/navigati...
content/common/navigation_params.h:249: // Any ResourceResponseInfo that occured
before the final response during
nit: how about "The ResourceResponseInfos received during server redirects."?
Also we may rename redirect_response_infos to mark that they are received during
redirects.

https://codereview.chromium.org/2316573002/diff/60001/content/renderer/render...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2316573002/diff/60001/content/renderer/render...
content/renderer/render_frame_impl.cc:5481: (common_params.navigation_start -
base::TimeTicks()).InSecondsF());
Didn't we have a convert to blink time helper function? If not, this would
probably be useful since we're doing the conversion a lot in RenderFrameImpl.

[arthursonzogni, 2016-09-17 00:52:56]

Patchset #5 (id:80001) has been deleted

[arthursonzogni, 2016-09-17 00:58:32]

Thanks for your comments!

https://codereview.chromium.org/2316573002/diff/60001/content/common/navigati...
File content/common/navigation_params.h (right):

https://codereview.chromium.org/2316573002/diff/60001/content/common/navigati...
content/common/navigation_params.h:249: // Any ResourceResponseInfo that occured
before the final response during
On 2016/09/12 15:46:33, clamy wrote:
> nit: how about "The ResourceResponseInfos received during server redirects."?
> Also we may rename redirect_response_infos to mark that they are received
during
> redirects.

Done.

https://codereview.chromium.org/2316573002/diff/60001/content/renderer/render...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2316573002/diff/60001/content/renderer/render...
content/renderer/render_frame_impl.cc:5481: (common_params.navigation_start -
base::TimeTicks()).InSecondsF());
On 2016/09/12 15:46:33, clamy wrote:
> Didn't we have a convert to blink time helper function? If not, this would
> probably be useful since we're doing the conversion a lot in RenderFrameImpl.

Yes you are right!

[clamy, 2016-09-19 12:10:47]

Thanks! Lgtm

https://codereview.chromium.org/2316573002/diff/100001/content/common/navigat...
File content/common/navigation_params.h (right):

https://codereview.chromium.org/2316573002/diff/100001/content/common/navigat...
content/common/navigation_params.h:249: // The ResourceResponseInfo received
during redirects.
nit:s/ResourceResponseInfo/ResourceResponseInfos (we can receive several of
them).

[arthursonzogni, 2016-09-19 13:15:36]

arthursonzogni@chromium.org changed reviewers:
+ jam@chromium.org, jochen@chromium.org

[arthursonzogni, 2016-09-19 13:15:37]

Thanks!

Now, I can add reviewers for the remaining files that aren't LGTM.
____

Hi everybody,

jochen@:
  -  third_party/WebKit/Source/platform/*

  I need a LGTM for the blink files that Nate doesn't own.
  Please, could you take a look? (or defer to someone else)

nasko@:
  -  content/common/frame_messages.h
  
  The set of redirect responses is passed to the renderer through the IPC.
  So I need you to check the line I added. Thanks in advance.
  
  
jam@:
  - media/blink/resource_multibuffer_data_provider.cc
  - media/blink/resource_multibuffer_data_provider.h

  I only removed one arguments to a function signature, and this argument was
unused.
  Please, could you LGTM theses files?

Thanks to everyone.

https://codereview.chromium.org/2316573002/diff/100001/content/common/navigat...
File content/common/navigation_params.h (right):

https://codereview.chromium.org/2316573002/diff/100001/content/common/navigat...
content/common/navigation_params.h:249: // The ResourceResponseInfo received
during redirects.
On 2016/09/19 12:10:47, clamy wrote:
> nit:s/ResourceResponseInfo/ResourceResponseInfos (we can receive several of
> them).

Done.

[jochen (gone - plz use gerrit), 2016-09-19 14:55:33]

lgtm with naming nit

https://codereview.chromium.org/2316573002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/exported/WebURLRequest.cpp (right):

https://codereview.chromium.org/2316573002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/exported/WebURLRequest.cpp:472: void
WebURLRequest::setNavigationStartTime(double navigationStart)
please add a unit to the variable name or method name

[jam, 2016-09-19 17:04:06]

lgtm

[arthursonzogni, 2016-09-20 11:02:34]

Thanks jochen@ and jam@!

https://codereview.chromium.org/2316573002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/exported/WebURLRequest.cpp (right):

https://codereview.chromium.org/2316573002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/exported/WebURLRequest.cpp:472: void
WebURLRequest::setNavigationStartTime(double navigationStart)
On 2016/09/19 14:55:33, jochen wrote:
> please add a unit to the variable name or method name

Done.
For consistency, maybe it should be done also for:
 - WebURLRequest::setUiStartTime
 - ResourceRequest::setTimeoutInterval
 - ResourceRequest::setNavigationStart
?

[jochen (gone - plz use gerrit), 2016-09-20 11:13:42]

yes please :)

[arthursonzogni, 2016-09-21 08:46:13]

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

[commit-bot: I haz the power, 2016-09-21 08:46:30]

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

[commit-bot: I haz the power, 2016-09-21 09:02:39]

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

[commit-bot: I haz the power, 2016-09-21 09:02:40]

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

[arthursonzogni, 2016-09-21 11:12:57]

Patchset #8 (id:160001) has been deleted

[arthursonzogni, 2016-09-21 11:13:11]

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

[commit-bot: I haz the power, 2016-09-21 11:13:27]

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

[commit-bot: I haz the power, 2016-09-21 11:27:10]

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

[commit-bot: I haz the power, 2016-09-21 11:27:11]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[arthursonzogni, 2016-09-21 12:34:45]

Patchset #8 (id:180001) has been deleted

[arthursonzogni, 2016-09-21 12:35:00]

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

[commit-bot: I haz the power, 2016-09-21 12:35:14]

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

[commit-bot: I haz the power, 2016-09-21 12:56:28]

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

[commit-bot: I haz the power, 2016-09-21 12:56:29]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[arthursonzogni, 2016-09-21 13:42:43]

Patchset #8 (id:200001) has been deleted

[arthursonzogni, 2016-09-21 13:42:58]

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

[commit-bot: I haz the power, 2016-09-21 13:43:18]

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

[commit-bot: I haz the power, 2016-09-21 14:15:37]

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

[commit-bot: I haz the power, 2016-09-21 14:15:39]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[arthursonzogni, 2016-09-21 14:43:23]

Patchset #8 (id:220001) has been deleted

[arthursonzogni, 2016-09-21 14:43:31]

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

[commit-bot: I haz the power, 2016-09-21 14:43:52]

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

[commit-bot: I haz the power, 2016-09-21 17:53:27]

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

[commit-bot: I haz the power, 2016-09-21 17:53:29]

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

[nasko, 2016-09-21 20:39:40]

LGTM and apologies for the delay.

https://codereview.chromium.org/2316573002/diff/240001/content/common/frame_m...
File content/common/frame_messages.h (right):

https://codereview.chromium.org/2316573002/diff/240001/content/common/frame_m...
content/common/frame_messages.h:375:
IPC_STRUCT_TRAITS_MEMBER(redirect_response_infos)
Very minor nit: I'd skip the "infos" as it is a bit strange to read. Matching it
to the existing "redirect_responses" makes a bit more sense to me.

[arthursonzogni, 2016-09-26 15:00:04]

Patchset #9 (id:250001) has been deleted

[arthursonzogni, 2016-09-27 13:38:49]

On 2016/09/21 20:39:40, nasko (slow) wrote:
> LGTM and apologies for the delay.
> 
>
https://codereview.chromium.org/2316573002/diff/240001/content/common/frame_m...
> File content/common/frame_messages.h (right):
> 
>
https://codereview.chromium.org/2316573002/diff/240001/content/common/frame_m...
> content/common/frame_messages.h:375:
> IPC_STRUCT_TRAITS_MEMBER(redirect_response_infos)
> Very minor nit: I'd skip the "infos" as it is a bit strange to read. Matching
it
> to the existing "redirect_responses" makes a bit more sense to me.

Thanks Nasko!

FYI, I am waiting that the following CL:
https://codereview.chromium.org/2368183004/
is applied before applying this one. There was a bug that violated one of the
assertion I put in the patch.

[arthursonzogni, 2016-10-04 12:07:26]

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

[commit-bot: I haz the power, 2016-10-04 12:07:37]

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

[arthursonzogni, 2016-10-04 12:16:32]

The CQ bit was unchecked by arthursonzogni@chromium.org

[arthursonzogni, 2016-10-04 12:26:53]

Patchset #10 (id:290001) has been deleted

[arthursonzogni, 2016-10-04 13:00:40]

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

[commit-bot: I haz the power, 2016-10-04 13:00:56]

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

[commit-bot: I haz the power, 2016-10-04 14:45:23]

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

[commit-bot: I haz the power, 2016-10-04 14:45:24]

Dry run: This issue passed the CQ dry run.

[arthursonzogni, 2016-10-04 15:03:05]

The CQ bit was checked by arthursonzogni@chromium.org

[arthursonzogni, 2016-10-04 15:03:05]

The patchset sent to the CQ was uploaded after l-g-t-m from japhet@chromium.org,
clamy@chromium.org, jochen@chromium.org, jam@chromium.org, nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/2316573002/#ps310001
(title: "Rebase")

[commit-bot: I haz the power, 2016-10-04 15:03:29]

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

[commit-bot: I haz the power, 2016-10-04 15:10:24]

Committed patchset #10 (id:310001)

[commit-bot: I haz the power, 2016-10-04 15:13:46]

Description was changed from

==========
PlzNavigate: Support ResourceTiming API

With browser-side-navigation, there are no trace of previous navigation
responses in the redirect chain. This CL forward these informations to
blink for making the Resource Timing API working.

BUG=641912
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
PlzNavigate: Support ResourceTiming API

With browser-side-navigation, there are no trace of previous navigation
responses in the redirect chain. This CL forward these informations to
blink for making the Resource Timing API working.

BUG=641912
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/d563574ba6b1beec7cf5fcdb994ffb3cc49ec64d
Cr-Commit-Position: refs/heads/master@{#422797}
==========

[commit-bot: I haz the power, 2016-10-04 15:13:48]

Patchset 10 (id:??) landed as
https://crrev.com/d563574ba6b1beec7cf5fcdb994ffb3cc49ec64d
Cr-Commit-Position: refs/heads/master@{#422797}