Deletes base::MessageLoop::set_thread_name().

Deletes base::MessageLoop::set_thread_name().

Implements base::MessageLoop::GetThreadName by fetching the platform
thread name. This new function replaces MessageLoop::thread_name_ and
avoids having to store the thread name in multiple places.

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

[alokp, 2016-05-02 23:03:43]

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

[jam, 2016-05-02 23:57:53]

It seems unfortunate that we have two places to store thread name. it seems
there are very few consumers of MessageLoop::thread_name, seems they can be
converted to use base/threading/thread_id_name_manager.h instead?

[alokp, 2016-05-03 05:58:04]

On 2016/05/02 23:57:53, jam wrote:
> It seems unfortunate that we have two places to store thread name. it seems
> there are very few consumers of MessageLoop::thread_name, seems they can be
> converted to use base/threading/thread_id_name_manager.h instead?

May be I am missing something. I do not see a way to get the thread-id from a
MessageLoop. Without that I am not sure how to convert this callsite:
https://code.google.com/p/chromium/codesearch#chromium/src/base/trace_event/t...

I might be able to get rid of MessageLoop::set_thread_name by fetching and
storing the thread name during MessageLoop::BindToCurrentThread and
MessageLoop::SetTaskRunner. Would that be better?

[jam, 2016-05-03 16:01:59]

On 2016/05/03 05:58:04, alokp wrote:
> On 2016/05/02 23:57:53, jam wrote:
> > It seems unfortunate that we have two places to store thread name. it seems
> > there are very few consumers of MessageLoop::thread_name, seems they can be
> > converted to use base/threading/thread_id_name_manager.h instead?
> 
> May be I am missing something. I do not see a way to get the thread-id from a
> MessageLoop. Without that I am not sure how to convert this callsite:
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/trace_event/t...
> 
> I might be able to get rid of MessageLoop::set_thread_name by fetching and
> storing the thread name during MessageLoop::BindToCurrentThread and
> MessageLoop::SetTaskRunner. Would that be better?

The name wouldn't be available in BindToCurrentThread (since that's called in
ctor of MessageLoop, and for example in this case base::PlatformThread::SetName
is called after). but BindToCurrentThread can perhaps store the
PlatformThred::CurrentId() and expose it in a getter, which that trace_log
method could use to get a name from ThreadIdNameManager?

[alokp, 2016-05-04 22:40:35]

Description was changed from

==========
Set thread name for renderer main message loop.
==========

to

==========
Implements SingleThreadTaskRunner::GetThreadName.

This new function replaces MessageLoop::thread_name_ and avoids having
to store the thread name in multiple places.
==========

[alokp, 2016-05-04 22:40:58]

Description was changed from

==========
Implements SingleThreadTaskRunner::GetThreadName.

This new function replaces MessageLoop::thread_name_ and avoids having
to store the thread name in multiple places.
==========

to

==========
Implements SingleThreadTaskRunner::GetThreadName.

This new function replaces MessageLoop::thread_name_ and avoids having
to store the thread name in multiple places.
==========

[alokp, 2016-05-04 22:46:33]

> The name wouldn't be available in BindToCurrentThread (since that's called in
> ctor of MessageLoop, and for example in this case
base::PlatformThread::SetName
> is called after). but BindToCurrentThread can perhaps store the
> PlatformThred::CurrentId() and expose it in a getter, which that trace_log
> method could use to get a name from ThreadIdNameManager?

ThreadId is actually already stored in various implementations of
SingleThreadTaskRunner. So it makes sense for this function to be there. PTAL.

[jam, 2016-05-06 16:50:23]

great, this is fine by me but a base/ owner should look.

https://codereview.chromium.org/1942053002/diff/20001/base/message_loop/messa...
File base/message_loop/message_loop.h (right):

https://codereview.chromium.org/1942053002/diff/20001/base/message_loop/messa...
base/message_loop/message_loop.h:298: const char* GetThreadName() const { return
task_runner_->GetThreadName(); }
nit: no need to have this wrapper method? i.e. caller can just do
task_runner()->GetThreadName()

