Convert MediaWebContentsObsever to be the client of WakeLock mojo interface.

Convert MediaWebContentsObsever to be the client of WakeLock mojo interface.

Convert MediaWebContentsObsever to be the client of WakeLock mojo interface
instead of direct client of PowerSaveBlocker.

TEST = Play audio or video and check the OS level wake locks.
On macOS: pmset -g assertions
On Windows: powercfg /requests.

BUG=689413
TBR=<jam@chromium.org>;

Review-Url: https://codereview.chromium.org/2846813002
Cr-Commit-Position: refs/heads/master@{#470889}
Committed: https://chromium.googlesource.com/chromium/src/+/d54f936badc7666b7b0fb09c545942836e4e9943

[ke.he, 2017-04-27 09:27:29]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-27 09:27:51]

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, 2017-04-27 09:59:14]

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

[commit-bot: I haz the power, 2017-04-27 09:59:15]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[ke.he, 2017-04-28 08:33:12]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-28 08:33:24]

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, 2017-04-28 10:13:51]

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

[commit-bot: I haz the power, 2017-04-28 10:13:52]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-04-28 11:06:58]

ke.he@intel.com changed reviewers:
+ blundell@chromium.org

[ke.he, 2017-04-28 11:06:59]

Colin, PTAL, Thanks:)

[blundell, 2017-05-02 11:30:43]

blundell@chromium.org changed reviewers:
+ xhwang@chromium.org

[blundell, 2017-05-02 11:30:44]

This looks good!

+xhwang@: Do you have any suggestions on manual tests of the functionality of
MediaWebContentsObserver?

[xhwang, 2017-05-02 16:29:07]

xhwang@chromium.org changed reviewers:
+ dalecurtis@chromium.org
- xhwang@chromium.org

[xhwang, 2017-05-02 16:29:08]

xhwang -> dalecurtis, who's more familiar with this class.

[DaleCurtis, 2017-05-02 17:08:42]

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
File content/browser/media/media_web_contents_observer.cc (right):

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.cc:55: if
(audio_stream_monitor->WasRecentlyAudible()) {
{} not necessary for single-line if.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.cc:100: if
(!active_video_players_.empty()) {
Ditto

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.cc:109:
GetVideoWakeLock()->CancelWakeLock();
Put in CancelVideoLock() function like the audio one?

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.cc:175: if
(!static_cast<WebContentsImpl*>(web_contents())->IsHidden()) {
no {} necessary.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.cc:230: if (!audio_wake_lock_)
{
if (audio_wake_lock_) return audio_wake_lock_.get(); to avoid large indented
section.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.cc:232:
mojo::MakeRequest(&audio_wake_lock_);
Inline into the call below?

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.cc:233:
device::mojom::WakeLockContext* wake_lock_context =
could be written:

if (auto* context = web_contents()->GetWakeLockContext()) {
 context->GetWakeLock(...);
}

That said, is it possible for the context to be null? See my comment below.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.cc:242: return
audio_wake_lock_;
It's possible this will return nullptr if there's no context, yet none of the
other functions guard against it.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
File content/browser/media/media_web_contents_observer.h (right):

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.h:64: return
has_audio_wake_lock_for_testing_;
Why change this? YOu don't need the bool variables if you keep the
!!audio_wake_lock_, etc.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.h:94:
device::mojom::WakeLockServicePtr& GetAudioWakeLock();
Return device::mojom::WakeLockService* instead? Per-style we don't return
non-const &.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.h:96: bool
has_audio_wake_lock_for_testing_;
Variables below functions.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.h:99: // Try to request audio
wake lock.
These comments aren't really useful, so no need for them.

[DaleCurtis, 2017-05-02 17:09:31]

The unittest modified in this CL should cover all the necessary test cases. We
unfortunately don't have a full one end to end, but presumably there are already
existing wake lock tests which compare functionality to powersaveblcoker?

[blundell, 2017-05-03 09:23:50]

On 2017/05/02 17:09:31, DaleCurtis wrote:
> The unittest modified in this CL should cover all the necessary test cases. We
> unfortunately don't have a full one end to end, but presumably there are
already
> existing wake lock tests which compare functionality to powersaveblcoker?

Thanks for the response! There is indeed a browsertest of wake lock. I'm mostly
concerned here with with getting something wrong in the glue specific to
MediaWebContentsObserver such that e.g. a connection silently gets dropped on
the floor when it shouldn't. I was wondering if there was any way to manually
test the functionality that MediaWebContentsObserver is providing.

[DaleCurtis, 2017-05-03 17:29:20]

Oh for a manual test you can play audio or video and check the OS level wake
locks. pmset -g assertions does this on macOS, on Windows I think it's powercfg
/requests.

[ke.he, 2017-05-04 08:56:21]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-04 08:56:32]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ke.he, 2017-05-04 10:09:11]

Description was changed from

==========
Convert MediaWebContentsObsever to be the client of WakeLock mojo interface.

Convert MediaWebContentsObsever to be the client of WakeLock mojo interface
instead of direct client of PowerSaveBlocker.

BUG = 689413
==========

to

==========
Generalize the API of WakeLockContext mojo interface.

1) Generalize the API of WakeLockContext mojo interface, so client can pass
   type, reason and description as they want to create the WakeLockService.
2) Convert MediaWebContentsObsever to be the client of WakeLock mojo interface
   instead of direct client of PowerSaveBlocker.

