Add mojo interface to load hyphenation dictionaries on Android

Add mojo interface to load hyphenation dictionaries on Android

This patch adds a mojo interface to load hyphenation dictionaries on
Android.

Calls to Minikin using the loaded dictionary files will be in following
patches.

BUG=605840
Committed: https://crrev.com/30add38fedb8b9bd44b931aa3adba40d7c79b929
Cr-Commit-Position: refs/heads/master@{#405405}

[kojii, 2016-07-07 04:59:22]

Description was changed from

==========
WIP: mojo-hyphenation

BUG=
==========

to

==========
WIP: mojo-hyphenation

BUG=605840
==========

[kojii, 2016-07-07 05:27:05]

Description was changed from

==========
WIP: mojo-hyphenation

BUG=605840
==========

to

==========
Add mojo interface to load hyphenation dictionaries on Android

This patch adds a mojo interface to load hyphenation dictionaries on
Android.

Calls to Minikin using the loaded dictionary files will be in following
patches.

BUG=605840
==========

[kojii, 2016-07-07 13:25:10]

kojii@chromium.org changed reviewers:
+ eae@chromium.org, esprehn@chromium.org, holte@google.com, jochen@chromium.org,
rockot@chromium.org,

[kojii, 2016-07-07 13:25:11]

PTAL.

jochen@: build, components, content
rockot@: mojo (or in another CL is also fine, I'll rebase)
esprehn@: third_party/WebKit/public
eae@: third_party/WebKit/Source/core, platform
holte@: tools/metrics/histograms

[Ken Rockot(use gerrit already), 2016-07-07 13:37:41]

I don't think we should do this SyncCallRestrictions change. Having to do
it for every sync IPC just because of single-process mode will make the
list of exceptions quite long for no good reason. We should just disable
the restriction in single process mode. I'll put up a CL to do that.
On Jul 7, 2016 6:25 AM, <kojii@chromium.org> wrote:

> Reviewers: jochen, rockot, esprehn, eae, holte
> CL: https://codereview.chromium.org/2113933003/
>
> Message:
> PTAL.
>
> jochen@: build, components, content
> rockot@: mojo (or in another CL is also fine, I'll rebase)
> esprehn@: third_party/WebKit/public
> eae@: third_party/WebKit/Source/core, platform
> holte@: tools/metrics/histograms
>
>
> Description:
> Add mojo interface to load hyphenation dictionaries on Android
>
> This patch adds a mojo interface to load hyphenation dictionaries on
> Android.
>
> Calls to Minikin using the loaded dictionary files will be in following
> patches.
>
> BUG=605840
>
> Base URL: https://chromium.googlesource.com/chromium/src.git@master
>
> Affected files (+377, -19 lines):
> M build/config/features.gni
> A components/hyphenation/BUILD.gn
> A components/hyphenation/DEPS
> A components/hyphenation/OWNERS
> A components/hyphenation/hyphenation_export.h
> A components/hyphenation/hyphenation_impl.h
> A components/hyphenation/hyphenation_impl.cc
> M content/browser/BUILD.gn
> M content/browser/renderer_host/DEPS
> M content/browser/renderer_host/render_process_host_impl.cc
> M mojo/public/cpp/bindings/sync_call_restrictions.h
> A
> third_party/WebKit/LayoutTests/fast/text/hyphens/can-hyphenate-locale.html
> A
>
third_party/WebKit/LayoutTests/fast/text/hyphens/can-hyphenate-locale-expected.txt
> A
>
third_party/WebKit/LayoutTests/platform/android/fast/text/hyphens/can-hyphenate-locale-expected.txt
> A
>
third_party/WebKit/LayoutTests/platform/mac/fast/text/hyphens/can-hyphenate-locale-expected.txt
> M third_party/WebKit/Source/core/testing/Internals.h
> M third_party/WebKit/Source/core/testing/Internals.cpp
> M third_party/WebKit/Source/core/testing/Internals.idl
> M third_party/WebKit/Source/platform/BUILD.gn
> M third_party/WebKit/Source/platform/DEPS
> M third_party/WebKit/Source/platform/blink_platform.gypi
> D third_party/WebKit/Source/platform/text/android/HyphenationAndroid.cpp
> A third_party/WebKit/Source/platform/text/minikin/HyphenationMinikin.cpp
> M third_party/WebKit/public/BUILD.gn
> M third_party/WebKit/public/blink.gyp
> A third_party/WebKit/public/platform/modules/hyphenation/OWNERS
> A +
> third_party/WebKit/public/platform/modules/hyphenation/hyphenation.mojom
> M tools/metrics/histograms/histograms.xml
>
>
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Ken Rockot(use gerrit already), 2016-07-07 13:37:41]

I don't think we should do this SyncCallRestrictions change. Having to do
it for every sync IPC just because of single-process mode will make the
list of exceptions quite long for no good reason. We should just disable
the restriction in single process mode. I'll put up a CL to do that.
On Jul 7, 2016 6:25 AM, <kojii@chromium.org> wrote:

> Reviewers: jochen, rockot, esprehn, eae, holte
> CL: https://codereview.chromium.org/2113933003/
>
> Message:
> PTAL.
>
> jochen@: build, components, content
> rockot@: mojo (or in another CL is also fine, I'll rebase)
> esprehn@: third_party/WebKit/public
> eae@: third_party/WebKit/Source/core, platform
> holte@: tools/metrics/histograms
>
>
> Description:
> Add mojo interface to load hyphenation dictionaries on Android
>
> This patch adds a mojo interface to load hyphenation dictionaries on
> Android.
>
> Calls to Minikin using the loaded dictionary files will be in following
> patches.
>
> BUG=605840
>
> Base URL: https://chromium.googlesource.com/chromium/src.git@master
>
> Affected files (+377, -19 lines):
> M build/config/features.gni
> A components/hyphenation/BUILD.gn
> A components/hyphenation/DEPS
> A components/hyphenation/OWNERS
> A components/hyphenation/hyphenation_export.h
> A components/hyphenation/hyphenation_impl.h
> A components/hyphenation/hyphenation_impl.cc
> M content/browser/BUILD.gn
> M content/browser/renderer_host/DEPS
> M content/browser/renderer_host/render_process_host_impl.cc
> M mojo/public/cpp/bindings/sync_call_restrictions.h
> A
> third_party/WebKit/LayoutTests/fast/text/hyphens/can-hyphenate-locale.html
> A
>
third_party/WebKit/LayoutTests/fast/text/hyphens/can-hyphenate-locale-expected.txt
> A
>
third_party/WebKit/LayoutTests/platform/android/fast/text/hyphens/can-hyphenate-locale-expected.txt
> A
>
third_party/WebKit/LayoutTests/platform/mac/fast/text/hyphens/can-hyphenate-locale-expected.txt
> M third_party/WebKit/Source/core/testing/Internals.h
> M third_party/WebKit/Source/core/testing/Internals.cpp
> M third_party/WebKit/Source/core/testing/Internals.idl
> M third_party/WebKit/Source/platform/BUILD.gn
> M third_party/WebKit/Source/platform/DEPS
> M third_party/WebKit/Source/platform/blink_platform.gypi
> D third_party/WebKit/Source/platform/text/android/HyphenationAndroid.cpp
> A third_party/WebKit/Source/platform/text/minikin/HyphenationMinikin.cpp
> M third_party/WebKit/public/BUILD.gn
> M third_party/WebKit/public/blink.gyp
> A third_party/WebKit/public/platform/modules/hyphenation/OWNERS
> A +
> third_party/WebKit/public/platform/modules/hyphenation/hyphenation.mojom
> M tools/metrics/histograms/histograms.xml
>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Ken Rockot(use gerrit already), 2016-07-07 19:26:33]

rockot@chromium.org changed reviewers:
- rockot@chromium.org,

[Ken Rockot(use gerrit already), 2016-07-07 19:26:34]

Should be fine in single-process mode as of
https://crrev.com/be559ecbba727fb1a78cbff249195cad244375a6

[kojii, 2016-07-08 03:31:22]

On 2016/07/07 at 19:26:34, rockot wrote:
> Should be fine in single-process mode as of
https://crrev.com/be559ecbba727fb1a78cbff249195cad244375a6

Thank you Ken, the changes to sync_call_restriction.h was reverted in PS13.

Presubmit says I need your review to add '+mojo/public' to
components/hyphenation/DEPS and 
third_party/WebKit/Source/platform/DEPS, could you please review them?

You need LGTM from owners of depends-on paths in DEPS that were modified in this
CL:
    '+mojo/public',

Suggested missing target path OWNERS:
    rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-07-08 05:01:19]

lgtm

[kojii, 2016-07-08 06:14:58]

kojii@chromium.org changed reviewers:
+ holte@chromium.org
- holte@google.com

[jochen (gone - plz use gerrit), 2016-07-08 12:56:35]

why do you need a component, as opposed to just putting this into content?

[kojii, 2016-07-08 13:56:38]

On 2016/07/08 at 12:56:35, jochen wrote:
> why do you need a component, as opposed to just putting this into content?

That was the recommendation from platform-architecture-dev@
https://groups.google.com/a/chromium.org/d/msg/platform-architecture-dev/8Pxk...

[jam, 2016-07-09 03:04:52]

On 2016/07/08 13:56:38, kojii wrote:
> On 2016/07/08 at 12:56:35, jochen wrote:
> > why do you need a component, as opposed to just putting this into content?
> 
> That was the recommendation from platform-architecture-dev@
>
https://groups.google.com/a/chromium.org/d/msg/platform-architecture-dev/8Pxk...

+1, no need to add a new component for these two files.  You can put them in
content/browser/renderer_host/ and add yourself to the owners file there for
them.

In the medium-term, as we decompose content/browser, most likely we will end up
with a component related to sandbox escaping that we can move a bunch of methods
& interfaces to. This would be consumable by any content handler. At that point
these methods/interfaces would be combined together in components/foo/public
directory. But since we're not there yet, I don't think we want to start a
pattern where every new IPC from blink has a separate component.

[kojii, 2016-07-09 08:23:29]

On 2016/07/09 at 03:04:52, jam wrote:
> On 2016/07/08 13:56:38, kojii wrote:
> > On 2016/07/08 at 12:56:35, jochen wrote:
> > > why do you need a component, as opposed to just putting this into content?
> > 
> > That was the recommendation from platform-architecture-dev@
> >
https://groups.google.com/a/chromium.org/d/msg/platform-architecture-dev/8Pxk...
> 
> +1, no need to add a new component for these two files.  You can put them in
content/browser/renderer_host/ and add yourself to the owners file there for
them.
> 
> In the medium-term, as we decompose content/browser, most likely we will end
up with a component related to sandbox escaping that we can move a bunch of
methods & interfaces to. This would be consumable by any content handler. At
that point these methods/interfaces would be combined together in
components/foo/public directory. But since we're not there yet, I don't think we
want to start a pattern where every new IPC from blink has a separate component.

I don't have strong opinion here, either way is fine with me, but since I saw
different opinions here and platform-architecture-dev@, I'll ping there to see
if people there are happy.

Just to note that, if the decision factor is based on the size and the number of
files, it will increase a bit more in 3-6 months. The current code only supports
Android, which has dictionary files in OS. Layout team is still in the middle of
discussion how to support platforms without built-in dictionaries; i.e., Linux
and Windows, but most likely we will need to download dictionaries and unzip. We
may switch to different engines on some platforms, it's still TBD.

I checked our old implementation we once removed in 2012 (crbug.com/47083), that
had code in content/renderer, but it's probably to old to use as reference.

[kojii, 2016-07-12 06:57:08]

PTAL, moved to content/browser.

Thank you for your reviews and suggestions. IIUC, what
platform-architecture-dev@ was concerned about was content/renderer, not
content/browser, so now I understand they don't conflict with your suggestions.

[jam, 2016-07-12 14:51:10]

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

[jam, 2016-07-12 14:51:12]

you'll need to add a security reviewer from ipc/SECURITY_OWNERS for the new mojo
file

https://codereview.chromium.org/2113933003/diff/280001/content/browser/hyphen...
File content/browser/hyphenation/BUILD.gn (right):

https://codereview.chromium.org/2113933003/diff/280001/content/browser/hyphen...
content/browser/hyphenation/BUILD.gn:8: component("hyphenation") {
no need to create a separate component (i.e. .so/.dll in component build) for
subdirectories in content.

If you just want to make sure it doesn't grow dependencies, you can make this
directory's DEPS file exclude content. then just add it to
private_browser_sources in
https://cs.chromium.org/chromium/src/content/content_browser.gypi?rcl=0&l=353
which would also fix the gyp broken

sorry I know Brett mentioned otherwise in his email that you can use BUILD files
(along with DEPS) to limit the dependencies.  We're not yet at the point where
we have different source sets for small directories in content/ (or other top
level directories other than components) until gyp is removed. tight DEPS rules
will accomplish almost the same thing though.

https://codereview.chromium.org/2113933003/diff/280001/content/browser/hyphen...
File content/browser/hyphenation/hyphenation_impl.cc (right):

https://codereview.chromium.org/2113933003/diff/280001/content/browser/hyphen...
content/browser/hyphenation/hyphenation_impl.cc:45: // Only ISO lang code is
accepted.
don't we want to check this and early exit before the map is updated?

https://codereview.chromium.org/2113933003/diff/280001/content/browser/hyphen...
content/browser/hyphenation/hyphenation_impl.cc:49: #if
defined(HYPHENATION_DIR_HOME_FOR_TEST)
normally this is done by adding a command line flag instead of a build flag

https://codereview.chromium.org/2113933003/diff/280001/content/browser/hyphen...
content/browser/hyphenation/hyphenation_impl.cc:52: return file;
here as well, don't we only want to put valid paths in the map?

https://codereview.chromium.org/2113933003/diff/280001/content/browser/hyphen...
File content/browser/hyphenation/hyphenation_impl.h (right):

https://codereview.chromium.org/2113933003/diff/280001/content/browser/hyphen...
content/browser/hyphenation/hyphenation_impl.h:5: #ifndef
COMPONENTS_HYPHENATION_HYPHENATION_IMPL_H_
nit: update

https://codereview.chromium.org/2113933003/diff/280001/content/browser/hyphen...
content/browser/hyphenation/hyphenation_impl.h:9: #include <vector>
nit: not used

https://codereview.chromium.org/2113933003/diff/280001/content/browser/hyphen...
content/browser/hyphenation/hyphenation_impl.h:11: #include "base/files/file.h"
ditto

[eae, 2016-07-12 16:11:13]

LGTM for Blink

https://codereview.chromium.org/2113933003/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/text/minikin/HyphenationMinikin.cpp
(right):

https://codereview.chromium.org/2113933003/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/text/minikin/HyphenationMinikin.cpp:1: //
Copyright 2016 The Chromium Authors. All rights reserved.
I'd prefer Source/platform/text/hyphenation instead of
Source/platform/text/minikin

[kojii, 2016-07-12 17:32:20]

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

[kojii, 2016-07-12 17:32:21]

Thank you for your reviews, jam & eae.

> you'll need to add a security reviewer from ipc/SECURITY_OWNERS for the new
mojo file

+dcheng@, PTAL.

All done ("done" omitted") except the one below.

https://codereview.chromium.org/2113933003/diff/280001/content/browser/hyphen...
File content/browser/hyphenation/hyphenation_impl.cc (right):

https://codereview.chromium.org/2113933003/diff/280001/content/browser/hyphen...
content/browser/hyphenation/hyphenation_impl.cc:49: #if
defined(HYPHENATION_DIR_HOME_FOR_TEST)
On 2016/07/12 at 14:51:12, jam wrote:
> normally this is done by adding a command line flag instead of a build flag

This is really for dev/testing on other platforms since debug on Android isn't
easy. I think we don't want to put a code to read files on user's home directory
in the shipped code.

Removed this. With your comment, I found a local patch is more appropriate.
Thank you for that.

[jam, 2016-07-12 19:26:51]

lgtm, thanks

[dcheng, 2016-07-13 02:56:29]

https://codereview.chromium.org/2113933003/diff/300001/content/browser/hyphen...
File content/browser/hyphenation/hyphenation_impl.cc (right):

https://codereview.chromium.org/2113933003/diff/300001/content/browser/hyphen...
content/browser/hyphenation/hyphenation_impl.cc:30: // when std::map::emplace()
is fully supported on all configurations.
FWIW, the C++ style owners don't really like piecewise_construct for readability
reasons:
https://groups.google.com/a/chromium.org/d/msg/cxx/nCdjQqnouO4/l4mbaQiXEQAJ

https://codereview.chromium.org/2113933003/diff/300001/content/browser/hyphen...
content/browser/hyphenation/hyphenation_impl.cc:36: std::make_pair(locale,
std::move(file_on_stack)));
Slightly shorter to write:
const auto& inserted = cache.insert(std::make_pair(locale, base::File()))

since we don't use |file_on_stack| after this anyway.

https://codereview.chromium.org/2113933003/diff/300001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp
(right):

https://codereview.chromium.org/2113933003/diff/300001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp:33:
if (service.is_bound())
I have a question about this pattern:
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-mojo/40iQ0dKWfVM

https://codereview.chromium.org/2113933003/diff/300001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/modules/hyphenation/hyphenation.mojom
(right):

https://codereview.chromium.org/2113933003/diff/300001/third_party/WebKit/pub...
third_party/WebKit/public/platform/modules/hyphenation/hyphenation.mojom:11:
OpenDictionary(string locale) => (handle? hyphenation_dictionary_handle);
Do we need the ? It seems like we always pass back a handle (even if it's
sometimes invalid). Basically, I'm not sure what ? means when you use it with a
handle =)

[kojii, 2016-07-13 05:02:29]

https://codereview.chromium.org/2113933003/diff/300001/content/browser/hyphen...
File content/browser/hyphenation/hyphenation_impl.cc (right):

https://codereview.chromium.org/2113933003/diff/300001/content/browser/hyphen...
content/browser/hyphenation/hyphenation_impl.cc:30: // when std::map::emplace()
is fully supported on all configurations.
On 2016/07/13 at 02:56:28, dcheng wrote:
> FWIW, the C++ style owners don't really like piecewise_construct for
readability reasons:
https://groups.google.com/a/chromium.org/d/msg/cxx/nCdjQqnouO4/l4mbaQiXEQAJ

Oh, thanks for the info. Removed the TODO. I originally used it in PS1, only to
figure out it fails on some compiler.

https://codereview.chromium.org/2113933003/diff/300001/content/browser/hyphen...
content/browser/hyphenation/hyphenation_impl.cc:36: std::make_pair(locale,
std::move(file_on_stack)));
On 2016/07/13 at 02:56:29, dcheng wrote:
> Slightly shorter to write:
> const auto& inserted = cache.insert(std::make_pair(locale, base::File()))
> 
> since we don't use |file_on_stack| after this anyway.

