Rename pollForCriteria to pollForTestThreadCriteria.

Rename poll*Criteria methods to for consistency

pollForCriteria -> pollInstrumentationThread
pollForUIThreadCriteria -> pollUiThread

More often than not in Android, you should be using
pollUiThread, so this is making the naming
more explicit while we migrate the tests to correctly
use that.

BUG=

Committed: https://crrev.com/445d015b38c93a4afea0cfbf16132ea377531eb6
Cr-Commit-Position: refs/heads/master@{#381504}

[Ted C, 2016-03-12 00:27:43]

tedchoc@chromium.org changed reviewers:
+ boliu@chromium.org, yfriedman@chromium.org

[Ted C, 2016-03-12 00:27:44]

PTAL

boliu - aw
yfriedman - ^aw

[boliu, 2016-03-12 00:30:51]

On 2016/03/12 00:27:44, Ted C wrote:
> PTAL
> 
> boliu - aw
> yfriedman - ^aw

Is "test thread" a consensus? I've been using "instrumentation thread". Plus
there's @UiThread annotation that just runs the test on the UI thread, so "test
thread" and UI thread are the same sometimes?

[Ted C, 2016-03-12 00:34:25]

On 2016/03/12 00:30:51, boliu wrote:
> On 2016/03/12 00:27:44, Ted C wrote:
> > PTAL
> > 
> > boliu - aw
> > yfriedman - ^aw
> 
> Is "test thread" a consensus? I've been using "instrumentation thread". Plus
> there's @UiThread annotation that just runs the test on the UI thread, so
"test
> thread" and UI thread are the same sometimes?

Wouldn't say a consensus, but it was brought up in:
https://codereview.chromium.org/1780833002/diff/1/chrome/android/javatests/sr...

InstrumentationThread is more correct, but it is oooh so long.

I don't have a strong preference, so I am happy to accept the popular choice.

[boliu, 2016-03-12 00:49:43]

I'm +1 for "instrumentation thread".

Given the number of bugs this uncovered, at least in webview tests, maybe do
this slowly component-by-component?

https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java
(right):

https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java:434:
return getKeepScreenOn(view) == expected
getKeepScreenOn should post to UI thread internally

https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientOnScaleChangedTest.java
(right):

https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientOnScaleChangedTest.java:40:
CriteriaHelper.pollForTestThreadCriteria(new Criteria() {
this should be pollForUiThreadCriteria

https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
(right):

https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java:497:
public static void poll(final Callable<Boolean> callable) throws Exception {
should rename this too

[Ted C, 2016-03-14 17:57:03]

On 2016/03/12 00:49:43, boliu wrote:
> I'm +1 for "instrumentation thread".
> 
> Given the number of bugs this uncovered, at least in webview tests, maybe do
> this slowly component-by-component?

I'm not suggesting I change any current Instrumentation Thread -> UI Thread
in this change.  But yes, it should go component to component.  This is
more about a name change to attempt to make it clearer to folks writing
tests.

> 
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> File
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java
> (right):
> 
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java:434:
> return getKeepScreenOn(view) == expected
> getKeepScreenOn should post to UI thread internally
> 
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> File
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientOnScaleChangedTest.java
> (right):
> 
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientOnScaleChangedTest.java:40:
> CriteriaHelper.pollForTestThreadCriteria(new Criteria() {
> this should be pollForUiThreadCriteria
> 
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> File
>
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
> (right):
> 
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
>
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java:497:
> public static void poll(final Callable<Boolean> callable) throws Exception {
> should rename this too

[Yaron, 2016-03-14 19:25:39]

On 2016/03/14 17:57:03, Ted C wrote:
> On 2016/03/12 00:49:43, boliu wrote:
> > I'm +1 for "instrumentation thread".
> > 
> > Given the number of bugs this uncovered, at least in webview tests, maybe do
> > this slowly component-by-component?
> 
> I'm not suggesting I change any current Instrumentation Thread -> UI Thread
> in this change.  But yes, it should go component to component.  This is
> more about a name change to attempt to make it clearer to folks writing
> tests.
> 
> > 
> >
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> > File
> >
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> >
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java:434:
> > return getKeepScreenOn(view) == expected
> > getKeepScreenOn should post to UI thread internally
> > 
> >
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> > File
> >
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientOnScaleChangedTest.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> >
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientOnScaleChangedTest.java:40:
> > CriteriaHelper.pollForTestThreadCriteria(new Criteria() {
> > this should be pollForUiThreadCriteria
> > 
> >
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> > File
> >
>
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> >
>
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java:497:
> > public static void poll(final Callable<Boolean> callable) throws Exception {
> > should rename this too

[Yaron, 2016-03-14 19:31:00]

On 2016/03/14 19:25:39, Yaron wrote:
> On 2016/03/14 17:57:03, Ted C wrote:
> > On 2016/03/12 00:49:43, boliu wrote:
> > > I'm +1 for "instrumentation thread".
> > > 
> > > Given the number of bugs this uncovered, at least in webview tests, maybe
do
> > > this slowly component-by-component?
> > 
> > I'm not suggesting I change any current Instrumentation Thread -> UI Thread
> > in this change.  But yes, it should go component to component.  This is
> > more about a name change to attempt to make it clearer to folks writing
> > tests.
> > 
Another option would be to introduce the newly named function and keep the old
one as deprecated while transitioning one component at a time. Basically, this
gives us an opportunity to catch mistakes rather than blurring them into the
existing functions which post to UI thread.

As for naming, I don't feel too strongly. I suspect there aren't too many
newlines as a result of this since many cases are
"CriteriaHelper.pollForInstrumenationThreadCriteria(new Criteria() {" (i.e.
there's a natural breaking place) and still fits even with a bunch of
indentation :)

> > > 
> > >
> >
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> > > File
> > >
> >
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> > >
> >
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java:434:
> > > return getKeepScreenOn(view) == expected
> > > getKeepScreenOn should post to UI thread internally
> > > 
> > >
> >
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> > > File
> > >
> >
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientOnScaleChangedTest.java
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> > >
> >
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientOnScaleChangedTest.java:40:
> > > CriteriaHelper.pollForTestThreadCriteria(new Criteria() {
> > > this should be pollForUiThreadCriteria
> > > 
> > >
> >
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> > > File
> > >
> >
>
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> > >
> >
>
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java:497:
> > > public static void poll(final Callable<Boolean> callable) throws Exception
{
> > > should rename this too

[Yaron, 2016-03-14 19:32:39]

On Mon, Mar 14, 2016 at 3:31 PM <yfriedman@chromium.org> wrote:

> On 2016/03/14 19:25:39, Yaron wrote:
> > On 2016/03/14 17:57:03, Ted C wrote:
> > > On 2016/03/12 00:49:43, boliu wrote:
> > > > I'm +1 for "instrumentation thread".
> > > >
> > > > Given the number of bugs this uncovered, at least in webview tests,
> maybe
> do
> > > > this slowly component-by-component?
> > >
> > > I'm not suggesting I change any current Instrumentation Thread -> UI
> Thread
> > > in this change. But yes, it should go component to component. This is
> > > more about a name change to attempt to make it clearer to folks writing
> > > tests.
> > >
> Another option would be to introduce the newly named function and keep the
> old
> one as deprecated while transitioning one component at a time. Basically,
> this
> gives us an opportunity to catch mistakes rather than blurring them into
> the
> existing functions which post to UI thread.
>

I mean you can still go through all the "pollforInstru." instances after
with your current approach but this gives you a chance to do a single audit
:)

>
> As for naming, I don't feel too strongly. I suspect there aren't too many
> newlines as a result of this since many cases are
> "CriteriaHelper.pollForInstrumenationThreadCriteria(new Criteria() {" (i.e.
> there's a natural breaking place) and still fits even with a bunch of
> indentation :)
>
>
> > > >
> > > >
> > >
> >
>
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> > > > File
> > > >
> > >
> >
>
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java
> > > > (right):
> > > >
> > > >
> > >
> >
>
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> > > >
> > >
> >
>
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java:434:
> > > > return getKeepScreenOn(view) == expected
> > > > getKeepScreenOn should post to UI thread internally
> > > >
> > > >
> > >
> >
>
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> > > > File
> > > >
> > >
> >
>
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientOnScaleChangedTest.java
> > > > (right):
> > > >
> > > >
> > >
> >
>
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> > > >
> > >
> >
>
>
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientOnScaleChangedTest.java:40:
> > > > CriteriaHelper.pollForTestThreadCriteria(new Criteria() {
> > > > this should be pollForUiThreadCriteria
> > > >
> > > >
> > >
> >
>
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> > > > File
> > > >
> > >
> >
>
>
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
> > > > (right):
> > > >
> > > >
> > >
> >
>
>
https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
> > > >
> > >
> >
>
>
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java:497:
> > > > public static void poll(final Callable<Boolean> callable) throws
> Exception
> {
> > > > should rename this too
>
>
> https://codereview.chromium.org/1786243003/
>

-- 
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.

[boliu, 2016-03-14 20:34:04]

One more point about the "OnFooThread" naming..

A lot of helper methods in AwTestBase use "OnUIThread" to mean post to UI
thread, but actually call the method from instrumentation (or any) thread. If
it's only webview tests are doing this, we can fix it. Is this the case though?

Maybe we should start doing threadchecks in java..

[boliu, 2016-03-14 22:57:25]

fwiw, I'm fixing the threading issues I found in webview here:
https://codereview.chromium.org/1797083002/ +
https://codereview.chromium.org/1799243003/

second one is renaming poll to pollOnInstrumentation, which I'm holding off
until we actually settle on a name..

[Ted C, 2016-03-15 18:53:39]

On 2016/03/14 20:34:04, boliu wrote:
> One more point about the "OnFooThread" naming..
> 
> A lot of helper methods in AwTestBase use "OnUIThread" to mean post to UI
> thread, but actually call the method from instrumentation (or any) thread. If
> it's only webview tests are doing this, we can fix it. Is this the case
though?
> 
> Maybe we should start doing threadchecks in java..

We definitely should start doing thread checks in java.

I think the suffix OnThreadBlah is a bit overloaded.

ThreadUtils#runOnUiThread means post this thing to the UI thread, but you
probably aren't on it.

I've seen code recently that uses OnUiThread to say that this thing must
be called on the UI thread and asserts that:
ChromiumBaseInputConnection#updateStateOnUiThread

And what was proposed here is actually a completely different thing.  Here
we are always running on the Instrumentation thread, but changing whether
the Criteria should be evaluated on some other thread.

This is probably why the previous name pollForUIThreadCriteria existed.  It
doesn't necessarily say it is on the UI thread.

Drop the prefixes all together (potentially dropping Criteria as it seems
redundant in CriteriaHelper):

pollUiThread
pollInstrumentationThread

Say that consistency isn't needed (although reading the arguments, this
does not look weird to me):

pollOnUiThread
pollOnInstrumetnationThread

Or just keep the existing naming structure:

pollForUIThreadCriteria
pollForInstrumentationThreadCriteria

----------------------------

As for keeping the existing name add adding these and doing the slow migration.
I would rather not as it would drag everything out and I don't necessarily have
the time to track down all the fixes.  I'm just hoping to make it easier to
write better tests going forward.  Fixing the world is a noble goal, but not
one I'm trying to do right now.

Final round of naming bikeshedding?

[newt (away), 2016-03-15 19:09:14]

On 2016/03/15 18:53:39, Ted C wrote:
> On 2016/03/14 20:34:04, boliu wrote:
> > One more point about the "OnFooThread" naming..
> > 
> > A lot of helper methods in AwTestBase use "OnUIThread" to mean post to UI
> > thread, but actually call the method from instrumentation (or any) thread.
If
> > it's only webview tests are doing this, we can fix it. Is this the case
> though?
> > 
> > Maybe we should start doing threadchecks in java..
> 
> We definitely should start doing thread checks in java.
> 
> I think the suffix OnThreadBlah is a bit overloaded.
> 
> ThreadUtils#runOnUiThread means post this thing to the UI thread, but you
> probably aren't on it.
> 
> I've seen code recently that uses OnUiThread to say that this thing must
> be called on the UI thread and asserts that:
> ChromiumBaseInputConnection#updateStateOnUiThread
> 
> And what was proposed here is actually a completely different thing.  Here
> we are always running on the Instrumentation thread, but changing whether
> the Criteria should be evaluated on some other thread.
> 
> This is probably why the previous name pollForUIThreadCriteria existed.  It
> doesn't necessarily say it is on the UI thread.
> 
> Drop the prefixes all together (potentially dropping Criteria as it seems
> redundant in CriteriaHelper):
> 
> pollUiThread
> pollInstrumentationThread
> 
> Say that consistency isn't needed (although reading the arguments, this
> does not look weird to me):
> 
> pollOnUiThread
> pollOnInstrumetnationThread
> 
> Or just keep the existing naming structure:
> 
> pollForUIThreadCriteria
> pollForInstrumentationThreadCriteria
> 
> ----------------------------
> 
> As for keeping the existing name add adding these and doing the slow
migration.
> I would rather not as it would drag everything out and I don't necessarily
have
> the time to track down all the fixes.  I'm just hoping to make it easier to
> write better tests going forward.  Fixing the world is a noble goal, but not
> one I'm trying to do right now.
> 
> Final round of naming bikeshedding?

Any of them seem fine. I'd vote for pollUiThread/pollInstrumentationThread.
Concise and they don't further muddle the meaning of "OnUiThread" as a method
suffix.

[Yaron, 2016-03-15 19:33:31]

On 2016/03/15 19:09:14, newt wrote:
> On 2016/03/15 18:53:39, Ted C wrote:
> > On 2016/03/14 20:34:04, boliu wrote:
> > > One more point about the "OnFooThread" naming..
> > > 
> > > A lot of helper methods in AwTestBase use "OnUIThread" to mean post to UI
> > > thread, but actually call the method from instrumentation (or any) thread.
> If
> > > it's only webview tests are doing this, we can fix it. Is this the case
> > though?
> > > 
> > > Maybe we should start doing threadchecks in java..
> > 
> > We definitely should start doing thread checks in java.
> > 
> > I think the suffix OnThreadBlah is a bit overloaded.
> > 
> > ThreadUtils#runOnUiThread means post this thing to the UI thread, but you
> > probably aren't on it.
> > 
> > I've seen code recently that uses OnUiThread to say that this thing must
> > be called on the UI thread and asserts that:
> > ChromiumBaseInputConnection#updateStateOnUiThread
> > 
> > And what was proposed here is actually a completely different thing.  Here
> > we are always running on the Instrumentation thread, but changing whether
> > the Criteria should be evaluated on some other thread.
> > 
> > This is probably why the previous name pollForUIThreadCriteria existed.  It
> > doesn't necessarily say it is on the UI thread.
> > 
> > Drop the prefixes all together (potentially dropping Criteria as it seems
> > redundant in CriteriaHelper):
> > 
> > pollUiThread
> > pollInstrumentationThread
> > 
> > Say that consistency isn't needed (although reading the arguments, this
> > does not look weird to me):
> > 
> > pollOnUiThread
> > pollOnInstrumetnationThread
> > 
> > Or just keep the existing naming structure:
> > 
> > pollForUIThreadCriteria
> > pollForInstrumentationThreadCriteria
> > 
> > ----------------------------
> > 
> > As for keeping the existing name add adding these and doing the slow
> migration.
> > I would rather not as it would drag everything out and I don't necessarily
> have
> > the time to track down all the fixes.  I'm just hoping to make it easier to
> > write better tests going forward.  Fixing the world is a noble goal, but not
> > one I'm trying to do right now.
> > 
> > Final round of naming bikeshedding?
> 
> Any of them seem fine. I'd vote for pollUiThread/pollInstrumentationThread.

sgtm

> Concise and they don't further muddle the meaning of "OnUiThread" as a method
> suffix.

[Ted C, 2016-03-15 23:21:18]

Renamed it to

pollInstrumentationThread

and

pollUiThread

https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
(right):

https://codereview.chromium.org/1786243003/diff/1/android_webview/javatests/s...
android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java:497:
public static void poll(final Callable<Boolean> callable) throws Exception {
On 2016/03/12 00:49:43, boliu wrote:
> should rename this too

Done.

[Ted C, 2016-03-15 23:22:22]

Description was changed from

==========
Rename pollForCriteria to pollForTestThreadCriteria.

More often than not in Android, you should be using
pollForUIThreadCriteria, so this is making the naming
more explicit while we migrate the tests to correctly
use that.

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

to

==========
Rename poll*Criteria methods to for consistency

pollForCriteria -> pollInstrumentationThread
pollForUIThreadCriteria -> pollUiThread

More often than not in Android, you should be using
pollUiThread, so this is making the naming
more explicit while we migrate the tests to correctly
use that.

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

[Yaron, 2016-03-16 14:50:48]

lgtm

[boliu, 2016-03-16 15:59:32]

lgtm

[Ted C, 2016-03-16 17:37:14]

The CQ bit was checked by tedchoc@chromium.org

[Ted C, 2016-03-16 17:37:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
yfriedman@chromium.org, boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/1786243003/#ps40001
(title: "Rebase and update MediaRouterIntegrationTest.java")

[commit-bot: I haz the power, 2016-03-16 17:37:33]

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

[commit-bot: I haz the power, 2016-03-16 18:35:13]

Description was changed from

==========
Rename poll*Criteria methods to for consistency

pollForCriteria -> pollInstrumentationThread
pollForUIThreadCriteria -> pollUiThread

More often than not in Android, you should be using
pollUiThread, so this is making the naming
more explicit while we migrate the tests to correctly
use that.

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

to

==========
Rename poll*Criteria methods to for consistency

pollForCriteria -> pollInstrumentationThread
pollForUIThreadCriteria -> pollUiThread

More often than not in Android, you should be using
pollUiThread, so this is making the naming
more explicit while we migrate the tests to correctly
use that.

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

[commit-bot: I haz the power, 2016-03-16 18:35:14]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-16 18:36:45]

Description was changed from

==========
Rename poll*Criteria methods to for consistency

pollForCriteria -> pollInstrumentationThread
pollForUIThreadCriteria -> pollUiThread

More often than not in Android, you should be using
pollUiThread, so this is making the naming
more explicit while we migrate the tests to correctly
use that.

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

to

==========
Rename poll*Criteria methods to for consistency

pollForCriteria -> pollInstrumentationThread
pollForUIThreadCriteria -> pollUiThread

More often than not in Android, you should be using
pollUiThread, so this is making the naming
more explicit while we migrate the tests to correctly
use that.

BUG=

Committed: https://crrev.com/445d015b38c93a4afea0cfbf16132ea377531eb6
Cr-Commit-Position: refs/heads/master@{#381504}
==========

[commit-bot: I haz the power, 2016-03-16 18:36:47]

Patchset 3 (id:??) landed as
https://crrev.com/445d015b38c93a4afea0cfbf16132ea377531eb6
Cr-Commit-Position: refs/heads/master@{#381504}