BUG = 689413
==========

[ke.he, 2017-05-04 10:23:05]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-04 10:23:14]

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, 2017-05-04 11:58:13]

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

[commit-bot: I haz the power, 2017-05-04 11:58:14]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-05-04 11:58:24]

Hi, DaleCurtis@, Thanks very much. I updated this CL based on your comments.

Hi, Colin, I merged the "Generalize WakeLock API" part into this CL,
MediaWebContentsObsever is one client and can verify the wakelock
implementation.

PTAL. thanks!

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
File content/browser/media/media_web_contents_observer.cc (right):

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.cc:55: if
(audio_stream_monitor->WasRecentlyAudible()) {
On 2017/05/02 17:08:42, DaleCurtis_OOO_May_5_To_May23 wrote:
> {} not necessary for single-line if.

Done.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.cc:100: if
(!active_video_players_.empty()) {
On 2017/05/02 17:08:42, DaleCurtis_OOO_May_5_To_May23 wrote:
> Ditto

Done.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.cc:109:
GetVideoWakeLock()->CancelWakeLock();
On 2017/05/02 17:08:42, DaleCurtis_OOO_May_5_To_May23 wrote:
> Put in CancelVideoLock() function like the audio one?

Done.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.cc:175: if
(!static_cast<WebContentsImpl*>(web_contents())->IsHidden()) {
On 2017/05/02 17:08:42, DaleCurtis_OOO_May_5_To_May23 wrote:
> no {} necessary.

Done.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.cc:232:
mojo::MakeRequest(&audio_wake_lock_);
On 2017/05/02 17:08:42, DaleCurtis_OOO_May_5_To_May23 wrote:
> Inline into the call below?

See comments below:)

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.cc:233:
device::mojom::WakeLockContext* wake_lock_context =
On 2017/05/02 17:08:42, DaleCurtis_OOO_May_5_To_May23 wrote:
> could be written:
> 
> if (auto* context = web_contents()->GetWakeLockContext()) {
>  context->GetWakeLock(...);
> }
> 
> That said, is it possible for the context to be null? See my comment below.

It is safe that context to be null. the "mojo::MakeRequest(&audio_wake_lock_)"
is always called, this guarantees the "audio_wake_lock_.get()" is non-nullptr. 
In this situation, client is safe to use the audio_wake_lock_ to try the mojo
call, although it cannot connect the remote successfully.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.cc:242: return
audio_wake_lock_;
On 2017/05/02 17:08:42, DaleCurtis_OOO_May_5_To_May23 wrote:
> It's possible this will return nullptr if there's no context, yet none of the
> other functions guard against it.

The GetAudioWakeLock() never returns nullptr as comments above.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
File content/browser/media/media_web_contents_observer.h (right):

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.h:64: return
has_audio_wake_lock_for_testing_;
On 2017/05/02 17:08:42, DaleCurtis_OOO_May_5_To_May23 wrote:
> Why change this? YOu don't need the bool variables if you keep the
> !!audio_wake_lock_, etc.

The "mojo::MakeRequest(&audio_wake_lock_)" is always called, so
"audio_wake_lock_.get()" is always non-nullptr.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.h:94:
device::mojom::WakeLockServicePtr& GetAudioWakeLock();
On 2017/05/02 17:08:42, DaleCurtis_OOO_May_5_To_May23 wrote:
> Return device::mojom::WakeLockService* instead? Per-style we don't return
> non-const &.

Done.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.h:96: bool
has_audio_wake_lock_for_testing_;
On 2017/05/02 17:08:42, DaleCurtis_OOO_May_5_To_May23 wrote:
> Variables below functions.

Done.

https://codereview.chromium.org/2846813002/diff/20001/content/browser/media/m...
content/browser/media/media_web_contents_observer.h:99: // Try to request audio
wake lock.
On 2017/05/02 17:08:42, DaleCurtis_OOO_May_5_To_May23 wrote:
> These comments aren't really useful, so no need for them.

Done.

[blundell, 2017-05-04 15:46:21]

The changes in //device look great, thanks! I would actually split them into a
CL that lands before this one. The reasoning is that in case this one gets
reverted for whatever reason, those additions wouldn't then also get reverted
(which would be problematic if in the meantime we started working on other
browser process clients).

[DaleCurtis, 2017-05-04 16:53:39]

media/ stuff lgtm, defer to blundell@ for the rest (or split out).

[ke.he, 2017-05-05 07:03:09]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-05 07:03:21]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ke.he, 2017-05-05 07:30:23]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-05 07:30:33]

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, 2017-05-05 09:16:04]

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