Thought some compiler complained, but it looks like it was my misunderstanding,
done.

https://codereview.chromium.org/2113933003/diff/300001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp
(right):

https://codereview.chromium.org/2113933003/diff/300001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp:33:
if (service.is_bound())
On 2016/07/13 at 02:56:29, dcheng wrote:
> I have a question about this pattern:
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-mojo/40iQ0dKWfVM

Thanks for asking in the group. I tried a different pattern in PS17, but will
change to "if (!service)" in the next patch.

https://codereview.chromium.org/2113933003/diff/300001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/modules/hyphenation/hyphenation.mojom
(right):

https://codereview.chromium.org/2113933003/diff/300001/third_party/WebKit/pub...
third_party/WebKit/public/platform/modules/hyphenation/hyphenation.mojom:11:
OpenDictionary(string locale) => (handle? hyphenation_dictionary_handle);
On 2016/07/13 at 02:56:29, dcheng wrote:
> Do we need the ? It seems like we always pass back a handle (even if it's
sometimes invalid). Basically, I'm not sure what ? means when you use it with a
handle =)

mojo runtime checks "handle.IsValid()" if "handle". This method returns invalid
handle when the dictionary is not found for the locale, and "handle?" solved it
for me. Is there better solution?

[jochen (gone - plz use gerrit), 2016-07-13 08:29:26]

