Pass RemoteFrame mojo channels through its creation messages. [chromium/src : main]
Dave Tapuska (Gerrit)
Attention is currently required from: Daniel Cheng.
1 comment:
Patchset:
Daniel do you mind having a first look as you reviewed the other patch from Igalia last year.
To view, visit change 3723298. To unsubscribe, or for help writing mail filters, visit settings.
Dave Tapuska (Gerrit)
Attention is currently required from: Daniel Cheng.
Dave Tapuska would like Daniel Cheng to review this change.
Pass RemoteFrame mojo channels through its creation messages.
A number of calls to GetAssociatedRemoteFrame needed to be adjusted to
check the liveness of the RenderFrameProxyHost before the call is made.
Alternatives to this were considered, one such was promising if we left
the associated remote bound but disconnected after death. However
with doing that is that some proxies are created but never bound,
their mojo channels aren't sent if the associated RenderProcessHost is
dead, so that alone doesn't fix the issues arising. Previously the
code would just allocate a new AssociatedRemote that was bound but
not sent over the channel so it would just queue.
This CL binds mojo endpoints of RemoteFrame/RemoteFrameHost in
RenderFrameProxyHost and passes corresponding endpoints through its
creation messages.
- content.mojom.Frame.Unload
- content.mojom.AgentSchedulingGroup.CreateFrameProxy
For portal and fenced frames creation messages are sent from the
renderer process. So,
mojo endpoints of RemoteFrame/RemoteFrameHost are initialized in
renderer and corresponding endpoints are bound to RenderFrameProxyHost.
- content.mojom.FrameHost.CreatePortal
- content.mojom.FrameHost.AdoptPortal
Finally, we no longer need AssociatedInterfaceProvider from
RenderFrameProxy.
Patch originally written by Yeunjoo Choi crrev.com/c/2859603 but
was never completed.
Bug: 1190158
Change-Id: Ifd130a6021613f2a09dde75d8625d0465bba76a3
---
M components/guest_view/browser/guest_view_base.cc
M content/browser/fenced_frame/fenced_frame.cc
M content/browser/fenced_frame/fenced_frame.h
M content/browser/fenced_frame/fenced_frame_browsertest.cc
M content/browser/loader/object_navigation_fallback_body_loader.cc
M content/browser/portal/portal.cc
M content/browser/portal/portal.h
M content/browser/portal/portal_browsertest.cc
M content/browser/renderer_host/agent_scheduling_group_host.cc
M content/browser/renderer_host/agent_scheduling_group_host.h
M content/browser/renderer_host/browsing_context_state.cc
M content/browser/renderer_host/cross_process_frame_connector.cc
M content/browser/renderer_host/frame_tree.cc
M content/browser/renderer_host/navigation_request.cc
M content/browser/renderer_host/render_frame_host_impl.cc
M content/browser/renderer_host/render_frame_host_impl.h
M content/browser/renderer_host/render_frame_host_manager.cc
M content/browser/renderer_host/render_frame_host_manager_unittest.cc
M content/browser/renderer_host/render_frame_proxy_host.cc
M content/browser/renderer_host/render_frame_proxy_host.h
M content/browser/renderer_host/render_view_host_impl.cc
M content/browser/security_exploit_browsertest.cc
M content/browser/site_per_process_browsertest.cc
M content/browser/web_contents/web_contents_impl.cc
M content/browser/web_contents/web_contents_impl.h
M content/browser/web_contents/web_contents_impl_browsertest.cc
M content/common/agent_scheduling_group.mojom
M content/common/frame.mojom
M content/public/browser/web_contents.h
M content/public/test/fake_remote_frame.cc
M content/public/test/fake_remote_frame.h
M content/public/test/test_utils.cc
M content/renderer/agent_scheduling_group.cc
M content/renderer/agent_scheduling_group.h
M content/renderer/render_frame_impl.cc
M content/renderer/render_frame_impl.h
M content/renderer/render_frame_impl_browsertest.cc
M content/renderer/render_frame_proxy.cc
M content/renderer/render_frame_proxy.h
M content/renderer/render_view_browsertest.cc
M content/renderer/render_view_impl.cc
M content/test/portal/portal_created_observer.cc
M content/test/portal/portal_created_observer.h
M content/test/test_render_frame.cc
M content/test/test_render_frame_host.cc
M content/test/test_render_view_host.cc
M content/test/test_web_contents.cc
M third_party/blink/public/web/web_remote_frame.h
M third_party/blink/public/web/web_remote_frame_client.h
M third_party/blink/renderer/core/frame/frame_test_helpers.cc
M third_party/blink/renderer/core/frame/frame_test_helpers.h
M third_party/blink/renderer/core/frame/remote_frame.cc
M third_party/blink/renderer/core/frame/remote_frame.h
M third_party/blink/renderer/core/frame/remote_frame_client.h
M third_party/blink/renderer/core/frame/remote_frame_client_impl.cc
M third_party/blink/renderer/core/frame/remote_frame_client_impl.h
M third_party/blink/renderer/core/frame/web_frame_test.cc
M third_party/blink/renderer/core/frame/web_remote_frame_impl.cc
M third_party/blink/renderer/core/frame/web_remote_frame_impl.h
M third_party/blink/renderer/core/paint/compositing/compositing_test.cc
M third_party/blink/renderer/core/testing/fake_remote_frame_host.cc
M third_party/blink/renderer/core/testing/fake_remote_frame_host.h
62 files changed, 768 insertions(+), 458 deletions(-)
Liviu Tinta (Gerrit)
Attention is currently required from: Daniel Cheng, Dave Tapuska.
Liviu Tinta removed null from this change.
Daniel Cheng (Gerrit)
Attention is currently required from: Dave Tapuska.
8 comments:
File content/browser/fenced_frame/fenced_frame.h:
Patch Set #5, Line 50:
RenderFrameProxyHost* CreateProxyAndAttachToOuterFrameTree(Maybe mention `remote_frame_interfaces` must not be null, and DCHECK() this at line 173.
File content/browser/loader/object_navigation_fallback_body_loader.cc:
Patch Set #5, Line 266:
if (proxy->is_render_frame_proxy_live()) {Are all these ifs added because:
- we have no idea when RFPHs may or may not be live, so we just added the checks everywhere or
- All the is_render_frame_proxy() live checks were added in response to some test somewhere failing.
Patch Set #5, Line 273:
->GetAssociatedLocalFrame()I wonder why we don't need a liveness check here? Is this just something we overlooked?
(I'm assuming the corresponding if above was added because of some failing test somewhere)
File content/browser/portal/portal.h:
Patch Set #5, Line 69:
// the proxy.Similarly mention that `remote_frame_interfaces` must not be null and DCHECK()?
File content/browser/renderer_host/browsing_context_state.cc:
Patch Set #5, Line 409:
proxy->GetAssociatedRemoteFrame()->SetFrameOwnerProperties(Were these also all bugs if we encountered a non-live RFPH?
File content/browser/renderer_host/render_frame_host_impl.cc:
Patch Set #5, Line 7453:
if (proxy_host->is_render_frame_proxy_live()) {Can CreateProxyAndAttachPortal() ever return a non-live RFPH? If so, why would it do so? Can we change that?
File third_party/blink/renderer/core/frame/frame.h:
Patch Set #5, Line 412:
bool Swap(WebRemoteFrame*,Somewhere, it would be good to explain why WebRemoteFrame needs these additional bits, but WebLocalFrame does not.
I would have expected the plumbing to largely be symmetrical, so it's a bit surprising from this aspect.
File third_party/blink/renderer/core/frame/frame_test_helpers.h:
Patch Set #5, Line 159:
// if necessary.Do we need something like this for local frames eventually?
To view, visit change 3723298. To unsubscribe, or for help writing mail filters, visit settings.
Dave Tapuska (Gerrit)
Attention is currently required from: Daniel Cheng.
8 comments:
File content/browser/fenced_frame/fenced_frame.h:
Patch Set #5, Line 50:
RenderFrameProxyHost* CreateProxyAndAttachToOuterFrameTree(Maybe mention `remote_frame_interfaces` must not be null, and DCHECK() this at line 173.
Done
File content/browser/loader/object_navigation_fallback_body_loader.cc:
Patch Set #5, Line 266:
if (proxy->is_render_frame_proxy_live()) {
Are all these ifs added because: […]
So I added them where I thought they were necessary. proxy_hosts_ in RenderFrameHostManager can have not-live RFPHs in it. So anything returned from RenderFrameHostManager now has a is_render_frame_proxy_live chceck before acessing GetAssociatedRemoteFrame.
See the commit msg. I attempted to not unbind the channel in TearDown, but because RFHM can actually have RFPH that haven't ever been sent to a renderer that approach didn't work.
Patch Set #5, Line 273:
->GetAssociatedLocalFrame()
I wonder why we don't need a liveness check here? Is this just something we overlooked? […]
The way GetAssociatedLocalFrame for RenderFrameHostImpl works is that it rebinds a new remote if it isn't bound. See: https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/render_frame_host_impl.cc;l=9374;drc=f1a93e5d28bef5c70d5552758d7aa6b85bc54418;bpv=1;bpt=1?q=GetAssociatedLocalFrame&ss=chromium%2Fchromium%2Fsrc
So this new remote will be bound but never passed across a channel (because of https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/render_frame_host_impl.cc;l=2683;drc=f1a93e5d28bef5c70d5552758d7aa6b85bc54418;bpv=1;bpt=1?q=GetAssociatedLocalFrame&ss=chromium%2Fchromium%2Fsrc)
This approach for RenderFrameProxyHost is more explicit. You can't call GetAssociatedRemoteFrame if it isn't live.
File content/browser/portal/portal.h:
Patch Set #5, Line 69:
// the proxy.Similarly mention that `remote_frame_interfaces` must not be null and DCHECK()?
Done
File content/browser/renderer_host/browsing_context_state.cc:
Patch Set #5, Line 409:
proxy->GetAssociatedRemoteFrame()->SetFrameOwnerProperties(Were these also all bugs if we encountered a non-live RFPH?
Yes. proxy_hosts_ stores non live ones. So instead of rewriting it here on everyone I reused the *already* correct ExecuteRemoteFramesBroadcastMethod.
File content/browser/renderer_host/render_frame_host_impl.cc:
Patch Set #5, Line 7453:
if (proxy_host->is_render_frame_proxy_live()) {
Can CreateProxyAndAttachPortal() ever return a non-live RFPH? If so, why would it do so? Can we chan […]
Oh yes they are. But proxy_host can be null, so fixed that.
File third_party/blink/renderer/core/frame/frame.h:
Patch Set #5, Line 412:
bool Swap(WebRemoteFrame*,
Somewhere, it would be good to explain why WebRemoteFrame needs these additional bits, but WebLocalF […]
I imagine they will look like that in the future. But for WebLocalFrame local frames being passed in. But I think getting away from the AssociatedInterfaceProvider for remote frames is good. Comments added.
File third_party/blink/renderer/core/frame/frame_test_helpers.h:
Patch Set #5, Line 159:
// if necessary.Do we need something like this for local frames eventually?
Eventually yes. I do want to pass the mojo remote/receiver from the renderer side eventually. Then we can remove the Sync IPC call to the IO thread for CreateFrame. But to do that we still need even more legacy IPC gone. (fortunately the only legacy IPC used for RemoteFrames is the associated interface fetching).
To view, visit change 3723298. To unsubscribe, or for help writing mail filters, visit settings.
Dave Tapuska (Gerrit)
Attention is currently required from: Daniel Cheng.
1 comment:
Patchset:
dcheng@ if you could look at this it would be appreciated. I'm trying to get to the place where the fenced frame creation is async. Removing routing ID on RenderFrameProxy gets us almost there.
To view, visit change 3723298. To unsubscribe, or for help writing mail filters, visit settings.
Dave Tapuska (Gerrit)
Attention is currently required from: Arthur Sonzogni, Daniel Cheng.
Dave Tapuska would like Arthur Sonzogni to review this change.
M third_party/blink/public/web/web_frame.h
M third_party/blink/public/web/web_remote_frame.h
M third_party/blink/public/web/web_remote_frame_client.h
M third_party/blink/renderer/controller/performance_manager/renderer_resource_coordinator_impl_test.cc
M third_party/blink/renderer/core/frame/frame.cc
M third_party/blink/renderer/core/frame/frame.h
M third_party/blink/renderer/core/frame/frame_test_helpers.cc
M third_party/blink/renderer/core/frame/frame_test_helpers.h
M third_party/blink/renderer/core/frame/remote_frame.cc
M third_party/blink/renderer/core/frame/remote_frame.h
M third_party/blink/renderer/core/frame/remote_frame_client.h
M third_party/blink/renderer/core/frame/remote_frame_client_impl.cc
M third_party/blink/renderer/core/frame/remote_frame_client_impl.h
M third_party/blink/renderer/core/frame/web_frame.cc
M third_party/blink/renderer/core/frame/web_frame_test.cc
M third_party/blink/renderer/core/frame/web_frame_widget_test.cc
M third_party/blink/renderer/core/frame/web_remote_frame_impl.cc
M third_party/blink/renderer/core/frame/web_remote_frame_impl.h
M third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc
M third_party/blink/renderer/core/page/spatial_navigation_test.cc
M third_party/blink/renderer/core/paint/compositing/compositing_test.cc
M third_party/blink/renderer/core/testing/fake_remote_frame_host.cc
M third_party/blink/renderer/core/testing/fake_remote_frame_host.h
70 files changed, 977 insertions(+), 670 deletions(-)
Arthur Sonzogni (Gerrit)
Attention is currently required from: Daniel Cheng, Dave Tapuska.
1 comment:
Patchset:
I am overwhelmed by code reviews today. I got >15
https://chromium-review.googlesource.com/q/reviewer:arthursonzogni%2540chromium.org+-age:1dI will do yours tomorrow. It is the largest one and it seems important to do it right.
To view, visit change 3723298. To unsubscribe, or for help writing mail filters, visit settings.
Arthur Sonzogni (Gerrit)
Attention is currently required from: Daniel Cheng, Dave Tapuska.
Dave Tapuska has uploaded this change for review.
To view, visit change 3723298. To unsubscribe, or for help writing mail filters, visit settings.
Arthur Sonzogni (Gerrit)
Attention is currently required from: Daniel Cheng, Dave Tapuska.
8 comments:
Commit Message:
Previously the
code would just allocate a new AssociatedRemote that was bound but
not sent over the channel so it would just queue.
If we were enqueing message but never delivering them, then I agree checking if the RFPH is alive before every call should be equivalent.
To double check, could you show me what is the part of the code where, we create a new AssociatedRemote, but never sent it over the channel?
Note for myself: The message continue to be properly ordered with frame/navigation IPC, because they are still associated interface and they are sent via mojom::FrameHost / mojom::AgenSchedulingGroup.
Patchset:
Thanks!
So that's similar to:
https://chromium-review.googlesource.com/c/chromium/src/+/2853696
but for RemoteFrame instead of RemoteMainFrame.I did a first pass. See some minor comment below.
+CC alexmos@ who might be interested. FYI.
File content/browser/renderer_host/render_frame_host_impl.cc:
proxy->GetAssociatedRemoteFrame()->SetNeedsOcclusionTracking(
needs_tracking);nit: Can you wrap it with {}, because it is now a multiline instruction.
File content/public/test/fake_remote_frame.h:
Patch Set #10, Line 30:
AssociatedInterfaceProviderThis comment needs to be updated.
File content/renderer/render_frame_impl.cc:
// TODO(dcheng): Improve this comment to clarify why it's important to sent
// state updates.It was not obvious to me why removing this TODO?
File content/renderer/render_view_browsertest.cc:
nit (×7): Could you specify the name of the argument whose value is true? `/*arg=*/true`
File third_party/blink/renderer/core/frame/frame_test_helpers.h:
nit: it belongs to the previous line.
To view, visit change 3723298. To unsubscribe, or for help writing mail filters, visit settings.
Dave Tapuska (Gerrit)
Attention is currently required from: Arthur Sonzogni, Daniel Cheng.
6 comments:
Commit Message:
Previously the
code would just allocate a new AssociatedRemote that was bound but
not sent over the channel so it would just queue.
If we were enqueing message but never delivering them, then I agree checking if the RFPH is alive be […]
The AssociatedInterfaceProvider was not recreated whenever the ChannelProxy is changed, so if the process died and was recreated its channel to get associated interfaces was bound but closed. So calls like https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/render_frame_proxy_host.cc;l=338;drc=843911a994428b40537550f48b52d0d58cf08dd3;bpv=1;bpt=1 wouldn't get sent across an IPC channel.
Also we do early outs if RFPH's process is dead: see https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/render_frame_proxy_host.cc;l=229;drc=843911a994428b40537550f48b52d0d58cf08dd3;bpv=1;bpt=1
File content/browser/renderer_host/render_frame_host_impl.cc:
proxy->GetAssociatedRemoteFrame()->SetNeedsOcclusionTracking(
needs_tracking);nit: Can you wrap it with {}, because it is now a multiline instruction.
Done
File content/public/test/fake_remote_frame.h:
Patch Set #10, Line 30:
AssociatedInterfaceProviderThis comment needs to be updated.
Done
File content/renderer/render_frame_impl.cc:
// TODO(dcheng): Improve this comment to clarify why it's important to sent
// state updates.It was not obvious to me why removing this TODO?
Added back. I agree, must have been a conflict resolution issue I had.
File content/renderer/render_view_browsertest.cc:
nit (×7): Could you specify the name of the argument whose value is true? `/*arg=*/true`
Done (x9) :-)
File third_party/blink/renderer/core/frame/frame_test_helpers.h:
nit: it belongs to the previous line.
Done
To view, visit change 3723298. To unsubscribe, or for help writing mail filters, visit settings.
Arthur Sonzogni (Gerrit)
Attention is currently required from: Daniel Cheng, Dave Tapuska.
Patch set 11:Code-Review +1
7 comments:
Commit Message:
Previously the
code would just allocate a new AssociatedRemote that was bound but
not sent over the channel so it would just queue.
So take a look at: https://source.chromium. […]
Thanks!
Patchset:
Thanks! This should be good.
content/ LGTM assuming all my comment are addressed below.I am starting vacation tomorrow. So if you proceed with significant change, could you request a review from @alexmos?
I am a bit sad about all those "if rfph_alive" everywhere before calling an IPC. I agree this is equivalent to the previous behavior. It is possible some not to be strictly necessary, but at least being consistent will prevent developers from missing it.
File content/browser/renderer_host/render_frame_host_manager.cc:
if (frame_tree_node_->IsFencedFrameRoot() &&
frame_tree_node_->frame_tree()->type() == FrameTree::Type::kFencedFrame &&
GetProxyToOuterDelegate()->is_render_frame_proxy_live()) {
GetProxyToOuterDelegate()->GetAssociatedRemoteFrame()->Collapse(collapsed);
return;
}This doesn't look equivalent to the previous behavior. Shouldn't this return independently of whether the proxy is live?
Shouldn't this be:
```
if (frame_tree_node_->IsFencedFrameRoot() &&
frame_tree_node_->frame_tree()->type() == FrameTree::Type::kFencedFrame) {
if (GetProxyToOuterDelegate()->is_render_frame_proxy_live()) {
GetProxyToOuterDelegate()->GetAssociatedRemoteFrame()->Collapse(collapsed);
}
return;
}
```
?
File content/common/frame.mojom:
Patch Set #11, Line 665:
proxy_routing_idThis needs to be updated
Patch Set #11, Line 683:
proxy_routing_idThis needs to be updated.
Patch Set #11, Line 701:
proxy_routing_idThis needs to be updated.
File content/renderer/render_frame_impl.cc:
Patch Set #11, Line 4094:
routing id
This comment needs to be updated.
To view, visit change 3723298. To unsubscribe, or for help writing mail filters, visit settings.
Dave Tapuska (Gerrit)
Attention is currently required from: Daniel Cheng.
5 comments:
File content/browser/renderer_host/render_frame_host_manager.cc:
if (frame_tree_node_->IsFencedFrameRoot() &&
frame_tree_node_->frame_tree()->type() == FrameTree::Type::kFencedFrame &&
GetProxyToOuterDelegate()->is_render_frame_proxy_live()) {
GetProxyToOuterDelegate()->GetAssociatedRemoteFrame()->Collapse(collapsed);
return;
}
This doesn't look equivalent to the previous behavior. […]
Done
File content/common/frame.mojom:
Patch Set #11, Line 665:
proxy_routing_idThis needs to be updated
Done
Patch Set #11, Line 683:
proxy_routing_idThis needs to be updated.
Done
Patch Set #11, Line 701:
proxy_routing_idThis needs to be updated.
Done
File content/renderer/render_frame_impl.cc:
Patch Set #11, Line 4094:
routing idThis comment needs to be updated.
Done
To view, visit change 3723298. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Cheng (Gerrit)
Attention is currently required from: Dave Tapuska.
Patch set 12:Code-Review +1
5 comments:
Patchset:
LGTM w/comments addressed
File components/guest_view/browser/guest_view_base.cc:
Patch Set #12, Line 528:
/*remote_frame_host_receiver=*/absl::nullopt, is_full_page_plugin);Nit: perhaps it'd be helpful to mention in a comment where these will be set later instead?
File content/browser/loader/object_navigation_fallback_body_loader.cc:
Patch Set #5, Line 266:
if (proxy->is_render_frame_proxy_live()) {
So I added them where I thought they were necessary. […]
Thanks, that makes sense.
Patch Set #5, Line 273:
->GetAssociatedLocalFrame()
The way GetAssociatedLocalFrame for RenderFrameHostImpl works is that it rebinds a new remote if it […]
I think the more explicit approach is strictly better. Thanks for the background.
File content/browser/web_contents/web_contents_impl.cc:
Patch Set #12, Line 2421:
absl::optional<mojo::PendingAssociatedRemote<blink::mojom::RemoteFrame>>Nit: it's a bit weird to have optional wrapping the Mojo endpoints, since the mojo endpoints can already be 'null' (e.g. a default-constructed PendingAssociatedRemote or default-constructed PendingAssociatedReceiver).
Let's drop the absl::optional here and use is_valid() on lines 2497.
mojo::NullAssociated{Receiver,Remote}() is more convenient shorthand for constructing null endpoints than writing out the full name of the pending types, and can be used to replace absl::nullopt.
To view, visit change 3723298. To unsubscribe, or for help writing mail filters, visit settings.
Dave Tapuska (Gerrit)
2 comments:
File components/guest_view/browser/guest_view_base.cc:
Patch Set #12, Line 528:
/*remote_frame_host_receiver=*/absl::nullopt, is_full_page_plugin);Nit: perhaps it'd be helpful to mention in a comment where these will be set later instead?
Done
File content/browser/web_contents/web_contents_impl.cc:
Patch Set #12, Line 2421:
absl::optional<mojo::PendingAssociatedRemote<blink::mojom::RemoteFrame>>
Nit: it's a bit weird to have optional wrapping the Mojo endpoints, since the mojo endpoints can alr […]
Done
To view, visit change 3723298. To unsubscribe, or for help writing mail filters, visit settings.
Dave Tapuska (Gerrit)
Attention is currently required from: Kevin McNee.
Dave Tapuska would like Kevin McNee to review this change.
Pass RemoteFrame mojo channels through its creation messages.
A number of calls to GetAssociatedRemoteFrame needed to be adjusted to
check the liveness of the RenderFrameProxyHost before the call is made.
Alternatives to this were considered, one such was promising if we left
the associated remote bound but disconnected after death. However
with doing that is that some proxies are created but never bound,
their mojo channels aren't sent if the associated RenderProcessHost is
dead, so that alone doesn't fix the issues arising. Previously the
code would just allocate a new AssociatedRemote that was bound but
not sent over the channel so it would just queue.
70 files changed, 987 insertions(+), 673 deletions(-)
Dave Tapuska (Gerrit)
Attention is currently required from: Kevin McNee.
1 comment:
Patchset:
+ Kevin for guest view
To view, visit change 3723298. To unsubscribe, or for help writing mail filters, visit settings.
Kevin McNee (Gerrit)
Attention is currently required from: Dave Tapuska.
Patch set 13:Code-Review +1
Dave Tapuska (Gerrit)
Attention is currently required from: Dave Tapuska.
Patch set 13:Commit-Queue +2
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3723298
Reviewed-by: Daniel Cheng <dch...@chromium.org>
Commit-Queue: Dave Tapuska <dtap...@chromium.org>
Reviewed-by: Kevin McNee <mc...@chromium.org>
Reviewed-by: Arthur Sonzogni <arthurs...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1024955}
70 files changed, 993 insertions(+), 673 deletions(-)