[commit-bot: I haz the power, 2017-05-05 09:16:05]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-05-05 10:03:25]

Thanks!
I use this CL to verify issue 2843353003. After 2843353003 get landed, I'll
split //device part into a separate CL.

[blundell, 2017-05-05 12:50:30]

On 2017/05/05 10:03:25, ke.he wrote:
> Thanks!
> I use this CL to verify issue 2843353003. After 2843353003 get landed, I'll
> split //device part into a separate CL.

SGTM. Can you add a TEST= line to the CL description given Dale's instructions
for manual testing from above?

[ke.he, 2017-05-05 14:55:27]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-05 14:55:42]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ke.he, 2017-05-05 15:14:45]

Description was changed from

==========
Generalize the API of WakeLockContext mojo interface.

1) Generalize the API of WakeLockContext mojo interface, so client can pass
   type, reason and description as they want to create the WakeLockService.
2) Convert MediaWebContentsObsever to be the client of WakeLock mojo interface
   instead of direct client of PowerSaveBlocker.

BUG = 689413
==========

to

==========
Convert MediaWebContentsObsever to be the client of WakeLock mojo interface.

Convert MediaWebContentsObsever to be the client of WakeLock mojo interface
instead of direct client of PowerSaveBlocker.

TEST = Play audio or video and check the OS level wake locks.
pmset -g assertions does this on macOS, on Windows it's powercfg /requests.

BUG = 689413
==========

[commit-bot: I haz the power, 2017-05-05 17:20:39]

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

[commit-bot: I haz the power, 2017-05-05 17:20:40]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-05-09 06:25:16]

Description was changed from

==========
Convert MediaWebContentsObsever to be the client of WakeLock mojo interface.

Convert MediaWebContentsObsever to be the client of WakeLock mojo interface
instead of direct client of PowerSaveBlocker.

TEST = Play audio or video and check the OS level wake locks.
pmset -g assertions does this on macOS, on Windows it's powercfg /requests.

BUG = 689413
==========

to

==========
Convert MediaWebContentsObsever to be the client of WakeLock mojo interface.

Convert MediaWebContentsObsever to be the client of WakeLock mojo interface
instead of direct client of PowerSaveBlocker.

TEST = Play audio or video and check the OS level wake locks.
pmset -g assertions does this on macOS, on Windows it's powercfg /requests.

BUG=689413
==========

[ke.he, 2017-05-09 06:26:27]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-09 06:26:56]

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, 2017-05-09 09:14:42]

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

[commit-bot: I haz the power, 2017-05-09 09:14:43]

Dry run: This issue passed the CQ dry run.

[ke.he, 2017-05-09 09:17:16]

Description was changed from

==========
Convert MediaWebContentsObsever to be the client of WakeLock mojo interface.

Convert MediaWebContentsObsever to be the client of WakeLock mojo interface
instead of direct client of PowerSaveBlocker.

TEST = Play audio or video and check the OS level wake locks.
pmset -g assertions does this on macOS, on Windows it's powercfg /requests.

BUG=689413
==========

to

==========
Convert MediaWebContentsObsever to be the client of WakeLock mojo interface.

Convert MediaWebContentsObsever to be the client of WakeLock mojo interface
instead of direct client of PowerSaveBlocker.

TEST = Play audio or video and check the OS level wake locks.
On macOS: pmset -g assertions
On Windows: powercfg /requests.

BUG=689413
==========

[ke.he, 2017-05-09 09:19:10]

Colin, split done. PTAL.
Thanks:)

[blundell, 2017-05-09 15:59:13]

lgtm, thanks!

[ke.he, 2017-05-10 04:50:48]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-10 04:51:12]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ke.he, 2017-05-10 05:23:41]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-10 05:24:21]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ke.he, 2017-05-10 07:10:55]