build lgtm

[dcheng, 2016-07-13 12:08:59]

mojo lgtm

https://codereview.chromium.org/2113933003/diff/300001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp
(right):

https://codereview.chromium.org/2113933003/diff/300001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp:33:
if (service.is_bound())
On 2016/07/13 05:02:29, kojii wrote:
> On 2016/07/13 at 02:56:29, dcheng wrote:
> > I have a question about this pattern:
>
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-mojo/40iQ0dKWfVM
> 
> Thanks for asking in the group. I tried a different pattern in PS17, but will
> change to "if (!service)" in the next patch.

Nod, either way is fine with me.

[Steven Holte, 2016-07-13 17:50:55]

On 2016/07/13 12:08:59, dcheng wrote:
> mojo lgtm
> 
>
https://codereview.chromium.org/2113933003/diff/300001/third_party/WebKit/Sou...
> File
third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp
> (right):
> 
>
https://codereview.chromium.org/2113933003/diff/300001/third_party/WebKit/Sou...
> third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp:33:
> if (service.is_bound())
> On 2016/07/13 05:02:29, kojii wrote:
> > On 2016/07/13 at 02:56:29, dcheng wrote:
> > > I have a question about this pattern:
> >
>
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-mojo/40iQ0dKWfVM
> > 
> > Thanks for asking in the group. I tried a different pattern in PS17, but
will
> > change to "if (!service)" in the next patch.
> 
> Nod, either way is fine with me.

