Introduce chrome.displaySource API

Introduce chrome.displaySource API

This patch introduces the 'chrome.displaySource' API including the idl file and the permission string and feature.

The patch also provides bindings code and the implementation delegate interface for the 'getAvailableSinks', 'requestAuthentication' methods and for the 'onSinksUpdated' event.
The API tests are included.

The API proposal doc: https://docs.google.com/document/d/1svs0p_b6KAIiBrKjaORsA2D8Pd84u8ksnTCfubYVVWA

BUG=242107
Committed: https://crrev.com/d19c83ee78b525d98a33f9fb320fcfffd1535206
Cr-Commit-Position: refs/heads/master@{#360299}

[Mikhail, 2015-10-30 16:28:27]

Description was changed from

==========
Introduce chrome.displaySource API

This patch introduces the 'chrome.displaySource' API including the idl file and
the permission string and feature.

The patch also provides bindings code and the implementation delegate interface
for the 'getAvailableSinks', 'requestAuthentication' methods and for the
'onSinksUpdated' event.
The API tests are included.

The API proposal doc:
https://docs.google.com/document/d/1svs0p_b6KAIiBrKjaORsA2D8Pd84u8ksnTCfubYVVWA

BUG=242107
==========

to

==========
Introduce chrome.displaySource API

This patch introduces the 'chrome.displaySource' API including the idl file and
the permission string and feature.

The patch also provides bindings code and the implementation delegate interface
for the 'getAvailableSinks', 'requestAuthentication' methods and for the
'onSinksUpdated' event.
The API tests are included.

The API proposal doc:
https://docs.google.com/document/d/1svs0p_b6KAIiBrKjaORsA2D8Pd84u8ksnTCfubYVVWA

BUG=242107
==========

[Mikhail, 2015-10-30 16:28:27]

mikhail.pozdnyakov@intel.com changed reviewers:
+ finnur@chromium.org, kalman@chromium.org

[Mikhail, 2015-10-30 16:33:36]

PTAL

[Finnur, 2015-11-02 11:30:27]

finnur@chromium.org changed reviewers:
+ rdcronin@google.com
- finnur@chromium.org, kalman@chromium.org

[Finnur, 2015-11-02 11:31:29]

Devlin is more appropriate for new extension APIs, as I haven't been doing much
extensions work for a long while now.

[Finnur, 2015-11-02 11:32:51]

finnur@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org
- rdcronin@google.com

[Finnur, 2015-11-02 11:33:30]

Retrying: now with Devlin's Chromium account...

[Devlin, 2015-11-02 22:32:15]

rdevlin.cronin@chromium.org changed reviewers:
+ asargent@chromium.org

[Devlin, 2015-11-02 22:32:15]

My review load's pretty full, so I'm gonna ask Antony to take this one.

[Mikhail, 2015-11-06 09:09:10]

gentle ping for review

[Mikhail, 2015-11-10 07:04:10]

On 2015/11/06 09:09:10, Mikhail wrote:
> gentle ping for review

CC'd more owners

[Reilly Grant (use Gerrit), 2015-11-10 18:51:33]

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

[Reilly Grant (use Gerrit), 2015-11-10 18:51:34]

This isn't a full review but I have some minor comments.

https://codereview.chromium.org/1410093008/diff/20001/extensions/browser/api/...
File extensions/browser/api/display_source/display_source_apitest.cc (right):

https://codereview.chromium.org/1410093008/diff/20001/extensions/browser/api/...
extensions/browser/api/display_source/display_source_apitest.cc:18:
api::display_source::SinkState state =
The style guide generally recommends against default arguments and I don't see
any case in this file where the default value is not used.

https://codereview.chromium.org/1410093008/diff/20001/extensions/browser/api/...
File extensions/browser/api/display_source/display_source_connection_delegate.h
(right):

https://codereview.chromium.org/1410093008/diff/20001/extensions/browser/api/...
extensions/browser/api/display_source/display_source_connection_delegate.h:17:
using AuthInfo = api::display_source::AuthenticationInfo;
It is okay to keep these in .cc files for brevity but please don't pollute the
extensions namespace with generically named types that are specific to this API.

https://codereview.chromium.org/1410093008/diff/20001/extensions/common/api/d...
File extensions/common/api/display_source.idl (right):

https://codereview.chromium.org/1410093008/diff/20001/extensions/common/api/d...
extensions/common/api/display_source.idl:58: DOMString method;
Why not an enum here?

https://codereview.chromium.org/1410093008/diff/20001/extensions/test/data/ap...
File extensions/test/data/api_test/display_source/api/background.js (right):

https://codereview.chromium.org/1410093008/diff/20001/extensions/test/data/ap...
extensions/test/data/api_test/display_source/api/background.js:1: // Copyright
(c) 2015 The Chromium Authors. All rights reserved.
No "(c)" in new files.

[Mikhail, 2015-11-11 13:36:04]

Thanks for having a look!

https://codereview.chromium.org/1410093008/diff/20001/extensions/browser/api/...
File extensions/browser/api/display_source/display_source_apitest.cc (right):

https://codereview.chromium.org/1410093008/diff/20001/extensions/browser/api/...
extensions/browser/api/display_source/display_source_apitest.cc:18:
api::display_source::SinkState state =
On 2015/11/10 18:51:34, Reilly Grant wrote:
> The style guide generally recommends against default arguments and I don't see
> any case in this file where the default value is not used.

Done.

https://codereview.chromium.org/1410093008/diff/20001/extensions/browser/api/...
File extensions/browser/api/display_source/display_source_connection_delegate.h
(right):

https://codereview.chromium.org/1410093008/diff/20001/extensions/browser/api/...
extensions/browser/api/display_source/display_source_connection_delegate.h:17:
using AuthInfo = api::display_source::AuthenticationInfo;
On 2015/11/10 18:51:34, Reilly Grant wrote:
> It is okay to keep these in .cc files for brevity but please don't pollute the
> extensions namespace with generically named types that are specific to this
API.

Good point, would it be OK if I prefix them?

https://codereview.chromium.org/1410093008/diff/20001/extensions/common/api/d...
File extensions/common/api/display_source.idl (right):

https://codereview.chromium.org/1410093008/diff/20001/extensions/common/api/d...
extensions/common/api/display_source.idl:58: DOMString method;
On 2015/11/10 18:51:34, Reilly Grant wrote:
> Why not an enum here?
The intention is to keep API generic and flexible for adding more auth
mechanisms, pls. see
https://docs.google.com/document/d/1svs0p_b6KAIiBrKjaORsA2D8Pd84u8ksnTCfubYVV...

https://codereview.chromium.org/1410093008/diff/20001/extensions/test/data/ap...
File extensions/test/data/api_test/display_source/api/background.js (right):

https://codereview.chromium.org/1410093008/diff/20001/extensions/test/data/ap...
extensions/test/data/api_test/display_source/api/background.js:1: // Copyright
(c) 2015 The Chromium Authors. All rights reserved.
On 2015/11/10 18:51:34, Reilly Grant wrote:
> No "(c)" in new files.

Done.

[Reilly Grant (use Gerrit), 2015-11-11 18:36:13]

https://codereview.chromium.org/1410093008/diff/20001/extensions/common/api/d...
File extensions/common/api/display_source.idl (right):

https://codereview.chromium.org/1410093008/diff/20001/extensions/common/api/d...
extensions/common/api/display_source.idl:58: DOMString method;
On 2015/11/11 13:36:04, Mikhail wrote:
> On 2015/11/10 18:51:34, Reilly Grant wrote:
> > Why not an enum here?
> The intention is to keep API generic and flexible for adding more auth
> mechanisms, pls. see
>
https://docs.google.com/document/d/1svs0p_b6KAIiBrKjaORsA2D8Pd84u8ksnTCfubYVV...

If more mechanisms are added the API can be very easily extended. It is
important for the documentation of this API to cover the expected authentication
mechanisms and how an application using them should use them. That is easier if
this is an enum. The fact is, in the Javascript bindings this becomes a string
anyways. Making it an enum simply means the list of possible values is
documented.

[Mikhail, 2015-11-12 10:08:49]

Patchset #4 (id:60001) has been deleted

[Mikhail, 2015-11-12 10:34:17]

On 2015/11/11 18:36:13, Reilly Grant wrote:
>
https://codereview.chromium.org/1410093008/diff/20001/extensions/common/api/d...
> File extensions/common/api/display_source.idl (right):
> 
>
https://codereview.chromium.org/1410093008/diff/20001/extensions/common/api/d...
> extensions/common/api/display_source.idl:58: DOMString method;
> On 2015/11/11 13:36:04, Mikhail wrote:
> > On 2015/11/10 18:51:34, Reilly Grant wrote:
> > > Why not an enum here?
> > The intention is to keep API generic and flexible for adding more auth
> > mechanisms, pls. see
> >
>
https://docs.google.com/document/d/1svs0p_b6KAIiBrKjaORsA2D8Pd84u8ksnTCfubYVV...
> 
> If more mechanisms are added the API can be very easily extended. It is
> important for the documentation of this API to cover the expected
authentication
> mechanisms and how an application using them should use them. That is easier
if
> this is an enum. The fact is, in the Javascript bindings this becomes a string
> anyways. Making it an enum simply means the list of possible values is
> documented.

done.

[asargent_no_longer_on_chrome, 2015-11-14 00:53:33]

Sorry for delay in reviewing - here are a couple of minor things to fix up that
I noticed while looking at some of the CL - I'll try and finish looking at the
other files soon.

https://codereview.chromium.org/1410093008/diff/80001/extensions/browser/api/...
File extensions/browser/api/display_source/OWNERS (right):

https://codereview.chromium.org/1410093008/diff/80001/extensions/browser/api/...
extensions/browser/api/display_source/OWNERS:1: alexander.shalamov@intel.com
I don't see this email address listed in the top-level AUTHORS file, and it
looks like there isn't a *@intel.com entry there.

It looks like we already have a submission of the Contributor License Agreement
for Intel, so I think you can you just add this email address to the src/AUTHORS
file in this CL. 


http://dev.chromium.org/developers/contributing-code#TOC-Get-your-code-ready
http://dev.chromium.org/developers/contributing-code/external-contributor-che...

https://codereview.chromium.org/1410093008/diff/80001/extensions/common/api/d...
File extensions/common/api/display_source.idl (right):

https://codereview.chromium.org/1410093008/diff/80001/extensions/common/api/d...
extensions/common/api/display_source.idl:12: create_media_pipeline_error,
nit: for readability, please add a newline in between each value when there are
comments >= 2 lines long. For example, here it's kind of hard to see each enum
value name:

// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod
// tempor incididunt ut labore et dolore magna aliqua.
foo,
// Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut
// aliquip ex ea commodo consequat.
bar,
// Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore
// eu fugiat nulla pariatur
baz



but the following is better:



// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod
// tempor incididunt ut labore et dolore magna aliqua.
foo,

// Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut
// aliquip ex ea commodo consequat.
bar,

// Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore
// eu fugiat nulla pariatur
baz

https://codereview.chromium.org/1410093008/diff/80001/extensions/common/api/d...
extensions/common/api/display_source.idl:13: // A new Display session cannot be
started before the existing one is terminated.
nit: please keep lines <= 80 chars long where possible

Same comment applies in a few other places down below.

[Mikhail, 2015-11-16 13:57:26]

Patchset #5 (id:100001) has been deleted

[Mikhail, 2015-11-16 14:22:24]

Thank you for your comments

https://codereview.chromium.org/1410093008/diff/80001/extensions/browser/api/...
File extensions/browser/api/display_source/OWNERS (right):

https://codereview.chromium.org/1410093008/diff/80001/extensions/browser/api/...
extensions/browser/api/display_source/OWNERS:1: alexander.shalamov@intel.com
On 2015/11/14 00:53:32, Antony Sargent wrote:
> I don't see this email address listed in the top-level AUTHORS file, and it
> looks like there isn't a mailto:*@intel.com entry there.
> 
> It looks like we already have a submission of the Contributor License
Agreement
> for Intel, so I think you can you just add this email address to the
src/AUTHORS
> file in this CL. 
> 
> 
> http://dev.chromium.org/developers/contributing-code#TOC-Get-your-code-ready
>
http://dev.chromium.org/developers/contributing-code/external-contributor-che...
> 
> 
Alexander is now mentioned in the AUTHORS file, after crrev.com/1439063002 has
landed (https://codereview.chromium.org/1439063002/diff/20001/AUTHORS)

https://codereview.chromium.org/1410093008/diff/80001/extensions/common/api/d...
File extensions/common/api/display_source.idl (right):

https://codereview.chromium.org/1410093008/diff/80001/extensions/common/api/d...
extensions/common/api/display_source.idl:12: create_media_pipeline_error,
On 2015/11/14 00:53:32, Antony Sargent wrote:
> nit: for readability, please add a newline in between each value when there
are
> comments >= 2 lines long. For example, here it's kind of hard to see each enum
> value name:
> 
> // Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod
> // tempor incididunt ut labore et dolore magna aliqua.
> foo,
> // Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut
> // aliquip ex ea commodo consequat.
> bar,
> // Duis aute irure dolor in reprehenderit in voluptate velit esse cillum
dolore
> // eu fugiat nulla pariatur
> baz
> 
> 
> 
> but the following is better:
> 
> 
> 
> // Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod
> // tempor incididunt ut labore et dolore magna aliqua.
> foo,
> 
> // Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut
> // aliquip ex ea commodo consequat.
> bar,
> 
> // Duis aute irure dolor in reprehenderit in voluptate velit esse cillum
dolore
> // eu fugiat nulla pariatur
> baz

Done.

https://codereview.chromium.org/1410093008/diff/80001/extensions/common/api/d...
extensions/common/api/display_source.idl:13: // A new Display session cannot be
started before the existing one is terminated.
On 2015/11/14 00:53:32, Antony Sargent wrote:
> nit: please keep lines <= 80 chars long where possible
> 
> Same comment applies in a few other places down below. 

Done.

[asargent_no_longer_on_chrome, 2015-11-17 00:51:56]

https://codereview.chromium.org/1410093008/diff/120001/chrome/app/generated_r...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1410093008/diff/120001/chrome/app/generated_r...
chrome/app/generated_resources.grd:4219: +          Manage Wi-Fi display
connections
I fear that this permission warning may not mean much to an ordinary user. I'd
imagine that even highly technical people who know a little about WiFi display
wouldn't get enough information from the word "manage".

Also, there was that thread of conversation in the design doc that ended with
the conclusion to rename the API namespace from "wifiDisplaySource to just
"displaySource". Should that same thing be reflected in this permission warning?


I think the kinds of questions that ordinary users might have about extensions
using this API are things like:

-Can this API be used by the extension to "see" what's on my screen? 
-Does the extension get to send media to any displays without my approval, or do
I get a chance to authorize it on every connection?

Would something like "Send audio and video to displays on the local network" be
both technically accurate and easier to understand?

https://codereview.chromium.org/1410093008/diff/120001/extensions/browser/api...
File extensions/browser/api/display_source/display_source_api.cc (right):

https://codereview.chromium.org/1410093008/diff/120001/extensions/browser/api...
extensions/browser/api/display_source/display_source_api.cc:26: if
(!args_->empty()) {
I don't know that it's really worth worrying about the arguments being empty in
practice.

Also, if we ever decided to add some extra (possibly optional) parameters to the
getAvailableSinks method, having this code here in older versions of the browser
could make life more difficult for developers because it would mean that those
older ever wouldn't be forward compatible. Then they'd have to either check the
browser version first or try calling the function with the new arguments, trap
the error, and try again without them.

https://codereview.chromium.org/1410093008/diff/120001/extensions/browser/api...
File extensions/browser/api/display_source/display_source_connection_delegate.h
(right):

https://codereview.chromium.org/1410093008/diff/120001/extensions/browser/api...
extensions/browser/api/display_source/display_source_connection_delegate.h:19:
class DisplaySourceConnectionDelegate : public KeyedService {
nit: please add at least a brief class overview comment (
https://google.github.io/styleguide/cppguide.html#Class_Comments )

https://codereview.chromium.org/1410093008/diff/120001/extensions/browser/api...
extensions/browser/api/display_source/display_source_connection_delegate.h:31:
std::string sink_ip;
I saw the discussion in the design doc around the privacy issues with ip
addresses and the decision to move to using an opaque sink_id instead. Do you
need to have these 2 strings here anymore? (If there is an implementation detail
that needs to do something like map between sink_id and ip address in future
changes that's fine; I just don't see any code in this CL that appears to use
these ip addresses)

https://codereview.chromium.org/1410093008/diff/120001/extensions/browser/api...
File extensions/browser/api/display_source/display_source_event_router.cc
(right):

https://codereview.chromium.org/1410093008/diff/120001/extensions/browser/api...
extensions/browser/api/display_source/display_source_event_router.cc:126: return
new DisplaySourceEventRouterImpl(browser_context);
Do we need to have the fancy extra indirection of a DisplaySourceEventRouter and
 DisplaySourceEventRouterImpl, instead of just having the functionality from
DisplaySourceEventRouterImpl live in DisplaySourceEventRouter? Usually we only
do that when there could be more than one implementation.

https://codereview.chromium.org/1410093008/diff/120001/extensions/common/perm...
File extensions/common/permissions/extensions_api_permissions.cc (right):

https://codereview.chromium.org/1410093008/diff/120001/extensions/common/perm...
extensions/common/permissions/extensions_api_permissions.cc:58:
APIPermissionInfo::kFlagCannotBeOptional},
Is there a reason this can't this be an optional permission, or did you just
copy the kFlagCannotBeOptional flag because other permissions here do the same
thing? 

For more background if you're unfamiliar with optional permissions, please see
https://developer.chrome.com/extensions/permissions

As a general rule all APIs should support optional permissions except for a very
few edge cases.

https://codereview.chromium.org/1410093008/diff/120001/extensions/test/data/a...
File extensions/test/data/api_test/display_source/api/background.js (right):

https://codereview.chromium.org/1410093008/diff/120001/extensions/test/data/a...
extensions/test/data/api_test/display_source/api/background.js:10:
chrome.test.assertEq("sink 1", sink.name);
consider also testing that 'state' is disconnected

[Mikhail, 2015-11-17 14:07:43]

https://codereview.chromium.org/1410093008/diff/120001/chrome/app/generated_r...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1410093008/diff/120001/chrome/app/generated_r...
chrome/app/generated_resources.grd:4219: +          Manage Wi-Fi display
connections
On 2015/11/17 00:51:55, Antony Sargent wrote:
> Would something like "Send audio and video to displays on the local network"
be
> both technically accurate and easier to understand?
Thanks for the proposal! I think it fits well.

https://codereview.chromium.org/1410093008/diff/120001/extensions/browser/api...
File extensions/browser/api/display_source/display_source_api.cc (right):

https://codereview.chromium.org/1410093008/diff/120001/extensions/browser/api...
extensions/browser/api/display_source/display_source_api.cc:26: if
(!args_->empty()) {
On 2015/11/17 00:51:55, Antony Sargent wrote:
> I don't know that it's really worth worrying about the arguments being empty
in
> practice.
> 
> Also, if we ever decided to add some extra (possibly optional) parameters to
the
> getAvailableSinks method, having this code here in older versions of the
browser
> could make life more difficult for developers because it would mean that those
> older ever wouldn't be forward compatible. Then they'd have to either check
the
> browser version first or try calling the function with the new arguments, trap
> the error, and try again without them. 
Agree, fixed.

https://codereview.chromium.org/1410093008/diff/120001/extensions/browser/api...
File extensions/browser/api/display_source/display_source_connection_delegate.h
(right):

https://codereview.chromium.org/1410093008/diff/120001/extensions/browser/api...
extensions/browser/api/display_source/display_source_connection_delegate.h:19:
class DisplaySourceConnectionDelegate : public KeyedService {
On 2015/11/17 00:51:55, Antony Sargent wrote:
> nit: please add at least a brief class overview comment (
> https://google.github.io/styleguide/cppguide.html#Class_Comments )

Done.

https://codereview.chromium.org/1410093008/diff/120001/extensions/browser/api...
extensions/browser/api/display_source/display_source_connection_delegate.h:31:
std::string sink_ip;
On 2015/11/17 00:51:55, Antony Sargent wrote:
> I saw the discussion in the design doc around the privacy issues with ip
> addresses and the decision to move to using an opaque sink_id instead. Do you
> need to have these 2 strings here anymore? (If there is an implementation
detail
> that needs to do something like map between sink_id and ip address in future
> changes that's fine; I just don't see any code in this CL that appears to use
> these ip addresses)
The ip info is only needed for internal usage (will not be accessible for the
user).

https://codereview.chromium.org/1410093008/diff/120001/extensions/browser/api...
File extensions/browser/api/display_source/display_source_event_router.cc
(right):

https://codereview.chromium.org/1410093008/diff/120001/extensions/browser/api...
extensions/browser/api/display_source/display_source_event_router.cc:126: return
new DisplaySourceEventRouterImpl(browser_context);
On 2015/11/17 00:51:56, Antony Sargent wrote:
> Do we need to have the fancy extra indirection of a DisplaySourceEventRouter
and
>  DisplaySourceEventRouterImpl, instead of just having the functionality from
> DisplaySourceEventRouterImpl live in DisplaySourceEventRouter? Usually we only
> do that when there could be more than one implementation.
It just hides out the implementation details, but we can do without this
indirection

https://codereview.chromium.org/1410093008/diff/120001/extensions/common/perm...
File extensions/common/permissions/extensions_api_permissions.cc (right):

https://codereview.chromium.org/1410093008/diff/120001/extensions/common/perm...
extensions/common/permissions/extensions_api_permissions.cc:58:
APIPermissionInfo::kFlagCannotBeOptional},
On 2015/11/17 00:51:56, Antony Sargent wrote:
> Is there a reason this can't this be an optional permission, or did you just
> copy the kFlagCannotBeOptional flag because other permissions here do the same
> thing? 
> 
> For more background if you're unfamiliar with optional permissions, please see
> https://developer.chrome.com/extensions/permissions
> 
> As a general rule all APIs should support optional permissions except for a
very
> few edge cases.  
Here I took after cast API which has quite similar functionality. The
kFlagCannotBeOptional is removed now.

https://codereview.chromium.org/1410093008/diff/120001/extensions/test/data/a...
File extensions/test/data/api_test/display_source/api/background.js (right):

https://codereview.chromium.org/1410093008/diff/120001/extensions/test/data/a...
extensions/test/data/api_test/display_source/api/background.js:10:
chrome.test.assertEq("sink 1", sink.name);
On 2015/11/17 00:51:56, Antony Sargent wrote:
> consider also testing that 'state' is disconnected

Done.

[asargent_no_longer_on_chrome, 2015-11-17 23:29:20]

lgtm

[Mikhail, 2015-11-18 06:59:35]

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    extensions/browser/extension_event_histogram_value.h
    extensions/browser/extension_function_histogram_value.h
    tools/metrics/histograms/histograms.xml

CC'd the metrics owners, PTAL

[Ilya Sherman, 2015-11-18 07:11:19]

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

[Ilya Sherman, 2015-11-18 07:11:20]

metrics lgtm

[Mikhail, 2015-11-18 07:12:54]

The CQ bit was checked by mikhail.pozdnyakov@intel.com

[commit-bot: I haz the power, 2015-11-18 07:13:18]

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

[commit-bot: I haz the power, 2015-11-18 07:18:37]

Committed patchset #6 (id:140001)

[commit-bot: I haz the power, 2015-11-18 07:19:39]

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