MediaWebContentsObsever pass mojom type when RequestWakeLock().
PTAL.

[ke.he, 2017-05-10 07:12:01]

On 2017/05/10 07:10:55, ke.he wrote:
> MediaWebContentsObsever pass mojom type when RequestWakeLock().
> PTAL.

pass mojom type when GetWakeLock(). not RequestWakeLock().

[ke.he, 2017-05-10 07:20:09]

The CQ bit was unchecked by ke.he@intel.com

[blundell, 2017-05-10 14:45:57]

still lgtm, thanks!

[ke.he, 2017-05-11 02:02:04]

ke.he@intel.com changed reviewers:
+ jam@chromium.org

[ke.he, 2017-05-11 02:02:05]

Hi, Jam@, PTAL on the //contents, thanks!

[ke.he, 2017-05-11 04:11:49]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 04:12:35]

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, 2017-05-11 06:33:44]

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

[commit-bot: I haz the power, 2017-05-11 06:33:45]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[blundell, 2017-05-11 06:51:26]

Hi Ke He,

It's fine to TBR jam@ on the changes to the WebContentsImpl unittest per
https://chromium.googlesource.com/chromium/src/+/master/docs/code_reviews.md#...
(in particular, the section about mechanical changes to callers of an API after
you've gotten an OWNERS review on the API change).

Remember that whenever you use TBR you should specify in the email to the TBR'd
reviewer what you're TBR'ing for, as often it's only a small part of a bigger
change (as is the case here).

Thanks,

Colin

[ke.he, 2017-05-11 08:07:37]

Description was changed from

==========
Convert MediaWebContentsObsever to be the client of WakeLock mojo interface.

Convert MediaWebContentsObsever to be the client of WakeLock mojo interface
instead of direct client of PowerSaveBlocker.

TEST = Play audio or video and check the OS level wake locks.
On macOS: pmset -g assertions
On Windows: powercfg /requests.

BUG=689413
==========

to

==========
Convert MediaWebContentsObsever to be the client of WakeLock mojo interface.

Convert MediaWebContentsObsever to be the client of WakeLock mojo interface
instead of direct client of PowerSaveBlocker.

TEST = Play audio or video and check the OS level wake locks.
On macOS: pmset -g assertions
On Windows: powercfg /requests.

BUG=689413
TBR=<jam@chromium.org>
==========

[ke.he, 2017-05-11 08:08:20]

Got it, thanks:)

[ke.he, 2017-05-11 09:19:17]

The CQ bit was checked by ke.he@intel.com

[ke.he, 2017-05-11 09:19:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/2846813002/#ps200001
(title: "remove dependency, code rebase")

[commit-bot: I haz the power, 2017-05-11 09:19:55]

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, 2017-05-11 09:25:16]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1494494357140910,
"parent_rev": "e563fd2fa55fd2d327c0f44bcfe6c4c98805bb55", "commit_rev":
"d54f936badc7666b7b0fb09c545942836e4e9943"}

[commit-bot: I haz the power, 2017-05-11 09:25:29]

Description was changed from

==========
Convert MediaWebContentsObsever to be the client of WakeLock mojo interface.

Convert MediaWebContentsObsever to be the client of WakeLock mojo interface
instead of direct client of PowerSaveBlocker.

TEST = Play audio or video and check the OS level wake locks.
On macOS: pmset -g assertions
On Windows: powercfg /requests.

BUG=689413
TBR=<jam@chromium.org>
==========

to

==========
Convert MediaWebContentsObsever to be the client of WakeLock mojo interface.

Convert MediaWebContentsObsever to be the client of WakeLock mojo interface
instead of direct client of PowerSaveBlocker.

TEST = Play audio or video and check the OS level wake locks.
On macOS: pmset -g assertions
On Windows: powercfg /requests.

BUG=689413
TBR=<jam@chromium.org>

Review-Url: https://codereview.chromium.org/2846813002
Cr-Commit-Position: refs/heads/master@{#470889}
Committed:
https://chromium.googlesource.com/chromium/src/+/d54f936badc7666b7b0fb09c5459...
==========

[commit-bot: I haz the power, 2017-05-11 09:25:30]

Committed patchset #11 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/d54f936badc7666b7b0fb09c5459...

[blundell, 2017-05-11 11:52:04]

TBR=jam@ for mechanical changes to web_contents_impl_unittest.cc

(Ke He, that's the kind of message I was referring to upthread :)

[blundell, 2017-05-12 07:51:16]

FYI, I tested this manually on my Macbook using today's canary (3097, includes
this change) using Dale's instructions. It works as expected, i.e., the same way
as stable does on the Macbook.

\o/