[Presentation API] (1-UA) fire PresentationConnection onterminate event if receiver page gets destr…

[Presentation API] (1-UA) fire PresentationConnection onterminate event if receiver page gets destroyed

We need to fire 'onterminate' event on controller frame if (Please add scenarios that are missing):

Controller initiates termination:

1. call connection.terminate() in js
2. click 'stop' on MR dialog
(closing controller frame will close but not terminate connection)

1. and 2. are equivalent in the sense that connection.terminate() just calls router->terminateRoute(). Cast MRP will notify state change and fire 'onterminate' event in these cases. (Existing implementation covers this)

Receiver initiates termination:

1. receiver frame gets destroyed, e.g. call window.close() in js
2. call connection.terminate() in js

We can make 1. and 2. equivalent, by having connection.terminate() call window.close(). When offscreen_tab is gone, cast MRP will get notified (mirroring has been stopped) and notify controller about state change.

BUG=588874
Review-Url: https://codereview.chromium.org/2484273003
Cr-Commit-Position: refs/heads/master@{#454497}
Committed: https://chromium.googlesource.com/chromium/src/+/ad129a062423b83cf71bbf1f3e114ea7d8828fab

[zhaobin, 2016-11-08 22:56:30]

zhaobin@chromium.org changed reviewers:
+ imcheng@chromium.org, mfoltz@chromium.org

[haraken, 2016-11-08 23:44:50]

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

[haraken, 2016-11-08 23:44:50]

Drive-by

https://codereview.chromium.org/2484273003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp
(right):

https://codereview.chromium.org/2484273003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp:73: if
(!window->closed())

You need to check if frame(), frame()->domWindow() and frame()->document() are
not null.

[zhaobin, 2016-11-09 03:40:12]

https://codereview.chromium.org/2484273003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp
(right):

https://codereview.chromium.org/2484273003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp:73: if
(!window->closed())
On 2016/11/08 23:44:50, haraken wrote:
> 
> You need to check if frame(), frame()->domWindow() and frame()->document() are
> not null.

Done.

[imcheng, 2016-11-28 22:12:01]

Some preliminary comments but I didn't look too far since I'd imagine this would
change along with the other CLs.

https://codereview.chromium.org/2484273003/diff/20001/content/browser/present...
File content/browser/presentation/presentation_service_impl.cc (right):

https://codereview.chromium.org/2484273003/diff/20001/content/browser/present...
content/browser/presentation/presentation_service_impl.cc:580: if
(receiver_delegate_) {
If the receiver frame is already being destroyed then isn't this unnecessary?

https://codereview.chromium.org/2484273003/diff/20001/content/renderer/presen...
File content/renderer/presentation/presentation_dispatcher.cc (right):

https://codereview.chromium.org/2484273003/diff/20001/content/renderer/presen...
content/renderer/presentation/presentation_dispatcher.cc:263:
receiver_->closeWindow();
If the receiver frame calls terminate, how do we ensure the controllers'
connection state goes to terminated? I feel like this should go through to
MediaRouter / OPM to clean up the offscreen tab and notify controllers.

https://codereview.chromium.org/2484273003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp
(right):

https://codereview.chromium.org/2484273003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp:65:
DCHECK(state == WebPresentationConnectionState::Terminated);
Is this temporary? Please add a TODO if so

https://codereview.chromium.org/2484273003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp:68:
connection->didChangeState(state);
will you also need to close the window if the connection state changes to
terminated?

[haraken, 2016-11-29 02:15:50]

https://codereview.chromium.org/2484273003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp
(right):

https://codereview.chromium.org/2484273003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp:80: if
(!document)

This check won't be needed. You can pass in nullptr to window->close().

[mark a. foltz, 2017-02-14 21:43:00]

It sounds like this is still relevant to land - but needs to be rebased to
reflect the actual implementation.  Do you have time to pick it up?  Or I can
take it on.

[zhaobin, 2017-02-14 22:31:58]

On 2017/02/14 21:43:00, mark a. foltz wrote:
> It sounds like this is still relevant to land - but needs to be rebased to
> reflect the actual implementation.  Do you have time to pick it up?  Or I can
> take it on.

Sure. I will rebase.

[zhaobin, 2017-02-16 00:42:31]

https://codereview.chromium.org/2484273003/diff/20001/content/browser/present...
File content/browser/presentation/presentation_service_impl.cc (right):

https://codereview.chromium.org/2484273003/diff/20001/content/browser/present...
content/browser/presentation/presentation_service_impl.cc:580: if
(receiver_delegate_) {
On 2016/11/28 22:12:00, imcheng wrote:
> If the receiver frame is already being destroyed then isn't this unnecessary?

This function is called before destroying PresentationDispatcher and
PresentationReceiver. Adding state change here could invoke receiver
connection's onterminate handler...

https://codereview.chromium.org/2484273003/diff/20001/content/renderer/presen...
File content/renderer/presentation/presentation_dispatcher.cc (right):

https://codereview.chromium.org/2484273003/diff/20001/content/renderer/presen...
content/renderer/presentation/presentation_dispatcher.cc:263:
receiver_->closeWindow();
On 2016/11/28 22:12:01, imcheng wrote:
> If the receiver frame calls terminate, how do we ensure the controllers'
> connection state goes to terminated? I feel like this should go through to
> MediaRouter / OPM to clean up the offscreen tab and notify controllers.

closeWindow() closes offscreen tab and MRP will signal controller that
connection is terminated (It is the same as close normal tab while we are
mirroring).

https://codereview.chromium.org/2484273003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp
(right):

https://codereview.chromium.org/2484273003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp:65:
DCHECK(state == WebPresentationConnectionState::Terminated);
On 2016/11/28 22:12:01, imcheng wrote:
> Is this temporary? Please add a TODO if so

Done.

https://codereview.chromium.org/2484273003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp:68:
connection->didChangeState(state);
On 2016/11/28 22:12:01, imcheng wrote:
> will you also need to close the window if the connection state changes to
> terminated?

No. This is invoked when destroying receiver frame (invoked by receiver frame's
PSImpl::Reset()).

https://codereview.chromium.org/2484273003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp:80: if
(!document)
On 2016/11/29 02:15:50, haraken wrote:
> 
> This check won't be needed. You can pass in nullptr to window->close().

Done.

[imcheng, 2017-02-24 19:20:33]

The controller-initiated terminate logic makes sense but I would like to get
some clarification on receiver-initiated terminate logic, particularly around
navigation.

https://codereview.chromium.org/2484273003/diff/20001/content/browser/present...
File content/browser/presentation/presentation_service_impl.cc (right):

https://codereview.chromium.org/2484273003/diff/20001/content/browser/present...
content/browser/presentation/presentation_service_impl.cc:580: if
(receiver_delegate_) {
On 2017/02/16 00:42:30, zhaobin wrote:
> On 2016/11/28 22:12:00, imcheng wrote:
> > If the receiver frame is already being destroyed then isn't this
unnecessary?
> 
> This function is called before destroying PresentationDispatcher and
> PresentationReceiver. Adding state change here could invoke receiver
> connection's onterminate handler...

I see. Note that Reset() also gets called on navigation.
- In that case does the offscreen tab capture stop?
- Does the controller side get notified of state change?
- Should we allow the offscreen tab to navigate at all?

https://codereview.chromium.org/2484273003/diff/20001/content/browser/present...
content/browser/presentation/presentation_service_impl.cc:580: if
(receiver_delegate_) {
On 2017/02/16 00:42:30, zhaobin wrote:
> On 2016/11/28 22:12:00, imcheng wrote:
> > If the receiver frame is already being destroyed then isn't this
unnecessary?
> 
> This function is called before destroying PresentationDispatcher and
> PresentationReceiver. Adding state change here could invoke receiver
> connection's onterminate handler...

It feels like we should be able to observe for this case in the renderer /
PresentationDispatcher and notify the connection object(s) directly without
going through Mojo.

https://codereview.chromium.org/2484273003/diff/40001/content/renderer/presen...
File content/renderer/presentation/presentation_dispatcher.cc (right):

https://codereview.chromium.org/2484273003/diff/40001/content/renderer/presen...
content/renderer/presentation/presentation_dispatcher.cc:564: if (!controller_)
this check no longer needed?

[zhaobin, 2017-02-25 01:31:03]

https://codereview.chromium.org/2484273003/diff/20001/content/browser/present...
File content/browser/presentation/presentation_service_impl.cc (right):

https://codereview.chromium.org/2484273003/diff/20001/content/browser/present...
content/browser/presentation/presentation_service_impl.cc:580: if
(receiver_delegate_) {
On 2017/02/24 19:20:33, imcheng wrote:
> On 2017/02/16 00:42:30, zhaobin wrote:
> > On 2016/11/28 22:12:00, imcheng wrote:
> > > If the receiver frame is already being destroyed then isn't this
> unnecessary?
> > 
> > This function is called before destroying PresentationDispatcher and
> > PresentationReceiver. Adding state change here could invoke receiver
> > connection's onterminate handler...
> 
> It feels like we should be able to observe for this case in the renderer /
> PresentationDispatcher and notify the connection object(s) directly without
> going through Mojo.

Move state change to PresentationDispatcher.

Controller will get notified by MRP.

Setting window.location.href on receiver page does not work. There seems to be
some mechanism disable navigation from offscreen_tab.

https://codereview.chromium.org/2484273003/diff/40001/content/renderer/presen...
File content/renderer/presentation/presentation_dispatcher.cc (right):

https://codereview.chromium.org/2484273003/diff/40001/content/renderer/presen...
content/renderer/presentation/presentation_dispatcher.cc:564: if (!controller_)
On 2017/02/24 19:20:33, imcheng wrote:
> this check no longer needed?

Done.

[mark a. foltz, 2017-02-25 01:43:12]

https://codereview.chromium.org/2484273003/diff/20001/content/browser/present...
File content/browser/presentation/presentation_service_impl.cc (right):

https://codereview.chromium.org/2484273003/diff/20001/content/browser/present...
content/browser/presentation/presentation_service_impl.cc:580: if
(receiver_delegate_) {
On 2017/02/24 at 19:20:33, imcheng wrote:
> On 2017/02/16 00:42:30, zhaobin wrote:
> > On 2016/11/28 22:12:00, imcheng wrote:
> > > If the receiver frame is already being destroyed then isn't this
unnecessary?
> > 
> > This function is called before destroying PresentationDispatcher and
> > PresentationReceiver. Adding state change here could invoke receiver
> > connection's onterminate handler...
> 
> I see. Note that Reset() also gets called on navigation.
> - In that case does the offscreen tab capture stop?

The offscreen tab should be closed if it attempts top-level navigation.  (I can
check the spec for the exact wording)

> - Does the controller side get notified of state change?
> - Should we allow the offscreen tab to navigate at all?

No.  However on close reading of the spec we don't have a flag blocking this. 
Filed https://github.com/w3c/presentation-api/issues/414 to update the spec.

https://codereview.chromium.org/2484273003/diff/40001/content/browser/present...
File content/browser/presentation/presentation_service_impl.cc (right):

https://codereview.chromium.org/2484273003/diff/40001/content/browser/present...
content/browser/presentation/presentation_service_impl.cc:561:
PresentationSessionInfo dummy_session(GURL("fake_url"), "fake_id");
This is another case where having separate mojoms for the controller and
receiver would make things clearer (no need to pass a dummy connection).  Can
you add a TODO to remove this when the refactoring is done?

https://codereview.chromium.org/2484273003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp
(right):

https://codereview.chromium.org/2484273003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp:70:
connection->didChangeState(state);
Will this fire a terminated event on the receiver connections?  The spec doesn't
actually require the browser to do that; since the window is being closed
anyway, it didn't make sense to run additional event loops, in addition to
onbeforeunload etc.

https://codereview.chromium.org/2484273003/diff/40001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/presentation/WebPresentationReceiver.h
(right):

https://codereview.chromium.org/2484273003/diff/40001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/presentation/WebPresentationReceiver.h:31:
virtual void closeWindow() = 0;
Would prefer terminateConnection() for consistency with WebPresentationClient
(replacing "session" with "connection" as the spec does now.)

[mark a. foltz, 2017-02-25 01:47:35]

Would it be possible to write a layout test that verifies that a receiver frame
is closed on terminate() and sets the connection status to "terminated" in the
onbeforeunload handler?  It might be tricky to write - you would have to open a
window so the test doesn't terminate itself before completion?  Or maybe there
are other layout test cases for onbeforeunload behavior that have a solution.

For bonus points test terminate() on a controller connection (requires some
additional mocking on the mojo side.)

[zhaobin, 2017-02-28 04:25:19]

Added layout test for connection.terminate().

Receiver connection state changes to 'terminated' after onbeforeunload() and
before onunload() (Could not find a place to hook state transition between
domWindow.close() and dispatching onbeforeunload event).

https://codereview.chromium.org/2484273003/diff/40001/content/browser/present...
File content/browser/presentation/presentation_service_impl.cc (right):

https://codereview.chromium.org/2484273003/diff/40001/content/browser/present...
content/browser/presentation/presentation_service_impl.cc:561:
PresentationSessionInfo dummy_session(GURL("fake_url"), "fake_id");
On 2017/02/25 01:43:12, mark a. foltz wrote:
> This is another case where having separate mojoms for the controller and
> receiver would make things clearer (no need to pass a dummy connection).  Can
> you add a TODO to remove this when the refactoring is done?

Removed :)

https://codereview.chromium.org/2484273003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp
(right):

https://codereview.chromium.org/2484273003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp:70:
connection->didChangeState(state);
On 2017/02/25 01:43:12, mark a. foltz wrote:
> Will this fire a terminated event on the receiver connections?  The spec
doesn't
> actually require the browser to do that; since the window is being closed
> anyway, it didn't make sense to run additional event loops, in addition to
> onbeforeunload etc.

Done.

https://codereview.chromium.org/2484273003/diff/40001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/presentation/WebPresentationReceiver.h
(right):

https://codereview.chromium.org/2484273003/diff/40001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/presentation/WebPresentationReceiver.h:31:
virtual void closeWindow() = 0;
On 2017/02/25 01:43:12, mark a. foltz wrote:
> Would prefer terminateConnection() for consistency with WebPresentationClient
> (replacing "session" with "connection" as the spec does now.)

Done.

[mark a. foltz, 2017-03-01 05:13:16]

lgtm

[imcheng, 2017-03-02 01:15:03]

lgtm

https://codereview.chromium.org/2484273003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp
(right):

https://codereview.chromium.org/2484273003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp:72: for
(auto connection : m_connectionList->connections())
Can this be auto* ?

[zhaobin, 2017-03-02 02:54:18]

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

[commit-bot: I haz the power, 2017-03-02 02:55:00]

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-03-02 03:07:58]

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

[commit-bot: I haz the power, 2017-03-02 03:07:59]

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

[zhaobin, 2017-03-02 07:05:23]

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

[commit-bot: I haz the power, 2017-03-02 07:05: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, 2017-03-02 07:14:51]

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

[commit-bot: I haz the power, 2017-03-02 07:14:52]

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...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[zhaobin, 2017-03-02 19:45:05]

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

[commit-bot: I haz the power, 2017-03-02 19:45:31]

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-03-02 21:22:48]

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

[commit-bot: I haz the power, 2017-03-02 21:22:50]

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

[zhaobin, 2017-03-02 21:44:40]

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

[commit-bot: I haz the power, 2017-03-02 21:45:17]

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-03-02 23:40:21]

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

[commit-bot: I haz the power, 2017-03-02 23:40:22]

Dry run: This issue passed the CQ dry run.

[zhaobin, 2017-03-02 23:44:33]

https://codereview.chromium.org/2484273003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp
(right):

https://codereview.chromium.org/2484273003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/presentation/PresentationReceiver.cpp:72: for
(auto connection : m_connectionList->connections())
On 2017/03/02 01:15:03, imcheng wrote:
> Can this be auto* ?

Cannot... m_connectionList->connections() returns
HeapVector<Member<PresentationConnection>>&, Member<PresentationConnection> is
not pointer.

[zhaobin, 2017-03-02 23:44:55]

The CQ bit was checked by zhaobin@chromium.org

[zhaobin, 2017-03-02 23:44:55]

The patchset sent to the CQ was uploaded after l-g-t-m from mfoltz@chromium.org,
imcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2484273003/#ps100001
(title: "fix unit test errors")

[commit-bot: I haz the power, 2017-03-02 23:45:21]

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-03-02 23:53:46]

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

[commit-bot: I haz the power, 2017-03-02 23:53:47]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[zhaobin, 2017-03-03 01:30:15]

The CQ bit was checked by zhaobin@chromium.org

[zhaobin, 2017-03-03 01:30:16]

The patchset sent to the CQ was uploaded after l-g-t-m from mfoltz@chromium.org,
imcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2484273003/#ps120001
(title: "rebase")

[commit-bot: I haz the power, 2017-03-03 01:30:38]

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-03-03 03:20:02]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1488504615414580,
"parent_rev": "738b1539bfb4411bbc7e1ca915c325804f1e39e0", "commit_rev":
"ad129a062423b83cf71bbf1f3e114ea7d8828fab"}