https://codereview.chromium.org/1942053002/diff/20001/base/single_thread_task...
File base/single_thread_task_runner.h (right):

https://codereview.chromium.org/1942053002/diff/20001/base/single_thread_task...
base/single_thread_task_runner.h:33: virtual const char* GetThreadName() const =
0;
premature optimization to return const char* instead of std::string? i.e. what
are the guarantees on how long the pointer will be valid, and that the caller
won't store it

https://codereview.chromium.org/1942053002/diff/20001/content/browser/browser...
File content/browser/browser_thread_impl.cc (right):

https://codereview.chromium.org/1942053002/diff/20001/content/browser/browser...
content/browser/browser_thread_impl.cc:77: return content::GetThreadName(id_);
nit: no content::

[alokp, 2016-05-07 04:13:08]

Description was changed from

==========
Implements SingleThreadTaskRunner::GetThreadName.

This new function replaces MessageLoop::thread_name_ and avoids having
to store the thread name in multiple places.
==========

to

==========
Implements SingleThreadTaskRunner::GetThreadName.

This new function replaces MessageLoop::thread_name_ and avoids having
to store the thread name in multiple places.
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[alokp, 2016-05-07 04:14:04]

alokp@chromium.org changed reviewers:
+ danakj@chromium.org

[alokp, 2016-05-07 04:14:05]

Dana: base/

https://codereview.chromium.org/1942053002/diff/20001/base/message_loop/messa...
File base/message_loop/message_loop.h (right):

https://codereview.chromium.org/1942053002/diff/20001/base/message_loop/messa...
base/message_loop/message_loop.h:298: const char* GetThreadName() const { return
task_runner_->GetThreadName(); }
On 2016/05/06 16:50:23, jam wrote:
> nit: no need to have this wrapper method? i.e. caller can just do
> task_runner()->GetThreadName()

Done.

https://codereview.chromium.org/1942053002/diff/20001/base/single_thread_task...
File base/single_thread_task_runner.h (right):

https://codereview.chromium.org/1942053002/diff/20001/base/single_thread_task...
base/single_thread_task_runner.h:33: virtual const char* GetThreadName() const =
0;
On 2016/05/06 16:50:23, jam wrote:
> premature optimization to return const char* instead of std::string? i.e. what
> are the guarantees on how long the pointer will be valid, and that the caller
> won't store it

Done.

https://codereview.chromium.org/1942053002/diff/20001/content/browser/browser...
File content/browser/browser_thread_impl.cc (right):

https://codereview.chromium.org/1942053002/diff/20001/content/browser/browser...
content/browser/browser_thread_impl.cc:77: return content::GetThreadName(id_);
On 2016/05/06 16:50:23, jam wrote:
> nit: no content::

The compiler cannot disambiguate between the member function and the global
function without that.

[danakj, 2016-05-10 21:49:28]

danakj@chromium.org changed reviewers:
+ gab@chromium.org

[danakj, 2016-05-10 21:49:29]

+gab thoughts?

[gab, 2016-05-12 15:06:06]

Description was changed from

==========
Implements SingleThreadTaskRunner::GetThreadName.

This new function replaces MessageLoop::thread_name_ and avoids having
to store the thread name in multiple places.
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Implements SingleThreadTaskRunner::GetThreadName.

This new function replaces MessageLoop::thread_name_ and avoids having
to store the thread name in multiple places.

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[gab, 2016-05-12 15:50:57]

Reviewed and added nits as I went, pasted below but probably irrelevant in the
light of the following meta comment:

I very much support getting rid of MessageLoop::thread_name() however I don't
like introducing SingleThreadTaskRunner::GetThreadName(). In the world of
TaskScheduler we're heading towards, most threads will be an implementation
detail and so will their names.

I could one day be convinced of SingleThreadTaskRunner::GetName() and
SequencedTaskRunner::GetName() where those would be the name of the sequences
they represent (i.e. in TaskScheduler, two SingleThreadTaskRunners can share the
same thread while having unrelated purposes), but I don't think we need to think
about that just yet.

With my comment @ [1] it looks like the only other two use cases of
MessageLoop::thread_name() are for logging statements (one of which is a DLOG).
That doesn't seem to warrant a new API surface to me.