histograms lgtm

[kojii, 2016-07-14 01:36:26]

The CQ bit was checked by kojii@chromium.org

[kojii, 2016-07-14 01:36:27]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org,
eae@chromium.org, jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2113933003/#ps320001
(title: "dcheng review")

[commit-bot: I haz the power, 2016-07-14 01:37:12]

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-07-14 02:10:47]

Committed patchset #17 (id:320001)

[commit-bot: I haz the power, 2016-07-14 02:13:08]

Description was changed from

==========
Add mojo interface to load hyphenation dictionaries on Android

This patch adds a mojo interface to load hyphenation dictionaries on
Android.

Calls to Minikin using the loaded dictionary files will be in following
patches.

BUG=605840
==========

to

==========
Add mojo interface to load hyphenation dictionaries on Android

This patch adds a mojo interface to load hyphenation dictionaries on
Android.

Calls to Minikin using the loaded dictionary files will be in following
patches.

BUG=605840

Committed: https://crrev.com/30add38fedb8b9bd44b931aa3adba40d7c79b929
Cr-Commit-Position: refs/heads/master@{#405405}
==========

[commit-bot: I haz the power, 2016-07-14 02:13:10]

Patchset 17 (id:??) landed as
https://crrev.com/30add38fedb8b9bd44b931aa3adba40d7c79b929
Cr-Commit-Position: refs/heads/master@{#405405}