[Remoting Android] DisplayUpdaterFactory cleanups

[Remoting Android] DisplayUpdaterFactory cleanups

This CL resolves comments in 2055143002 after CL being committed:

* Move Initialize() function to protocol::VideoRenderer interface
* Rename JniFrameConsumer to JniVideoRenderer
* Resolves other comments

BUG=385924
Committed: https://crrev.com/9915862a79639f07174a2cfbc210d78389783501
Cr-Commit-Position: refs/heads/master@{#403263}

[Yuwei, 2016-06-28 00:18:34]

yuweih@chromium.org changed reviewers:
+ sergeyu@chromium.org

[Yuwei, 2016-06-28 00:18:35]

ptal

https://codereview.chromium.org/2101833002/diff/1/remoting/client/jni/jni_vid...
File remoting/client/jni/jni_video_renderer.h (right):

https://codereview.chromium.org/2101833002/diff/1/remoting/client/jni/jni_vid...
remoting/client/jni/jni_video_renderer.h:20: class ClientContext;
This is from the old JniFrameConsumer. The code review tool is confused by the
old interface.

[Sergey Ulanov, 2016-06-28 17:59:50]

Thanks for the cleanups!

https://codereview.chromium.org/2101833002/diff/1/remoting/client/jni/jni_vid...
File remoting/client/jni/jni_video_renderer.cc (right):

https://codereview.chromium.org/2101833002/diff/1/remoting/client/jni/jni_vid...
remoting/client/jni/jni_video_renderer.cc:1: // Copyright 2013 The Chromium
Authors. All rights reserved.
update year.

https://codereview.chromium.org/2101833002/diff/1/remoting/client/jni/jni_vid...
File remoting/client/jni/jni_video_renderer.h (right):

https://codereview.chromium.org/2101833002/diff/1/remoting/client/jni/jni_vid...
remoting/client/jni/jni_video_renderer.h:1: // Copyright 2013 The Chromium
Authors. All rights reserved.
2016
Year needs to be updated whenever a file is renamed/moved

https://codereview.chromium.org/2101833002/diff/1/remoting/client/jni/jni_vid...
remoting/client/jni/jni_video_renderer.h:71: #endif
add // REMOTING_CLIENT_JNI_JNI_VIDEO_RENDERER_H_

https://codereview.chromium.org/2101833002/diff/1/remoting/client/plugin/chro...
File remoting/client/plugin/chromoting_instance.cc (right):

https://codereview.chromium.org/2101833002/diff/1/remoting/client/plugin/chro...
remoting/client/plugin/chromoting_instance.cc:646: video_renderer_.reset();
add {}

https://codereview.chromium.org/2101833002/diff/1/remoting/client/plugin/pepp...
File remoting/client/plugin/pepper_video_renderer.h (right):

https://codereview.chromium.org/2101833002/diff/1/remoting/client/plugin/pepp...
remoting/client/plugin/pepper_video_renderer.h:51: virtual bool
InitializePepper(pp::Instance* instance,
This now overlaps with Initialize(). Remove |context| and |perf_tracker|
arguments from this methods. Then ChromotingInstance will need to call both
InitializePepper() and then Initialize()
Also maybe better to call it SetPepperContext() or something like that, to make
the purpose clearer.

https://codereview.chromium.org/2101833002/diff/1/remoting/protocol/video_ren...
File remoting/protocol/video_renderer.h (right):

https://codereview.chromium.org/2101833002/diff/1/remoting/protocol/video_ren...
remoting/protocol/video_renderer.h:31: virtual void Initialize(const
ClientContext& client_context,
Return bool here as Initialization may fail (for the pepper implementation of
this interface).

https://codereview.chromium.org/2101833002/diff/1/remoting/protocol/video_ren...
remoting/protocol/video_renderer.h:32: protocol::PerformanceTracker*
perf_tracker) {}
Please make this pure virtual. Default stub implementation in an interface is
acceptable for optional methods in event-handler interfaces, but in most other
cases all methods in interfaces should be pure virtual.

[Yuwei, 2016-06-28 18:46:06]

ptal

https://codereview.chromium.org/2101833002/diff/1/remoting/client/jni/jni_vid...
File remoting/client/jni/jni_video_renderer.cc (right):

https://codereview.chromium.org/2101833002/diff/1/remoting/client/jni/jni_vid...
remoting/client/jni/jni_video_renderer.cc:1: // Copyright 2013 The Chromium
Authors. All rights reserved.
On 2016/06/28 17:59:50, Sergey Ulanov wrote:
> update year.

Done.

https://codereview.chromium.org/2101833002/diff/1/remoting/client/jni/jni_vid...
File remoting/client/jni/jni_video_renderer.h (right):

https://codereview.chromium.org/2101833002/diff/1/remoting/client/jni/jni_vid...
remoting/client/jni/jni_video_renderer.h:1: // Copyright 2013 The Chromium
Authors. All rights reserved.
On 2016/06/28 17:59:50, Sergey Ulanov wrote:
> 2016
> Year needs to be updated whenever a file is renamed/moved

Done.

https://codereview.chromium.org/2101833002/diff/1/remoting/client/jni/jni_vid...
remoting/client/jni/jni_video_renderer.h:71: #endif
On 2016/06/28 17:59:50, Sergey Ulanov wrote:
> add // REMOTING_CLIENT_JNI_JNI_VIDEO_RENDERER_H_

Done.

https://codereview.chromium.org/2101833002/diff/1/remoting/client/plugin/chro...
File remoting/client/plugin/chromoting_instance.cc (right):

https://codereview.chromium.org/2101833002/diff/1/remoting/client/plugin/chro...
remoting/client/plugin/chromoting_instance.cc:646: video_renderer_.reset();
On 2016/06/28 17:59:50, Sergey Ulanov wrote:
> add {}

Done.

https://codereview.chromium.org/2101833002/diff/1/remoting/client/plugin/pepp...
File remoting/client/plugin/pepper_video_renderer.h (right):

https://codereview.chromium.org/2101833002/diff/1/remoting/client/plugin/pepp...
remoting/client/plugin/pepper_video_renderer.h:51: virtual bool
InitializePepper(pp::Instance* instance,
On 2016/06/28 17:59:50, Sergey Ulanov wrote:
> This now overlaps with Initialize(). Remove |context| and |perf_tracker|
> arguments from this methods. Then ChromotingInstance will need to call both
> InitializePepper() and then Initialize()
> Also maybe better to call it SetPepperContext() or something like that, to
make
> the purpose clearer.

Done.

https://codereview.chromium.org/2101833002/diff/1/remoting/protocol/video_ren...
File remoting/protocol/video_renderer.h (right):

https://codereview.chromium.org/2101833002/diff/1/remoting/protocol/video_ren...
remoting/protocol/video_renderer.h:31: virtual void Initialize(const
ClientContext& client_context,
On 2016/06/28 17:59:50, Sergey Ulanov wrote:
> Return bool here as Initialization may fail (for the pepper implementation of
> this interface).

Done.

https://codereview.chromium.org/2101833002/diff/1/remoting/protocol/video_ren...
remoting/protocol/video_renderer.h:32: protocol::PerformanceTracker*
perf_tracker) {}
On 2016/06/28 17:59:50, Sergey Ulanov wrote:
> Please make this pure virtual. Default stub implementation in an interface is
> acceptable for optional methods in event-handler interfaces, but in most other
> cases all methods in interfaces should be pure virtual.

Done.

[Yuwei, 2016-06-28 18:46:31]

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

[commit-bot: I haz the power, 2016-06-28 18:47:58]

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

[commit-bot: I haz the power, 2016-06-28 19:07:53]

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

[commit-bot: I haz the power, 2016-06-28 19:07:54]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Sergey Ulanov, 2016-06-28 19:08:02]

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/plugin/...
File remoting/client/plugin/chromoting_instance.cc (right):

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/plugin/...
remoting/client/plugin/chromoting_instance.cc:637:
!video_renderer_->SetPepperContext(this, this)) {
Call SetPepperContext() before Initialize()

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/plugin/...
File remoting/client/plugin/pepper_video_renderer_2d.cc (right):

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/plugin/...
remoting/client/plugin/pepper_video_renderer_2d.cc:106:
software_video_renderer_.reset(new SoftwareVideoRenderer(
Since now SoftwareVideoRenderer() now has Initialize() it can be created from
the PepperVideoRenderer2D constructor and it doesn't need to be allocated
separately on heap.

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/plugin/...
remoting/client/plugin/pepper_video_renderer_2d.cc:108: 
Need to call software_video_renderer_->Initialize() here.

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/plugin/...
File remoting/client/plugin/pepper_video_renderer_3d.cc (right):

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/plugin/...
remoting/client/plugin/pepper_video_renderer_3d.cc:81: bool
PepperVideoRenderer3D::SetPepperContext(
It's strange that Set* method returns boolean value. Please change
SetPepperContext() return type to void and move pepper initialization code to
Initialize()

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/softwar...
File remoting/client/software_video_renderer.h (right):

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/softwar...
remoting/client/software_video_renderer.h:50: protocol::PerformanceTracker*
perf_tracker);
remove decode_task_runner and perf_tracker, since now they are passed to
Initialize()

[Yuwei, 2016-06-28 21:37:20]

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

[commit-bot: I haz the power, 2016-06-28 21:38:26]

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

[Yuwei, 2016-06-28 21:43:03]

ptal

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/plugin/...
File remoting/client/plugin/chromoting_instance.cc (right):

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/plugin/...
remoting/client/plugin/chromoting_instance.cc:637:
!video_renderer_->SetPepperContext(this, this)) {
On 2016/06/28 19:08:01, Sergey Ulanov wrote:
> Call SetPepperContext() before Initialize()

Done.

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/plugin/...
File remoting/client/plugin/pepper_video_renderer_2d.cc (right):

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/plugin/...
remoting/client/plugin/pepper_video_renderer_2d.cc:106:
software_video_renderer_.reset(new SoftwareVideoRenderer(
On 2016/06/28 19:08:01, Sergey Ulanov wrote:
> Since now SoftwareVideoRenderer() now has Initialize() it can be created from
> the PepperVideoRenderer2D constructor and it doesn't need to be allocated
> separately on heap.

Done.

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/plugin/...
remoting/client/plugin/pepper_video_renderer_2d.cc:108: 
On 2016/06/28 19:08:01, Sergey Ulanov wrote:
> Need to call software_video_renderer_->Initialize() here.

Done.

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/plugin/...
File remoting/client/plugin/pepper_video_renderer_3d.cc (right):

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/plugin/...
remoting/client/plugin/pepper_video_renderer_3d.cc:81: bool
PepperVideoRenderer3D::SetPepperContext(
On 2016/06/28 19:08:01, Sergey Ulanov wrote:
> It's strange that Set* method returns boolean value. Please change
> SetPepperContext() return type to void and move pepper initialization code to
> Initialize()

Done. Store pp::Instance* here and use later in Initialize()

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/softwar...
File remoting/client/software_video_renderer.h (right):

https://codereview.chromium.org/2101833002/diff/20001/remoting/client/softwar...
remoting/client/software_video_renderer.h:50: protocol::PerformanceTracker*
perf_tracker);
On 2016/06/28 19:08:01, Sergey Ulanov wrote:
> remove decode_task_runner and perf_tracker, since now they are passed to
> Initialize()

Done. Looks like SoftwareVideoRenderer is used in several places and also in the
iOS repo so I left a legacy constructor here.

[commit-bot: I haz the power, 2016-06-28 22:06:58]

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

[commit-bot: I haz the power, 2016-06-28 22:06:59]

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

[Yuwei, 2016-06-28 22:22:44]

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

[commit-bot: I haz the power, 2016-06-28 22:24:37]

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

[Yuwei, 2016-06-28 23:00:45]

The CQ bit was unchecked by yuweih@chromium.org

[Yuwei, 2016-06-28 23:01:27]

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

[commit-bot: I haz the power, 2016-06-28 23:04:11]

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

[commit-bot: I haz the power, 2016-06-29 01:05:07]

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

[commit-bot: I haz the power, 2016-06-29 01:05:08]

Dry run: This issue passed the CQ dry run.

[Sergey Ulanov, 2016-06-29 23:02:13]

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/jni/jni...
File remoting/client/jni/jni_video_renderer.cc (right):

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/jni/jni...
remoting/client/jni/jni_video_renderer.cc:98: video_renderer_(this),
call this software_video_renderer_?

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/jni/jni...
remoting/client/jni/jni_video_renderer.cc:150: return true;
return video_renderer_.Initialize(context, perf_tracker);

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/plugin/...
File remoting/client/plugin/pepper_video_renderer_2d.cc (right):

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/plugin/...
remoting/client/plugin/pepper_video_renderer_2d.cc:104:
software_video_renderer_.Initialize(client_context, perf_tracker);
return the result.

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/plugin/...
File remoting/client/plugin/pepper_video_renderer_3d.h (right):

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/plugin/...
remoting/client/plugin/pepper_video_renderer_3d.h:94: pp::Instance* instance_ =
nullptr;
call it pp_instance_ please

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/softwar...
File remoting/client/software_video_renderer.cc (right):

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/softwar...
remoting/client/software_video_renderer.cc:51:
thread_checker_.DetachFromThread();
Why do you need this?

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/softwar...
File remoting/client/software_video_renderer.h (right):

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/softwar...
remoting/client/software_video_renderer.h:47: // Deprecated constructor.
TODO(yuweih): remove.
Remove it in this CL?

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/softwar...
remoting/client/software_video_renderer.h:64: // case.
Don't need to duplicate documentation when implementing interfaces. This method
is already documented in video_renderer.h

https://codereview.chromium.org/2101833002/diff/80001/remoting/protocol/video...
File remoting/protocol/video_renderer.h (right):

https://codereview.chromium.org/2101833002/diff/80001/remoting/protocol/video...
remoting/protocol/video_renderer.h:9: class ClientContext;
nit: add empty line here

[Yuwei, 2016-06-29 23:38:03]

ptal

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/jni/jni...
File remoting/client/jni/jni_video_renderer.cc (right):

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/jni/jni...
remoting/client/jni/jni_video_renderer.cc:98: video_renderer_(this),
On 2016/06/29 23:02:12, Sergey Ulanov wrote:
> call this software_video_renderer_? 

Done.

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/jni/jni...
remoting/client/jni/jni_video_renderer.cc:150: return true;
On 2016/06/29 23:02:12, Sergey Ulanov wrote:
> return video_renderer_.Initialize(context, perf_tracker);

Done.

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/plugin/...
File remoting/client/plugin/pepper_video_renderer_2d.cc (right):

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/plugin/...
remoting/client/plugin/pepper_video_renderer_2d.cc:104:
software_video_renderer_.Initialize(client_context, perf_tracker);
On 2016/06/29 23:02:12, Sergey Ulanov wrote:
> return the result.

Done.

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/plugin/...
File remoting/client/plugin/pepper_video_renderer_3d.h (right):

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/plugin/...
remoting/client/plugin/pepper_video_renderer_3d.h:94: pp::Instance* instance_ =
nullptr;
On 2016/06/29 23:02:12, Sergey Ulanov wrote:
> call it pp_instance_ please

Done.

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/softwar...
File remoting/client/software_video_renderer.cc (right):

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/softwar...
remoting/client/software_video_renderer.cc:51:
thread_checker_.DetachFromThread();
On 2016/06/29 23:02:12, Sergey Ulanov wrote:
> Why do you need this? 

I think after adding Initialize, the renderer can be created on a different
thread?

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/softwar...
File remoting/client/software_video_renderer.h (right):

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/softwar...
remoting/client/software_video_renderer.h:47: // Deprecated constructor.
TODO(yuweih): remove.
On 2016/06/29 23:02:12, Sergey Ulanov wrote:
> Remove it in this CL?

Not sure whether we can do it... Looks like the iOS code is using this ctor?

https://chromoting-internal.git.corp.google.com/chromoting-ios/+/master/bridg...

https://codereview.chromium.org/2101833002/diff/80001/remoting/client/softwar...
remoting/client/software_video_renderer.h:64: // case.
On 2016/06/29 23:02:12, Sergey Ulanov wrote:
> Don't need to duplicate documentation when implementing interfaces. This
method
> is already documented in video_renderer.h

Done.

https://codereview.chromium.org/2101833002/diff/80001/remoting/protocol/video...
File remoting/protocol/video_renderer.h (right):

https://codereview.chromium.org/2101833002/diff/80001/remoting/protocol/video...
remoting/protocol/video_renderer.h:9: class ClientContext;
On 2016/06/29 23:02:12, Sergey Ulanov wrote:
> nit: add empty line here

Done. Also added one below namespace protocol. Not sure what is the guideline
for adding empty lines?

[Yuwei, 2016-06-30 07:51:02]

https://codereview.chromium.org/2101833002/diff/100001/remoting/client/jni/jn...
File remoting/client/jni/jni_display_handler.cc (right):

https://codereview.chromium.org/2101833002/diff/100001/remoting/client/jni/jn...
remoting/client/jni/jni_display_handler.cc:96: new JniVideoRenderer(runtime_,
weak_factory_.GetWeakPtr()));
After separating initialization code from the ctor of SoftwareVideoRenderer,
looks like we don't even need to make JniFrameConsumer a video renderer, and
here we can simply return an uninitialized software renderer like

return base::WrapUnique(new SoftwareVideoRenderer(frame_consumer));

The slight problem is that it doesn't take ownership of the frame consumer and
JniDisplayHandler will need to delete the consumer on the network thread.

[Sergey Ulanov, 2016-06-30 19:03:09]

LGTM

[Yuwei, 2016-06-30 19:13:54]

https://codereview.chromium.org/2101833002/diff/100001/remoting/client/jni/jn...
File remoting/client/jni/jni_display_handler.cc (right):

https://codereview.chromium.org/2101833002/diff/100001/remoting/client/jni/jn...
remoting/client/jni/jni_display_handler.cc:96: new JniVideoRenderer(runtime_,
weak_factory_.GetWeakPtr()));
On 2016/06/30 07:51:02, Yuwei wrote:
> After separating initialization code from the ctor of SoftwareVideoRenderer,
> looks like we don't even need to make JniFrameConsumer a video renderer, and
> here we can simply return an uninitialized software renderer like
> 
> return base::WrapUnique(new SoftwareVideoRenderer(frame_consumer));
> 
> The slight problem is that it doesn't take ownership of the frame consumer and
> JniDisplayHandler will need to delete the consumer on the network thread.

I will hold off this change to a future CL. Currently SoftwareVideoRenderer
takes in a raw pointer so it must not outlive the underlying frame consumer,
while the user of DisplayUpdaterFactory doesn't need to know anything about the
underlying frame consumer. Maybe later we can make SoftwareVideoRenderer take in
WeakPtr of frame consumer but that will involve lots of code changes of existing
frame consumers...

[Yuwei, 2016-06-30 19:14:56]

The CQ bit was checked by yuweih@chromium.org

[commit-bot: I haz the power, 2016-06-30 19:15:40]

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-06-30 20:09:57]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-06-30 20:10:21]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-06-30 20:11:51]

Description was changed from

==========
[Remoting Android] DisplayUpdaterFactory cleanups

This CL resolves comments in 2055143002 after CL being committed:

* Move Initialize() function to protocol::VideoRenderer interface
* Rename JniFrameConsumer to JniVideoRenderer
* Resolves other comments

BUG=385924
==========

to

==========
[Remoting Android] DisplayUpdaterFactory cleanups

This CL resolves comments in 2055143002 after CL being committed:

* Move Initialize() function to protocol::VideoRenderer interface
* Rename JniFrameConsumer to JniVideoRenderer
* Resolves other comments

BUG=385924

Committed: https://crrev.com/9915862a79639f07174a2cfbc210d78389783501
Cr-Commit-Position: refs/heads/master@{#403263}
==========

[commit-bot: I haz the power, 2016-06-30 20:11:52]

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