Since it's only for logging, whatever we do to keep those working for now
doesn't need to be efficient either. e.g. we could get rid of
MessageLoop::set_thread_name() and have MessageLoop::GetThreadNameForLogging()
which would return MessageLoop::thread_name_ which would be synchronized and set
by `thread_name_ = PlatformThread::GetName(); DCHECK(thread_name_) <<
"MessageLoop bound threads need to have an explicit name"` in
MessageLoop::BindToCurrentThread() or something -- i.e. name is not always
available but for logging purposes it's available early enough to work..?
Or maybe using PlatformThread::CurrentID() is better (then you at least have an
ID and in most cases you can translate that to a name?)

Anyways, I'll let you figure out how to do it for the two remaining log
statements but don't it warrants a new API which would soon be confusing in the
TaskScheduler world.

Thanks!
Gab

[1]
https://codereview.chromium.org/1942053002/diff/140001/content/browser/browse...

https://codereview.chromium.org/1942053002/diff/140001/base/task_scheduler/sc...
File base/task_scheduler/scheduler_worker_thread.cc (right):

https://codereview.chromium.org/1942053002/diff/140001/base/task_scheduler/sc...
base/task_scheduler/scheduler_worker_thread.cc:35: return std::string();
This is wrong, these threads do have a name (set here:
https://code.google.com/p/chromium/codesearch#chromium/src/base/task_schedule...
)

i.e. SchedulerWorkerThread::Delegate could have GetThreadName() and this here
could return delegate_->GetThreadName();

https://codereview.chromium.org/1942053002/diff/140001/content/browser/browse...
File content/browser/browser_thread_impl.cc (right):

https://codereview.chromium.org/1942053002/diff/140001/content/browser/browse...
content/browser/browser_thread_impl.cc:77: return content::GetThreadName(id_);
content:: is redundant here

