This implements the following behaviour (behind kGuestViewMPArch):
- attaches guests in FrameAutoAttacher
- updates in RFDAH to correctly expose guest target information
- sets the placeholder RemoteFrame's devtools_frame_token with the
guest's main frame's devtools_frame_token after the guest is
attached
Still TODO:
- support inspecting new guests added & waitForDebugger
See bug for design doc.
Bug: 376084061
Change-Id: I326fe0dfe0a6e3a33a3dd2806b69efc239c23af1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6050701
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Adithya Srinivasan <adithyas@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Joe Mason <joenotcharles@google.com>
Cr-Commit-Position: refs/heads/main@{#1395977}
Previously we would try to differentiate pointer interactions which
happened to have pointermove events, and assumed that these were "drag"
vs tap or click. However, this never quite worked, as it is very common
to have a tiny bit of pointermove even for a tap (threshold distance).
Also, even the value of this feature is minimal, as "drag" itself can
vary a lot and we only sometimes measure some parts of drag interactions.
We are just going to remove this attempt to differentiate.
Bug: 40930016, 382949422
Change-Id: I009f83779f1d04f31f9216bd52c9d4089a2d66ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6059123
Reviewed-by: Mikhail Khokhlov <khokhlov@google.com>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Noam Rosenthal <nrosenthal@chromium.org>
Reviewed-by: Joe Mason <joenotcharles@google.com>
Commit-Queue: Michal Mocny <mmocny@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1395225}
Move some more blink media code into platform/media from public/platform/media. This allows us to use KURL directly and
not go to a WebURL on some calls.
Apply INSIDE_BLINK macro to other code that is in platform/media so
it will use the internal types. This is necessary because this code uses
blink mojom types and we want to enforce that blink mojo headers are
only included inside blink. Fortunately this addresses some of the
discouraged type overrides in the media code.
BYPASS_LARGE_CHANGE_WARNING=File move
Change-Id: I712128d82bb7e1971548f22aaadfdf31a7393ad6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5962517
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1374973}
On LocalFrame <-> LocalFrame swaps for local root frames, we will
destruct the previous compositor before committing the new document.
Actions like releasing the LayerTreeFrameSink might block the main
thread and regress LCP. To prevent that from happening, this CL
introduces a way to keep the previous document's LayerTreeView
alive by moving its ownership to a task posted to delete
it after some delay (defaulting to 1000ms), instead of immediately
deleting it on widget destruction.
Bug: 936696
Change-Id: I2a5683ba80c959205e66a8ed6d6820f5af77e842
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5938512
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1373659}
This is a reland of commit 33493d2d85
This restores a branch in RenderViewHostImpl::CreateRenderView() that
was mistakenly removed in the original CL.
Original change's description:
> [prerender] Fix crash when discarding a speculative main RenderFrameHost
>
> Previously, starting a prerender navigation would create a speculative
> RenderFrameHost for the root frame tree node in the browser process.
> However, the corresponding RenderFrame in the renderer process would
> be created as the active/current main RenderFrame. This is a result of
> the interaction between `CreateProxiesForSiteInstanceGroup()`, which
> skips creating a proxy if it thinks the early commit optimization will
> be used, and `PerformEarlyRenderFrameHostSwapIfNeeded()`, which skips
> performing early commits in prerendering frame trees.
>
> Intentional or not, this is problematic because:
>
> - The browser process assumes speculative RenderFrameHosts can be
> cleaned up by deleting the corresponding RenderFrame in the renderer
> process.
> - The renderer process assumes the active/current main RenderFrame is
> never explicitly deleted, and that it will only be implicitly deleted
> as a side effect of explicitly deleting its blink::WebView.
> - Blink assumes a blink::WebView always has an active main frame (i.e.
> non-null). Rather than crashing non-deterministically, the renderer
> process CHECK-fails if the browser tries to explicitly delete a
> non-provisional main RenderFrame.
>
> Taken together, this means discarding a speculative main RenderFrameHost
> during a prerender navigation will crash the renderer process. Needless
> to say, this is not desirable. Previous fixes have centered around
> identifying the cases that would discard a speculative RenderFrameHost
> and disabling prerendering for those cases.
>
> However, this doesn't fix the underlying issue and ends up being a game
> of whack-a-mole. This CL implements a proper fix: the corresponding
> main RenderFrame in the renderer process is now created as a provisional
> frame during a prerender navigation. This is implemented using a
> placeholder RemoteFrame; RenderDocument already uses this concept for
> local -> local frame swaps.
>
> This CL also refactors several ancillary components to make the updated
> logic easier to follow:
>
> - Rather than using a non-null `previous_frame_token` as the implicit
> signal for creating a provisional main local frame, Mojo now uses the
> CreateProvisionalLocalMainFrameParams variant subtype to convey this
> intent. This is a required cleanup, since prerender navigations should
> not set a `previous_frame_token`, but still want the placeholder
> RemoteFrame + provisional LocalFrame behavior.
> - With the updated union, the different main frame cases are also broken
> out more distinctly in `AgentSchedulingGroup::CreateView()`.
> - Rename `EnsureWidgetInitialized()` to `InitializeWidgetAtSwap()`,
> since that is the only situation it is called.
> - Remove `previous_frame_token_for_compositor_reuse` from the frame
> widget creation params, and replace it with the `reuse_compositor`
> bool. The frame token is redundant, since the frame widget creation
> params are always paired with frame creation params.
> - Refactor `WillDetach()` and `InitializeWidgetAtSwap()` to account for
> the removal of `previous_frame_token_for_compositor_reuse`. It turns
> out the renderer process already has all the information needed to
> determine the previous frame; passing it directly significantly
> simplifies this coordination.
>
> Finally, this also removes the crash instrumentation that was previously
> added to help track down situations where prerender navigations would
> discard speculative main RenderFrameHosts. Since this no longer crashes,
> the instrumentation is no longer needed.
>
> Bug: 40076091
> Change-Id: I1c19853c75580492653e608afb1db3442a264b3a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5922799
> Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
> Commit-Queue: Daniel Cheng <dcheng@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1369647}
Bug: 40076091
Change-Id: I10ecdf8233f6dfd42e0478c819447831d50256c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5937586
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1369798}
This reverts commit 33493d2d85.
Reason for revert: CL has bug :)
Original change's description:
> [prerender] Fix crash when discarding a speculative main RenderFrameHost
>
> Previously, starting a prerender navigation would create a speculative
> RenderFrameHost for the root frame tree node in the browser process.
> However, the corresponding RenderFrame in the renderer process would
> be created as the active/current main RenderFrame. This is a result of
> the interaction between `CreateProxiesForSiteInstanceGroup()`, which
> skips creating a proxy if it thinks the early commit optimization will
> be used, and `PerformEarlyRenderFrameHostSwapIfNeeded()`, which skips
> performing early commits in prerendering frame trees.
>
> Intentional or not, this is problematic because:
>
> - The browser process assumes speculative RenderFrameHosts can be
> cleaned up by deleting the corresponding RenderFrame in the renderer
> process.
> - The renderer process assumes the active/current main RenderFrame is
> never explicitly deleted, and that it will only be implicitly deleted
> as a side effect of explicitly deleting its blink::WebView.
> - Blink assumes a blink::WebView always has an active main frame (i.e.
> non-null). Rather than crashing non-deterministically, the renderer
> process CHECK-fails if the browser tries to explicitly delete a
> non-provisional main RenderFrame.
>
> Taken together, this means discarding a speculative main RenderFrameHost
> during a prerender navigation will crash the renderer process. Needless
> to say, this is not desirable. Previous fixes have centered around
> identifying the cases that would discard a speculative RenderFrameHost
> and disabling prerendering for those cases.
>
> However, this doesn't fix the underlying issue and ends up being a game
> of whack-a-mole. This CL implements a proper fix: the corresponding
> main RenderFrame in the renderer process is now created as a provisional
> frame during a prerender navigation. This is implemented using a
> placeholder RemoteFrame; RenderDocument already uses this concept for
> local -> local frame swaps.
>
> This CL also refactors several ancilliary components to make the updated
> logic easier to follow:
>
> - Rather than using a non-null `previous_frame_token` as the implicit
> signal for creating a provisional main local frame, Mojo now uses the
> CreateProvisionalLocalMainFrameParams variant subtype to convey this
> intent. This is a required cleanup, since prerender navigations should
> not set a `previous_frame_token`, but still want the placeholder
> RemoteFrame + provisional LocalFrame behavior.
> - With the updated union, the different main frame cases are also broken
> out more distinctly in `AgentSchedulingGroup::CreateView()`.
> - Rename `EnsureWidgetInitialized()` to `InitializeWidgetAtSwap()`,
> since that is the only situation it is called.
> - Remove `previous_frame_token_for_compositor_reuse` from the frame
> widget creation params, and replace it with the `reuse_compositor`
> bool. The frame token is redundant, since the frame widget creation
> params are always paired with frame creation params.
> - Refactor `WillDetach()` and `InitializeWidgetAtSwap()` to account for
> the removal of `previous_frame_token_for_compositor_reuse`. It turns
> out the renderer process already has all the information needed to
> determine the previous frame; passing it directly significantly
> simplifies this coordination.
>
> Finally, this also removes the crash instrumentation that was previously
> added to help track down situations where prerender navigations would
> discard speculative main RenderFrameHosts. Since this no longer crashes,
> the instrumentation is no longer needed.
>
> Bug: 40076091
> Change-Id: I1c19853c75580492653e608afb1db3442a264b3a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5922799
> Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
> Commit-Queue: Daniel Cheng <dcheng@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1369647}
Bug: 40076091
Change-Id: Ic6caff54e1d3085603809fdefb036b277c4720d5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5938653
Auto-Submit: Daniel Cheng <dcheng@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1369651}
Previously, starting a prerender navigation would create a speculative
RenderFrameHost for the root frame tree node in the browser process.
However, the corresponding RenderFrame in the renderer process would
be created as the active/current main RenderFrame. This is a result of
the interaction between `CreateProxiesForSiteInstanceGroup()`, which
skips creating a proxy if it thinks the early commit optimization will
be used, and `PerformEarlyRenderFrameHostSwapIfNeeded()`, which skips
performing early commits in prerendering frame trees.
Intentional or not, this is problematic because:
- The browser process assumes speculative RenderFrameHosts can be
cleaned up by deleting the corresponding RenderFrame in the renderer
process.
- The renderer process assumes the active/current main RenderFrame is
never explicitly deleted, and that it will only be implicitly deleted
as a side effect of explicitly deleting its blink::WebView.
- Blink assumes a blink::WebView always has an active main frame (i.e.
non-null). Rather than crashing non-deterministically, the renderer
process CHECK-fails if the browser tries to explicitly delete a
non-provisional main RenderFrame.
Taken together, this means discarding a speculative main RenderFrameHost
during a prerender navigation will crash the renderer process. Needless
to say, this is not desirable. Previous fixes have centered around
identifying the cases that would discard a speculative RenderFrameHost
and disabling prerendering for those cases.
However, this doesn't fix the underlying issue and ends up being a game
of whack-a-mole. This CL implements a proper fix: the corresponding
main RenderFrame in the renderer process is now created as a provisional
frame during a prerender navigation. This is implemented using a
placeholder RemoteFrame; RenderDocument already uses this concept for
local -> local frame swaps.
This CL also refactors several ancilliary components to make the updated
logic easier to follow:
- Rather than using a non-null `previous_frame_token` as the implicit
signal for creating a provisional main local frame, Mojo now uses the
CreateProvisionalLocalMainFrameParams variant subtype to convey this
intent. This is a required cleanup, since prerender navigations should
not set a `previous_frame_token`, but still want the placeholder
RemoteFrame + provisional LocalFrame behavior.
- With the updated union, the different main frame cases are also broken
out more distinctly in `AgentSchedulingGroup::CreateView()`.
- Rename `EnsureWidgetInitialized()` to `InitializeWidgetAtSwap()`,
since that is the only situation it is called.
- Remove `previous_frame_token_for_compositor_reuse` from the frame
widget creation params, and replace it with the `reuse_compositor`
bool. The frame token is redundant, since the frame widget creation
params are always paired with frame creation params.
- Refactor `WillDetach()` and `InitializeWidgetAtSwap()` to account for
the removal of `previous_frame_token_for_compositor_reuse`. It turns
out the renderer process already has all the information needed to
determine the previous frame; passing it directly significantly
simplifies this coordination.
Finally, this also removes the crash instrumentation that was previously
added to help track down situations where prerender navigations would
discard speculative main RenderFrameHosts. Since this no longer crashes,
the instrumentation is no longer needed.
Bug: 40076091
Change-Id: I1c19853c75580492653e608afb1db3442a264b3a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5922799
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1369647}
This reduces some flaky tests when a race condition happens and a full serialization occurs that needed info from the location updates.
If we're sending a full serialization anyways it actually makes sense to use one serialization than two seperate ones anyways.. Less overhead.
Change-Id: Ie64acd34e6da92344b423dc013ba40497e5d17d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5878322
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Ahmed Elwasefi (Ahmad45123) <a.m.elwasefi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1362560}
SerializeFrame now returns serialized resources in a callback.
A subsequent change will take advantage of this and fetch some
resources asynchronously.
This change should be a no-op.
Bug: 363289333
Change-Id: If00c8b47add50e384921c29e2a88a167b5b4a058
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5899715
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Dan H <harringtond@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1362478}
Duplicate navigations are navigations that are the same as the
current ongoing navigations, and likely to be triggered accidentally
by e.g. double link/button clicks. This CL makes it so that we
record duplicate navs as a separate NavigationDiscardReason.
Also, currently when a new renderer-initiated navigation is
triggered while there is already another navigation initiated by
the same renderer ongoing, the older navigation is canceled by
overwriting the NavigationClient. This cancellation is incorrectly
marked as kInternalCancellation because the overwrite didn't
set a reason. This CL fixes that by setting the reason to
indicate that the navigation client is disconnected because of
a new navigation / duplicate navigation. This CL also makes
form submission-initiated cancellation mark the disconnection
correctly (as "new navigation").
Bug: 366060351
Change-Id: I084ca4dbd56805192cf4d553ab866e011d7d1917
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5856745
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1355642}
If 2 link clicks to the same URL happens on the same time, it's likely
that it is caused by accident. We probably should keep the first
navigation since they will most likely result in the same page.
This CL adds the logic to do that, protected behind a flag.
Bug: 366060351
Change-Id: I344fcfbf0831b0fa16fcb19b8695f2986a1cf612
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5837959
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1354370}
Specifically if we're going to use a resource from the cache
then a lot of the work during prepare isn't necessary. To enable
this I've moved some of the work earlier, specifically into
a new function PopulateResourceRequestBeforeCacheAccess. I've also
introduced a new client interface specifically for modifying the url
(rather than through WillSendRequest). In FetchContext I renamed
PopulateResourceRequest to UpgradeResourceRequestForLoader. In
resource_requestion_utils I renamed PrepareResourceRequest to
UpgradeResourceRequestForLoader.
This change is quite risky, so I added a kill switch. Unfortunately
the kill switch makes the code pretty ugly at the moment. This is
part of the reason I haven't looked into combining
PopulateResourceRequest and PrepareResourceRequest. I will tackle
that assuming this change sticks.
There are a couple of things not gated by the kill switch:
. new function is always called on client
. transparent placeholder image logic in ResourceFetcher.
These changes are pretty straightforward though, and it would
have made the code even more awkward to gate it by the kill switch.
Bug: 359910398
Change-Id: Ic25b302b9b3372084ff23496e623d0b6a21ed908
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5794358
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1347392}
With PDF enforcements from r1319795, PDF renderer processes are not
allowed to access localStorage and sessionStorage; the browser process
expects that the renderers would never use these interfaces to begin
with and terminates the renderers if it does see such an access (in
DOMStorageContextWrapper::IsRequestValid()).
However, it turns out that these interfaces may still be accessed via
DevTools when switching context to the PDF document. The goal of this
CL is to make this fail more gracefully than with a renderer kill, by
denying access to localStorage/sessionStorage in Blink, similarly to
how storage access is denied for other cases like opaque origins or
the WebPreferences::local_storage_enabled setting. This is done by
adding plumbing to ask ChromeContentRendererClient, which knows
whether the current renderer is for PDF. See
https://crbug.com/357014503 for a summary of alternatives
considered. Longer-term, we should explore whether PDF documents
should just get an opaque origin, which would be a more comprehensive
fix if it doesn't break compat.
Bug: 357014503
Change-Id: I90995daceccd4b448523d44bbfc5229d4f27348a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5768152
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1338897}
Given a media playing iframe, this CL implements the plumbing required
to send the frame's FrameVisibility to the WebMediaPlayer object
responsible for the frame's media playback. This CL also cover the
scenario where a local frame needs to send a frame visibility update to
a remote frame.
During LocalFrameView::RunIntersectionObserverSteps all the MainFrame's
children will have their FrameVisibility status checked. Then, this data
should be sent to the frames RenderFrameImpl object, which implements
the blink::WebLocalFrameClient interface. Then, the frame's
RendererWebMediaPlayerDelegate, which implements the RenderFrameObserver
interface will also be informed of the visibility change. Finally, the
frame WebMediaPlayerImpl and WebMediaPlayerMS objects will be informed
of the frame visibility changes.
There are 2 different routes to send the visibility change data to a
frame's RenderFrameImpl object from its FrameView object:
- Local Frames
The visibility information is kept in the renderer process.
LocalFrameView::VisibilityChanged will pass this information to the
frame's RenderFrameImpl by calling
RenderFrameImpl::OnFrameVisibilityChanged.
- Remote Frames
The visibility change information needs to be sent first to the browser
process, so that it can then redirected to the renderer process which
contains the RemoteFrame's LocalFrame equivalent. The info flow is:
Renderer process:
- RemoteFrameView::VisibilityChanged
Browser process:
- RenderFrameProxyHost::VisibilityChanged
- CrossProcessFrameConnector::OnVisibilityChanged
- RenderFrameHostImpl::VisibilityChanged
Renderer process
- LocalFrameMojoHandler::OnFrameVisibilityChanged
- RenderFrameImpl::OnFrameVisibilityChanged
Then the process continues as the Local Frame case.
This feature is under active development. Therefore, its behavior is still under discussion and may change in the future.
Bug: 351354996
Change-Id: Ie1247ff31564b599a25327ec217cd6e3df43228d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5689283
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Gabriel Brito <gabrielbrito@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1335033}
This CL moves
`third_party/blink/public/common/browser_interface_broker_proxy.h`
to
`third_party/blink/public/platform/browser_interface_broker_proxy.h`
Motivation:
* This header is not included from any browser-process code and
therefore doesn't need to be in `.../common/...`.
* This resolves layering ickiness introduced by the previous CL (see
https://crrev.com/c/5647559) which moved implementation of this type
deeper into Blink (e.g. using `CrossVariantMojoRemote` and similar
utilities which are normally available to the `platform` code but not
to the `common` code).
This CL has been generated semi-automatically - most of the files
changed by this CL have just been updated to use the new `#include` path
(as generated by `tools/git/mass-rename.sh`). Manual changes are
concentrated in the following files:
* Moving
`third_party/blink/public/common/browser_interface_broker_proxy.h`
to
`third_party/blink/public/platform/browser_interface_broker_proxy.h`
- This also removed a `nogncheck` ickiness left by the previous CL
- This had to switch from `BLINK_COMMON_EXPORT` to
`BLINK_PLATFORM_EXPORT`
* Removing layering ickiness in `third_party/blink/public/common/DEPS`
* `DEPS` update and/or simplification in:
- `chromecast/media/DEPS`
- `components/page_image_annotation/content/renderer/DEPS`
- `components/translate/content/renderer/DEPS`
- `media/mojo/clients/DEPS`
* Changing a dependency in `media/fuchsia/video/BUILD.gn` and
`chromecast/media/audio/BUILD.gn`
* Adjusting which target builds the moved files:
- `third_party/blink/common/BUILD.gn`
- `third_party/blink/public/BUILD.gn`
* Deleting `third_party/blink/common/browser_interface_broker_proxy.cc`
- This also involves tweaking
`third_party/blink/public/common/BUILD.gn`
- Parts of that `.cc` file have been preserved in
`.../platform/mojo/browser_interface_broker_proxy_impl.cc`
(e.g. `EmptyBrowserInterfaceBrokerProxy` and default definitions
of `BrowserInterfaceBrokerProxy`'s constructor and destructor)
Bug: 41482945
Change-Id: I6048c855279c438b7dbfc8467b859df70e2d2012
AX-Relnotes: n/a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5651622
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1327283}
At a high-level this CL moves the
`content::RenderFrameImpl::browser_interface_broker_proxy_` field into
`blink::LocalFrame`. This CL is based on an earlier attempt from
@mstensho at https://crrev.com/c/5105333 (thanks!).
This CL is motivated by the desire to make `BrowserInterfaceBrokerProxy`
store `blink::HeapMojoRemote<T>` (instead of a regular
`mojo::Remote<T>`) which means that `BrowserInterfaceBrokerProxy` can
**only** be used as a field of garbage collected classes (since
`HeapMojoRemote` has a `Member<T>` field and exposes a `Trace` method).
This CL does exactly this - after this CL `BrowserInterfaceBrokerProxy`
will only be embedded in garbage-collected classes (like
`blink::LocalFrame` and `blink::WorkerGlobalScope` and *unlike*
`content::RenderFrameImpl`).
The move requires splitting `BrowserInterfaceBrokerProxy` into a pure
virtual interface (exposing a public, somewhat Blink-agnostic API that
uses types like `CrossVariantMojo...`) and an impl (using Blink/WTF
idioms). The move opportunistically simplifies the public API by
pushing `Bind` and `is_bound` methods into the impl. The move
temporarily introduces some layering-related ickiness which will be
resolved by the follow-up CL at https://crrev.com/c/5651622.
Bug: 41482945
AX-Relnotes: n/a.
Change-Id: I411092bf1301e4f7f3994c8ec11532ab1a9eba25
Binary-Size: Artifact of PGO - see https://crbug.com/344608183#comment9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5647559
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1326840}
This CL changes the signature of the `GetBrowserInterfaceBroker` method
from:
`blink::BrowserInterfaceBrokerProxy* GetBrowserInterfaceBroker()`
to:
`const blink::BrowserInterfaceBrokerProxy& GetBrowserInterfaceBroker()`.
Motivation:
* The old implementation never returns null - this is communicated
in the new API by returning a reference instead of a pointer.
* Users of the old API never call `Bind` or `Reset` - this is
communicated/enforced in the new API by returning a **`const`**
reference (`Bind` and `Reset` methods of `BrowserInterfaceBrokerProxy`
are non-`const`).
Bug: 41482945
AX-Relnotes: n/a.
Change-Id: Ifec4c1eb0ee556f9e10a6da2e0f90f4c39bd2647
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5651361
Owners-Override: Alexander Timin <altimin@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1319846}
The canonical bug format is TODO(crbug.com/<id>). TODOs of the
following forms will all be migrated to the new format:
- TODO(crbug.com/<old id>)
- TODO(https://crbug.com/<old id>)
- TODO(crbug/<old id>)
- TODO(crbug/monorail/<old id>)
- TODO(<old id>)
- TODO(issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/u/1/issues/<old id>)
- TODO(bugs.chromium.org/<old id>)
Bug id mapping is sourced from go/chrome-on-buganizer-prod-issues.
See go/crbug-todo-migration for details.
#crbug-todo-migration
Bug: b/321899722
Change-Id: Ibc66b8c440e4bcdef414e77fef4d9874d2ea9951
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5493800
Auto-Submit: Alison Gale <agale@chromium.org>
Commit-Queue: Alison Gale <agale@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Owners-Override: Alison Gale <agale@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1293330}
The canonical bug format is TODO(crbug.com/<id>). TODOs of the
following forms will all be migrated to the new format:
- TODO(crbug.com/<old id>)
- TODO(https://crbug.com/<old id>)
- TODO(crbug/<old id>)
- TODO(crbug/monorail/<old id>)
- TODO(<old id>)
- TODO(issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/u/1/issues/<old id>)
- TODO(bugs.chromium.org/<old id>)
Bug id mapping is sourced from go/chrome-on-buganizer-prod-issues.
See go/crbug-todo-migration for details.
#crbug-todo-migration
Bug: b/321899722
Change-Id: Iee14d10d544e9f0ec046117cc4ec8a55c427adc0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5469947
Reviewed-by: Darryl James <dljames@chromium.org>
Owners-Override: Alison Gale <agale@chromium.org>
Commit-Queue: Alison Gale <agale@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1290838}
- When a same-doc nav finishes in the renderer, we generate a new
token. The token will be sent to both viz and back to the browser
(this CL). The renderer also bumps the LocalSurfaceId.
- When the browser receives this token via the DidCommitSameDocNav
message, the browser tags the corresponding navigation entry with
this token (follow up CL).
- When viz receives this token via the CompositorFrameMetadata, viz
issues a copy request against the old LocalSurfaceId, and tag the
copy request with the token (follow up CL). When the copy request
is fulfilled, viz sends the screenshot back to browser, and the
browser stashes the screenshot into the correct navigation entry
(follow up CL).
Low-Coverage-Reason: TESTS_IN_SEPARATE_CL browser tests in the last CL
Bug: 40259037
Change-Id: Ia284aca9abc81afe841ed7ab4bb74e0622841c74
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5444524
Reviewed-by: Khushal Sagar <khushalsagar@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: William Liu <liuwilliam@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1288916}
content/browser
The changes of this CL are made using the following script.
```
target_directory="content"
replace_string_in_files() {
old_string="$1"
new_string="$2"
find "$target_directory" -type d \( -path "$target_directory/browser" -prune \) -o \( -name "*.cc" -o -name "*.h" \) -exec sed -i '' "s/$old_string/$new_string/g" {} +
}
delete_include() {
find "$target_directory" -type d \( -path "$target_directory/browser" -prune \) -o \( -name "*.h" -o -name "*.cc" \) -print0 | while IFS= read -r -d '' file; do
grep -v '#include "base/strings/string_piece.h"' "$file" > "$file.tmp" && mv "$file.tmp" "$file"
done
}
add_include() {
find "$target_directory" -type d \( -path "$target_directory/browser" -prune \) -o \( -name "*.h" -o -name "*.cc" \) -print0 | while IFS= read -r -d '' file; do
local include_added=false
local tempfile=$(mktemp)
if grep -qE 'std::(string|u16string)_view' "$file"; then
while IFS= read -r line; do
echo "$line" >> "$tempfile"
if [[ $line =~ ^\s*#include ]]; then
if ! $include_added; then
echo "#include <string_view>" >> "$tempfile"
include_added=true
fi
fi
done < "$file"
mv "$tempfile" "$file"
if $include_added; then
echo "Added #include <string_view> after the first include line in $file"
else
echo "No include line found in $file"
fi
else
echo "std::string_view not found in $file"
fi
done
}
replace_string_in_files "base::StringPiece16" "std::u16string_view"
replace_string_in_files "base::StringPiece" "std::string_view"
delete_include
add_include
```
Replaced base::StringPiece16 with std::u16string_view
Replaced base::StringPiece with std::string_view
Removed header "base/strings/string_piece.h"
Added header "<string_view>" where applicable
Bug: 40506050
Change-Id: Ia6d2fd65a16e1a3db59532c085652fbb45dc6abc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5401324
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1285651}
This is achieved by preparing the updates to be serialized before the
call to AXReadyCallback. The call to RAI will consist all the updates
to be serialized from the start. The general aim is that the flow
becomes "AXCache -> RAI -> Browser" with zero going back and forth.
Change-Id: Ia32889f057f5d8503e96805a7142b946a12ad2cb
Bug: 41492890
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5321720
Reviewed-by: Stefan Zager <szager@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Ahmed Elwasefi (Ahmad45123) <a.m.elwasefi@gmail.com>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1276953}
This change replaces the implementation of createAuctionNonce so that
the Renderer process can immediately fulfill the Promise, removing
potential latency caused by a loss of the Renderer thread in the caller
to createAuctionNonce.
Change-Id: I2f4c19fdf712f855822e10e48c9ac5f692000581
Bug: 40275797
Fixed: 330545263
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5370409
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Orr Bernstein <orrb@google.com>
Cr-Commit-Position: refs/heads/main@{#1276320}
- Commit finish time on compositor is plumbed through to
WindowPerformance.
- Record commit finish time on all pending events that has
finished processing and are waiting for presentation promises.
- A new namehash is used for the tagging
Blink.Responsiveness.UserInteraction.MaxEventDurationFromQueuedToCommitFinish
Design doc: go/ssm-inp-tagging-queued
Bug: 323362553
Change-Id: Ifb34fa3ead3577ba09b08935e35bdab9e7c2dbe7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5315120
Reviewed-by: Deep Roy <dproy@chromium.org>
Commit-Queue: Rodney Ding <rodneyding@google.com>
Reviewed-by: Michal Mocny <mmocny@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1275029}
It's possible for a frame to be detached while in the middle of a
navigation commit on a provisional RenderFrame. See repro test at
third_party/blink/web_tests/http/tests/navigation/resources/reentrant-eventsource-onerror-crash-during-commit-iframe.html.
In this case, if the navigation commits on a provisional RenderFrame,
the provisional RenderFrame will be deleted first, before the previous
RenderFrame. Since we track provisional_frame_for_local_root_swap_
as a raw_ptr in the previous RenderFrame, this hits the dangling ptr
check. To avoid that, track the provisioanl RenderFarme with a WeakPtr
instead. It's ok to do that since if the frame is getting detached
there is no need to do anything to the provisional RenderFrame.
Bug: 936696
Change-Id: I8e855a21b30e2f1fbfb6def712bc29e79ed01ea1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5359220
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Fergal Daly <fergal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1271384}
This CL relands https://crrev.com/c/5107229.
The differences are
- This CL uses raw information e.g. WebInputEvent::Button instead of
WebMouseEventBuilder because layout object is not available in some
cases. See also b/327651304.
- This CL improves cancellation on D&D; Long press trigger is cancelled
when a mouse is dragged out of an anchor element and button is
released.
Fixed: b:314233999, 1522049
Bug: b/327651304
Change-Id: Icab554e31c2b7f52b1dd95b68a830aafa607fd79
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5344649
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Commit-Queue: Ken Okada <kenoss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1271379}
This reverts commit bbb01fbd12.
Reason for revert: issue 327651304 top Canary crasher
Original change's description:
> LinkPreview: Add alt+hover and long press triggers
>
> This CL adds alt+hover and long press triggers, and a feature param
> "trigger_type" for LinkPreview to control available trigger type.
>
> Before this CL, Link Preview used only Alt+click trigger. Now it is
> controlled by features::LinkPreviewTriggerType::kAltClick.
>
> For this purpose, this CL introduces
>
> - WebLinkPreviewTriggerer: receives signals and determine to trigger
> Link Preview.
> - WebLinkPreviewInitiator: mediates initiation request to blink side.
>
> Fixed: b:314233999, 1522049
> Change-Id: Ic3f2bc4108ce514156086c473a9727d7a10c27d0
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5107229
> Reviewed-by: Alex Ilin <alexilin@chromium.org>
> Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
> Reviewed-by: Kent Tamura <tkent@chromium.org>
> Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
> Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
> Commit-Queue: Ken Okada <kenoss@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1266909}
Change-Id: I2bf4e1ab0315bf327da2dc9f914fe18d5af5ba23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5334966
Owners-Override: Prudhvikumar Bommana <pbommana@google.com>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Prudhvikumar Bommana <pbommana@google.com>
Cr-Commit-Position: refs/heads/main@{#1267369}
This CL adds alt+hover and long press triggers, and a feature param
"trigger_type" for LinkPreview to control available trigger type.
Before this CL, Link Preview used only Alt+click trigger. Now it is
controlled by features::LinkPreviewTriggerType::kAltClick.
For this purpose, this CL introduces
- WebLinkPreviewTriggerer: receives signals and determine to trigger
Link Preview.
- WebLinkPreviewInitiator: mediates initiation request to blink side.
Fixed: b:314233999, 1522049
Change-Id: Ic3f2bc4108ce514156086c473a9727d7a10c27d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5107229
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Ken Okada <kenoss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1266909}
- Event queueing time on compositor is plumbed through to
ResponsivenessMetrics
- When an interaction is ready to be tagged we use the
queueing time as the start of samples
- A new namehash is used for the tagging
Blink.Responsiveness.UserInteraction.MaxEventDurationFromQueued
Design doc: go/ssm-inp-tagging-queued
Change-Id: Id7d29beeb4fdc200c495f1acf3d70fd042263144
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5259857
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Deep Roy <dproy@chromium.org>
Commit-Queue: Rodney Ding <rodneyding@google.com>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Reviewed-by: Michal Mocny <mmocny@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1260441}
This is a speculative fix for the crash in the referenced bug.
In the crash, SerializeEntireTree() hits a check that "has dirty
objects" is true, meaning that a previous serialization did not
complete.
Terminology -- two different concepts for "dirty" (we may want to
improve the terminology in a follow-up, so that the difference
between these concepts is less confusing):
1. AXObjectCacheImpl::IsDirty() means that the cache's tree updates
need to be processed so that it's stored tree is up to date.
2. AXObjectCacheImpl::HasDirtyObjects() means that there are objects
that haven't been serialized yet.
This CL adds checks to ensure that the 2 cache states related to
dirtiness have correct values before and after serializations,
when freezing, etc.
Fixes for triggered checks:
1. Return true from IsDirty() if the document has been marked dirty
via mark_all_dirty_, which ensures there's no chance of not
serializing in this case.
2. After processing tree updates, clean up any leftover aria-owns
changes in the relation cache that may have not been processed.
Bug: 1523372
Change-Id: I00d51660dc4fb255896ff75b0ea2b59784877389
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5249680
Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1257124}
This change uses base::ProtectedMemory, which was restored in
https://crrev.com/c/5161001 to harden MojoJS enablement against data-
only attacks.
The change adds a new static member to blink::ContextFeatureSettings
that is a base::ProtectedMemory<bool>. A method to set this value to
true is added, and is called in RenderFrameImpl in all code paths that
enable MojoJS for a RenderFrame.
Finally, the isMojoJSEnabled method of blink::ContextFeatureSettings,
is augmented to CHECK that the new base::ProtectedMemory<bool>
value is true if enable_mojo_js_ is true.
The result is that if a data-only attack is used to directly enable
MojoJS outside of the method calls, the CHECK will fail and crash the
process.
Tests to verify that all the normal code paths continue to properly
enable MojoJS, as well as tests that verify that tampering with the
vulnerable properties results in a crash have been added.
Bug: 976506
Change-Id: If4f8f856edab0e60ac7e48ad65ac759aa24b2ebc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5218835
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Jeffrey Gour <jegour@microsoft.com>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1255826}
- Add a new GetNewProcessLabelId method to return a unique label id.
- This avoids the dependency on routing IDs in the RenderFrame which
eventually will go away.
Bug: 1502660
Change-Id: Ifcaeff30dad038233a53c6a55ee1dcf2bec3f7d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5181418
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1245410}
- Do not build RenderFrame as an IPC::Sender/Listener
- Remove public FromRoutingID lookup
- Remove requesting the routing ID from RenderFrame
- Do not build ChildThread as an IPC::Sender
- This leaves all the legacy IPC code in ChildThreadImpl the same.
- We still require legacy IPC for some associated channel interfaces (see
RenderThread::GetChannel callsites, and
RenderThreadObserver::RegisterMojoInterfaces).
- The scheduling differences between associated channel interfaces and
normal associated interfaces still also need to be resolved to disable legacy IPC over the wire.
Bug: 1502660
Change-Id: I6343377ee82a8284ce8a9734f6e5719f6ca102a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5105871
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1242431}