[commit-bot: I haz the power, 2017-03-03 03:20:52]

Description was changed from

==========
[Presentation API] (1-UA) fire PresentationConnection onterminate event if
receiver page gets destroyed

We need to fire 'onterminate' event on controller frame if (Please add scenarios
that are missing):

Controller initiates termination:

1. call connection.terminate() in js
2. click 'stop' on MR dialog
(closing controller frame will close but not terminate connection)

1. and 2. are equivalent in the sense that connection.terminate() just calls
router->terminateRoute(). Cast MRP will notify state change and fire
'onterminate' event in these cases. (Existing implementation covers this)

Receiver initiates termination:

1. receiver frame gets destroyed, e.g. call window.close() in js
2. call connection.terminate() in js

We can make 1. and 2. equivalent, by having connection.terminate() call
window.close(). When offscreen_tab is gone, cast MRP will get notified
(mirroring has been stopped) and notify controller about state change.

BUG=588874
==========

to

==========
[Presentation API] (1-UA) fire PresentationConnection onterminate event if
receiver page gets destroyed

We need to fire 'onterminate' event on controller frame if (Please add scenarios
that are missing):

Controller initiates termination:

1. call connection.terminate() in js
2. click 'stop' on MR dialog
(closing controller frame will close but not terminate connection)

1. and 2. are equivalent in the sense that connection.terminate() just calls
router->terminateRoute(). Cast MRP will notify state change and fire
'onterminate' event in these cases. (Existing implementation covers this)

Receiver initiates termination:

1. receiver frame gets destroyed, e.g. call window.close() in js
2. call connection.terminate() in js

We can make 1. and 2. equivalent, by having connection.terminate() call
window.close(). When offscreen_tab is gone, cast MRP will get notified
(mirroring has been stopped) and notify controller about state change.

BUG=588874

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

[commit-bot: I haz the power, 2017-03-03 03:20:53]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/ad129a062423b83cf71bbf1f3e11...