https://codereview.chromium.org/1942053002/diff/140001/content/browser/browse...
content/browser/browser_thread_impl.cc:435: std::string actual_name("Unknown
Thread");
435-440 can all be replaced by:

std::string actual_name = PlatformThread::GetName();

https://codereview.chromium.org/1942053002/diff/140001/content/test/test_blin...
File content/test/test_blink_web_unit_test_support.cc (right):

https://codereview.chromium.org/1942053002/diff/140001/content/test/test_blin...
content/test/test_blink_web_unit_test_support.cc:76: std::string
MessageLoopTaskRunner::GetThreadName() const override {
Don't think you need MessageLoopTaskRunner:: here

https://codereview.chromium.org/1942053002/diff/140001/sync/engine/sync_sched...
File sync/engine/sync_scheduler_impl.cc (right):

https://codereview.chromium.org/1942053002/diff/140001/sync/engine/sync_sched...
sync/engine/sync_scheduler_impl.cc:16: #include
"base/thread_task_runner_handle.h"
FYI, that header moved to base/threading/thread_task_runner_handle.h yesterday.

[alokp, 2016-05-18 23:45:46]

Description was changed from

==========
Implements SingleThreadTaskRunner::GetThreadName.

This new function replaces MessageLoop::thread_name_ and avoids having
to store the thread name in multiple places.

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Deletes base::MessageLoop::set_thread_name().

Implements base::MessageLoop::GetThreadName by fetching the platform
thread name. This new function replaces MessageLoop::thread_name_ and
avoids having to store the thread name in multiple places.
==========

[alokp, 2016-05-18 23:46:02]

Description was changed from

==========
Deletes base::MessageLoop::set_thread_name().

Implements base::MessageLoop::GetThreadName by fetching the platform
thread name. This new function replaces MessageLoop::thread_name_ and
avoids having to store the thread name in multiple places.
==========

to

==========
Deletes base::MessageLoop::set_thread_name().

Implements base::MessageLoop::GetThreadName by fetching the platform
thread name. This new function replaces MessageLoop::thread_name_ and
avoids having to store the thread name in multiple places.
==========

[alokp, 2016-05-18 23:48:15]

Gabriel: Thanks for the detailed feedback. I have removed the
SingleThreadTaskRunner::GetThreadName interface and implemented
MessageLoop::GetThreadName instead by storing the thread id. PTAL.

[gab, 2016-05-20 20:25:39]

lg % synchronization requirement

https://codereview.chromium.org/1942053002/diff/160001/base/message_loop/mess...
File base/message_loop/message_loop.cc (right):

https://codereview.chromium.org/1942053002/diff/160001/base/message_loop/mess...
base/message_loop/message_loop.cc:421: thread_id_ = PlatformThread::CurrentId();
This needs to be synchronized as it's set outside the constructor. Though all
that's required is a memory barrier, @danakj think this warrants a Lock? In
practice we probably have an implicit memory barrier from the task posting
machinery... or we could add an explicit memory barrier instead of a lock?

Or we just call the method GetThreadNameForLogging() and document it as being
synchronized and only intended to be used for logging?

[alokp, 2016-06-01 19:23:18]

https://codereview.chromium.org/1942053002/diff/160001/base/message_loop/mess...
File base/message_loop/message_loop.cc (right):

https://codereview.chromium.org/1942053002/diff/160001/base/message_loop/mess...
base/message_loop/message_loop.cc:421: thread_id_ = PlatformThread::CurrentId();
On 2016/05/20 20:25:39, gab wrote:
> This needs to be synchronized as it's set outside the constructor. Though all
> that's required is a memory barrier, @danakj think this warrants a Lock? In
> practice we probably have an implicit memory barrier from the task posting
> machinery... or we could add an explicit memory barrier instead of a lock?
> 
> Or we just call the method GetThreadNameForLogging() and document it as being
> synchronized and only intended to be used for logging?

Done. Can you please check the barrier usage and memory_order. Thanks!

[gab, 2016-06-03 15:31:17]

Reply below.

PS: Please do separate patch sets when rebasing (one for the rebase, one for the
true diff), makes diff reviews easier :-).

https://codereview.chromium.org/1942053002/diff/180001/base/message_loop/mess...
File base/message_loop/message_loop.cc (right):

https://codereview.chromium.org/1942053002/diff/180001/base/message_loop/mess...
base/message_loop/message_loop.cc:424: subtle::Release_Load(&thread_id_));
I don't think you need to use an Atomic32. Instead you can initialize
|thread_id_| to |PlatformThread::kInvalidThreadId| and here do:

if (thread_id_ == PlatformThread::kInvalidThreadId) {
  // |thread_id_| should already have been initialized but this thread might not
have received the update yet.
  subtle::MemoryBarrier();
  DCHECK_NE(PlatformThread::kInvalidThreadId, thread_id_);
}
return ThreadIdNameManager::GetInstance()->GetName(thread_id_);

[alokp, 2016-06-03 16:34:56]

https://codereview.chromium.org/1942053002/diff/180001/base/message_loop/mess...
File base/message_loop/message_loop.cc (right):

https://codereview.chromium.org/1942053002/diff/180001/base/message_loop/mess...
base/message_loop/message_loop.cc:424: subtle::Release_Load(&thread_id_));
On 2016/06/03 15:31:17, gab wrote:
> I don't think you need to use an Atomic32. Instead you can initialize
> |thread_id_| to |PlatformThread::kInvalidThreadId| and here do:
> 
> if (thread_id_ == PlatformThread::kInvalidThreadId) {
>   // |thread_id_| should already have been initialized but this thread might
not
> have received the update yet.
>   subtle::MemoryBarrier();
>   DCHECK_NE(PlatformThread::kInvalidThreadId, thread_id_);
> }
> return ThreadIdNameManager::GetInstance()->GetName(thread_id_);

Used subtle::MemoryBarrier() in the new patch. I did not add the DCHECK_NE
because I think it is possible that this function is called before
BindToCurrentThread is executed. No?

May be I should add a WARNING that message loop is not bound to any thread yet.
WDYT?

https://codereview.chromium.org/1942053002/diff/220001/base/message_loop/mess...
File base/message_loop/message_loop.cc (right):

https://codereview.chromium.org/1942053002/diff/220001/base/message_loop/mess...
base/message_loop/message_loop.cc:418: thread_id_ = PlatformThread::CurrentId();
Do I need a memory barrier here as well?

[gab, 2016-06-07 00:31:00]

https://codereview.chromium.org/1942053002/diff/180001/base/message_loop/mess...
File base/message_loop/message_loop.cc (right):

https://codereview.chromium.org/1942053002/diff/180001/base/message_loop/mess...
base/message_loop/message_loop.cc:424: subtle::Release_Load(&thread_id_));
On 2016/06/03 16:34:56, alokp wrote:
> On 2016/06/03 15:31:17, gab wrote:
> > I don't think you need to use an Atomic32. Instead you can initialize
> > |thread_id_| to |PlatformThread::kInvalidThreadId| and here do:
> > 
> > if (thread_id_ == PlatformThread::kInvalidThreadId) {
> >   // |thread_id_| should already have been initialized but this thread might
> not
> > have received the update yet.
> >   subtle::MemoryBarrier();
> >   DCHECK_NE(PlatformThread::kInvalidThreadId, thread_id_);
> > }
> > return ThreadIdNameManager::GetInstance()->GetName(thread_id_);
> 
> Used subtle::MemoryBarrier() in the new patch. I did not add the DCHECK_NE
> because I think it is possible that this function is called before
> BindToCurrentThread is executed. No?
> 
> May be I should add a WARNING that message loop is not bound to any thread
yet.
> WDYT?

I think it's reasonable to document in the header that this API is only valid 
when the MessageLoop is running (and BindToCurrentThread is called before the
loop runs [1]). In which case the DCHECK makes sense.

The only place where this is not called from MessageLoop::current() is for
tracing logging when that thread is hung (and thus clearly running).

[1] https://cs.chromium.org/chromium/src/base/threading/thread.cc?rcl=0&l=233

https://codereview.chromium.org/1942053002/diff/220001/base/message_loop/mess...
File base/message_loop/message_loop.cc (right):

https://codereview.chromium.org/1942053002/diff/220001/base/message_loop/mess...
base/message_loop/message_loop.cc:418: thread_id_ = PlatformThread::CurrentId();
On 2016/06/03 16:34:56, alokp wrote:
> Do I need a memory barrier here as well?

Ah yes, let's put one and scope this code inline for documentation purposes:

{
  // Save the current thread's ID for potential use by other threads
  // later from GetThreadName().
  thread_id_ = PlatformThread::CurrentId();
  subtle::MemoryBarrier();
}

(in practice there is an implicit barrier soon enough with the loop about to run
but it's easier to read if there is one here and most likely harmless)

[alokp, 2016-06-10 22:13:00]

https://codereview.chromium.org/1942053002/diff/180001/base/message_loop/mess...
File base/message_loop/message_loop.cc (right):

https://codereview.chromium.org/1942053002/diff/180001/base/message_loop/mess...
base/message_loop/message_loop.cc:424: subtle::Release_Load(&thread_id_));
On 2016/06/07 00:30:59, gab wrote:
> On 2016/06/03 16:34:56, alokp wrote:
> > On 2016/06/03 15:31:17, gab wrote:
> > > I don't think you need to use an Atomic32. Instead you can initialize
> > > |thread_id_| to |PlatformThread::kInvalidThreadId| and here do:
> > > 
> > > if (thread_id_ == PlatformThread::kInvalidThreadId) {
> > >   // |thread_id_| should already have been initialized but this thread
might
> > not
> > > have received the update yet.
> > >   subtle::MemoryBarrier();
> > >   DCHECK_NE(PlatformThread::kInvalidThreadId, thread_id_);
> > > }
> > > return ThreadIdNameManager::GetInstance()->GetName(thread_id_);
> > 
> > Used subtle::MemoryBarrier() in the new patch. I did not add the DCHECK_NE
> > because I think it is possible that this function is called before
> > BindToCurrentThread is executed. No?
> > 
> > May be I should add a WARNING that message loop is not bound to any thread
> yet.
> > WDYT?
> 
> I think it's reasonable to document in the header that this API is only valid 
> when the MessageLoop is running (and BindToCurrentThread is called before the
> loop runs [1]). In which case the DCHECK makes sense.
> 
> The only place where this is not called from MessageLoop::current() is for
> tracing logging when that thread is hung (and thus clearly running).
> 
> [1] https://cs.chromium.org/chromium/src/base/threading/thread.cc?rcl=0&l=233

Done.

https://codereview.chromium.org/1942053002/diff/220001/base/message_loop/mess...
File base/message_loop/message_loop.cc (right):

https://codereview.chromium.org/1942053002/diff/220001/base/message_loop/mess...
base/message_loop/message_loop.cc:418: thread_id_ = PlatformThread::CurrentId();
On 2016/06/07 00:30:59, gab wrote:
> On 2016/06/03 16:34:56, alokp wrote:
> > Do I need a memory barrier here as well?
> 
> Ah yes, let's put one and scope this code inline for documentation purposes:
> 
> {
>   // Save the current thread's ID for potential use by other threads
>   // later from GetThreadName().
>   thread_id_ = PlatformThread::CurrentId();
>   subtle::MemoryBarrier();
> }
> 
> (in practice there is an implicit barrier soon enough with the loop about to
run
> but it's easier to read if there is one here and most likely harmless)

Done.

[gab, 2016-06-13 18:57:35]

lgtm w/ question, thanks!!

https://codereview.chromium.org/1942053002/diff/280001/ios/web/web_thread_imp...
File ios/web/web_thread_impl.cc (right):

https://codereview.chromium.org/1942053002/diff/280001/ios/web/web_thread_imp...
ios/web/web_thread_impl.cc:118: : Thread(message_loop->GetThreadName()),
identifier_(identifier) {
Is |message_loop| already running at this point?

[jam, 2016-06-13 19:28:39]

lgtm

[alokp, 2016-06-14 12:31:22]

https://codereview.chromium.org/1942053002/diff/280001/ios/web/web_thread_imp...
File ios/web/web_thread_impl.cc (right):

https://codereview.chromium.org/1942053002/diff/280001/ios/web/web_thread_imp...
ios/web/web_thread_impl.cc:118: : Thread(message_loop->GetThreadName()),
identifier_(identifier) {
On 2016/06/13 18:57:35, gab wrote:
> Is |message_loop| already running at this point?

Good catch. This constructor seems to be used in unittests only:
https://cs.chromium.org/chromium/src/ios/web/test/test_web_thread_bundle.cc?q...

I am going to change it to use the same pattern used in BrowserThreadImpl.

[alokp, 2016-06-14 12:31:32]

The CQ bit was checked by alokp@chromium.org

[alokp, 2016-06-14 12:31:32]

The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org,
jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1942053002/#ps320001
(title: "uses PlatformThread::GetName")

[commit-bot: I haz the power, 2016-06-14 12:31:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1942053002/320001

[commit-bot: I haz the power, 2016-06-14 14:00:04]

Description was changed from

==========
Deletes base::MessageLoop::set_thread_name().

Implements base::MessageLoop::GetThreadName by fetching the platform
thread name. This new function replaces MessageLoop::thread_name_ and
avoids having to store the thread name in multiple places.
==========

to

==========
Deletes base::MessageLoop::set_thread_name().

Implements base::MessageLoop::GetThreadName by fetching the platform
thread name. This new function replaces MessageLoop::thread_name_ and
avoids having to store the thread name in multiple places.
==========

[commit-bot: I haz the power, 2016-06-14 14:00:06]

Committed patchset #17 (id:320001)

[commit-bot: I haz the power, 2016-06-14 14:00:27]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-14 14:01:25]

Description was changed from

==========
Deletes base::MessageLoop::set_thread_name().

Implements base::MessageLoop::GetThreadName by fetching the platform
thread name. This new function replaces MessageLoop::thread_name_ and
avoids having to store the thread name in multiple places.
==========

to

==========
Deletes base::MessageLoop::set_thread_name().

Implements base::MessageLoop::GetThreadName by fetching the platform
thread name. This new function replaces MessageLoop::thread_name_ and
avoids having to store the thread name in multiple places.

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

[commit-bot: I haz the power, 2016-06-14 14:01:27]

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