This a reland of f3df55ae2d. It adds
an explicit public_dep to the automationInternal API that's needed
for a fuzzer test.
This is the first in a series of CLS to clean up the dependencies in
the various API implementations.
Bug: 40593486
Change-Id: I00bb1fdbab85778caa7eae228e048332610626c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6090029
Commit-Queue: Emilia Paz <emiliapaz@chromium.org>
Reviewed-by: Emilia Paz <emiliapaz@chromium.org>
Auto-Submit: David Bertoni <dbertoni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1395196}
The extension system could incorrectly assume a service worker was
still running even though it was stopping or had stopped. This could
lead to webRequest events being dispatched to inactive listeners,
potentially causing events to be sent but never received by the worker.
This change ensures that active listeners associated with a stopping or
stopped service worker are removed immediately. This prevents events
from being sent to these inactive listeners.
Additionally, the service worker is now untracked from the
ProcessManager when it's in the stopping state, similar to how it's
handled in the stopped state.
This also additionally untracks both the browser and renderer state
during stopping and stopped synchronous browser notifications.
Bug: 331358156
Change-Id: I18c055c697a0b7897d917d0084f2f340962d6ac7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6071609
Commit-Queue: Justin Lulejian <jlulejian@chromium.org>
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Auto-Submit: Justin Lulejian <jlulejian@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1395117}
Before this change HidDeviceManager held onto a raw_ptr<EventRouter>
even after context keyed service (EventRouter) could be destroyed.
After this the HidDeviceManager now depends on EventRouter so it will
be destroyed before EventRouter in the future.
This fixes a crash in a downstream CL where we change the dependencies
of EventRouter which causes it to destruct earlier.
Bug: 331358156
Change-Id: I3dcecdab12661037fdc918b8508b2c8a8d20351b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6085811
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Auto-Submit: Justin Lulejian <jlulejian@chromium.org>
Commit-Queue: Justin Lulejian <jlulejian@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1394546}
Before this change we covered some of the early bail-outs when
attempting to open a channel to an extension, but there are five that
were not. If these cases were hit the metric would be emitted as "hung"
which isn't accurate.
After this we emit for every known control flow of opening a channel to
an extension so any future "hung" metrics are truly because the attempt
hung during one of the stages of opening a channel.
Bug: 371011217
Change-Id: I12eeb62900ef392c0ffc40ac1feb9375c8f95a48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6085803
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Auto-Submit: Justin Lulejian <jlulejian@chromium.org>
Commit-Queue: Justin Lulejian <jlulejian@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1394465}
media permission requests in webviews must be approved
by the embedder. Approved requests are currently forwarded
without modification -- they will appear to come from the
embedded origin. This CL changes this behavior for chrome:
origins that embed webviews: the forwarded permission
request will appear as coming from the chrome: origin.
Note that media permission differs from location permission,
which is already forwarded as coming from the embedder origin.
This CL does not change behavior on chrome-extension embedders,
so is unlikely to materially affect any existing code or features.
This change is to support chrome://glic
Bug: 377975471
Change-Id: Ia67d4a6435ce61b46579709052af39760691c018
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6072787
Commit-Queue: Dan H <harringtond@chromium.org>
Reviewed-by: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1394350}
Include Chrome commandline variations in feedback report so developers
can repro experiment related issues for debugging.
When user create a feadback report and the "Include system information"
checkbox is checked, we will include Chrome commandline variations in
the report. The data is encrypted. Only authorized developers can
decrypt the report for repro purpose.
Only affects Chrome on Windows, Linux, Mac.
The public key was created with cl/700793735. Please see the cl for
the details about how I created the key and testing.
Design doc: go/finch-listnr-design
successfully with the tool in cl/700793735.
Test: Tested on Linux. Created a feedback report and decrypted
Bug: 373470046
Change-Id: Ib252818bf257cce7fed7c3ef4ca449599b590887
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6001442
Reviewed-by: Elly FJ <ellyjones@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Sven Zheng <svenzheng@chromium.org>
Reviewed-by: Xiangdong Kong <xiangdongkong@google.com>
Cr-Commit-Position: refs/heads/main@{#1393801}
This change adds support for incognito split mode in the Android extension.
In this mode, each profile (regular and incognito) have its own extension system, but they need to share some underlying services to work correctly. The desktop implementation achieves this using a
`shared_` instance (a profile keyed service under //chrome/browser),
see:
https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/extensions/extension_system_impl.h;drc=5108636c70c0b08fdbeb57de2640a22e138f6685;bpv=1;bpt=1;l=91
However, DesktopAndroidExtensionSystem currently doesn't have this functionality. Each instance of DAExtensionSystem has its own instances
of the underlying services, which are initialized for regular profile
by InitForRegularProfile(), but never initialized for incognito profile.
As a result, when an extension running with incognito profile in split
mode tries to execute a extension function which uses a service (like
the quota service), it crashes because the service hasn't been initialized. For example,
OffscreenApitest.IncognitoModeHandling_SplitMode crashed at:
https://source.chromium.org/chromium/chromium/src/+/main:extensions/browser/extension_function_dispatcher.cc;l=375?q=ExtensionFunctionDispatcher::DispatchWithCallbackInternal&ss=chromium%2Fchromium%2Fsrc
This cl fixes the crash by explicitly calling InitForRegularProfile()
to initialize the services from the test, even though it's not the ideal solution.
The correct approach is to implement a shared instance mechanism for ExtensionSystem on Android, like the desktop implementation.
This cl also ports 2 api tests for incognito profile in split
mode.
Bug: 356905053, 378916068, 371263572
Change-Id: I0c64db2261c8680171ab6c005146670de2f22b90
Cq-Include-Trybots: luci.chromium.try:android-desktop-14-x64-rel
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6068688
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Commit-Queue: Jenny Zhang <jennyz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1393050}
When parsing JSON ruleset files, we assume that each rule would be
specified as a dictionary if we couldn't parse it into a dnr_api::Rule
Unfortunately this is not the case, which causes the crash when
attempting to access rules_list[i[.GetDict() which CHECKs that
rules_list[i] is in fact a dict (and crashes if it's not)...
Somewhat simple fix: fall back to the index for reporting parse error
if the rule is not a dict.
Bug: 378335062
Change-Id: I196e33f236fe5ad6607f6e5e1b2b8d62a3a233cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6074925
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Auto-Submit: Kelvin Jiang <kelvinjiang@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1392680}
New Metric:
Extensions.MessagePipeline.OpenChannelWorkerDispatchStatus{DispatchTarget}{ChannelType}
This metric tracks the result of each time the DispatchOnConnect IPC is
sent to a frame or a worker. It tracks whether the message was:
* received by the browser (acked)
* channel/port disconnect occurred before the message was acked
* or none of the above (hung)
Bug: 371011217
Change-Id: I35ef71c9e176feae1c8e1d4856bcec2932f1d94a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6065439
Commit-Queue: Justin Lulejian <jlulejian@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Auto-Submit: Justin Lulejian <jlulejian@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1391446}
It looks like the goal at one point was to disable this in M95, but that
has long come and passed. The tracking task for it was closed too, so I
think this can be removed.
Fixed: 356624393, 40752831
Change-Id: I6abb5ab24ec4216698252c837891327eedcb5720
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6052091
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Commit-Queue: Ari Chivukula <arichiv@chromium.org>
Auto-Submit: Ari Chivukula <arichiv@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1391129}
This is a reland of commit cd4c24d550
Patch set 1 is as landed. Patch set 2 fixes Android desktop build.
Original change's description:
> [MPArch guest view] Fix loading extension resources inside a guest view
>
> - Ensure that the view type is correct for guest views, this previously
> was computed from the WebContents but we should use the RenderFrameHost
> for this.
> - Provide an API to get the owner RenderFrameHost before attachment.
>
> Bug: 40202416
> Change-Id: I351313f06cd7a221e9c9bae0e3c155e827b3be51
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6053978
> Reviewed-by: Greg Thompson <grt@chromium.org>
> Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
> Reviewed-by: David Bertoni <dbertoni@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1390990}
Bug: 40202416
Change-Id: I60c6cb3b2f7929e280a27d456b4ab1deef44b3ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6064556
Reviewed-by: Greg Thompson <grt@chromium.org>
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1391117}
- Ensure that the view type is correct for guest views, this previously
was computed from the WebContents but we should use the RenderFrameHost
for this.
- Provide an API to get the owner RenderFrameHost before attachment.
Bug: 40202416
Change-Id: I351313f06cd7a221e9c9bae0e3c155e827b3be51
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6053978
Reviewed-by: Greg Thompson <grt@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1390990}
This works around a few macros that sometimes-CHECK() since we can't
generate dead-code warnings in some builds but not others.
CHECK_LT(1, 1) still isn't understood as [[noreturn]] when
CHECK_WILL_STREAM() in the current implementation. A TODO is left to
address that, but whether that's practically useful/beneficial.
Bug: 40122554
Cq-Include-Trybots: luci.chromium.try:linux-asan-rel
Validate-Test-Flakiness: skip
Change-Id: If3b074982b65abb9e7f8ccc46ffb030285e0b7b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6043197
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Owners-Override: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1389181}
This relands commit 0e35ed6882.
This was reverted because of an issue where a MimeHandlerViewGuest
had a RenderFrameHost swap, which meant that handles in the test
were invalidated, leading to UAFs (in the test only). However, since
other code may be relying on the same type of behavior, fix this by
going back to using a direct SiteInstance in the MimeHandlerViewGuest.
----- Original CL Description -----
This CL is based off nasko@'s CL at
https://chromium-review.googlesource.com/c/chromium/src/+/5142350.
For various historical reasons, there's a "primordial" shared
SiteInstance used for most extension hosts that's owned by the
ProcessManager. This should no longer be necessary, and has some
potentially undesirable side effects.
This CL sets that SiteInstance to null if the corresponding
feature kRemoveCoreSiteInstance is enabled. This is effectively the
same as removing the member, since the SiteInstance is now only used
in the creation of ExtensionHosts' WebContents, which can accept a
null SiteInstance (the same as passing no SiteInstance).
This requires adjusting a number of callsites, primarily those around
ExtensionHost creation, in order to provide them with a BrowserContxt
member directly. In the future, once we're sure this change is safe,
we will remove the SiteInstance member from ProcessManager entirely.
In addition to plumbing, this required:
- Changing where we set a temporary zoom level for extension popups
to be immediately before we're ready to commit, rather than in the
constructor; this is necessary since the primary RenderFrameHost
will be swapped between these points.
- Updating how a few tests wait for readiness (to use more correct
methods, rather than relying on racy conditions)
- Updating IsExtensionIdle() to look for a process entry in the
ProcessMap instead of checking the SiteInstance from the
ProcessManager.
Bug: 334991035
Change-Id: Ib682f8dee02985bbef32be83e17fe3d42323d257
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6041245
Reviewed-by: Andy Phan <andyphan@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Kevin McNee <mcnee@chromium.org>
Reviewed-by: Danil Somsikov <dsv@chromium.org>
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Reviewed-by: Owen Min <zmin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1388577}
Metrics:
Extensions.MessagePipeline.OpenChannelStatus{WorkerStatus}{ChannelType}
Extensions.MessagePipeline.OpenChannelWorkerDispatchStatus{ChannelType}
Extensions.MessagePipeline.OpenChannelWorkerWakeUpStatus{ChannelType}
This tracks several stages of the initial open channel portion of
sending an extension message.
It tracks each stage of this process as a separate metric. It emits a
relevant value from MessageTracker::OpenChannelMessagePipelineResult
that indicates where the process finishes (or fails).
It also make some minor changes to ServiceWorkerTaskQueue to make it
easier to emit a metric that is worker-based extension specific.
After this change we will be able to determine where in the open channel
process we might be experiencing issues which could contribute to
messages to worker failures.
Low-Coverage-Reason: OTHER message_port.cc DispatchOnConnect non-pure virtual method is empty.
Bug: 371011217
Change-Id: I4ae6628018a4d3d1c2e367244bf787f7db1b326c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5938256
Auto-Submit: Justin Lulejian <jlulejian@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1388520}
Changes subspan(), split_at(), at(), and get_at() to match pre-existing
usage in constructors, first(), last(), etc.
Bug: 364987728
Low-Coverage-Reason: LARGE_SCALE_REFACTOR not significantly changing existing functionality
Change-Id: Ic7a33ebbfd7f70d4eed961fa78435d26bfac4bbe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6044946
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Owners-Override: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1388513}
This class was only ever implemented by ProcessManager, and had two
methods WakeEventPage() and IsEventPageSuspended().
IsEventPageSuspended() was only used in a single test file, and just
checks that there is no background host for an extension. This can be
trivially done by checking GetBackgroundHostForExtension() (which is
what the implementation did).
WakeEventPage() is called by numerous sites, but all of them do so by
accessing the ProcessManager. No sites accept or use the class as the
EventPageTracker, so having a separate class that the ProcessManager
inherits from is unnecessary.
Bug: None
Change-Id: Ie83d9b5eb83dcb458a29ab51a46ea33797203af4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6044582
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1387960}
Per conversations in the WECG [1], it's agreed that we should rename
permissions.addSiteAccessRequest and permissions.removeSiteAccessRequest
to permissions.addHostAccessRequest and
permissions.removeHostAccessRequest, since the "host" term is more
commonly used in other APIs and extension surfaces (such as
"host_permissions" in the manifest).
This CL renames the API function and corresponding C++ classes and
methods. Since the API was only released in M133 (which is still
pre-stable), this shouldn't be a breaking change for any published
extensions.
[1] https://github.com/w3c/webextensions/pull/529#issuecomment-2492042845
Fixed: 380291902
Change-Id: I38f118caba94526336ddbbfe89bea34ce42fd500
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6039497
Reviewed-by: Oliver Dunk <oliverdunk@chromium.org>
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Reviewed-by: Kelvin Jiang <kelvinjiang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1387925}
Currently DNR treats the checksum in prefs as the source of truth.
If a checksum for a ruleset is in prefs, then whenever that ruleset gets
enabled or re-indexed (except in the case of a version mismatch), then
the ruleset fails to load if the checksum from prefs does not match
the checksum obtained from re-indexing.
However, if prefs are corrupted or updated to a bad value as a result
of some kind of race condition, then that ruleset will never load.
Instead, we should treat the ruleset JSON files in the extension
package as the source of truth, so if a checksum mismatch occurs,
simply re-index the ruleset and update the checksum.
Adding content verification to guard against corruption/unwanted edits
to those ruleset files will be done in a follow up.
Bug: 380434972
Change-Id: I7aabb269c7ecb6935b328e07dbb8fdca5c563287
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6043767
Commit-Queue: Kelvin Jiang <kelvinjiang@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1387868}
This CL cleans up ProcessManager construction in a few ways:
* Removes CreateForTesting(). This just called the (public) ctor, and so
was unnecessary.
* Removes CreateIncognitoFortesting(). This was never used.
* Removes the `original_context` parameter passed to
ProcessManager's ctor. This was only used to DCHECK that it was the
same context as the one used on the ExtensionRegistry since the
ExtensionRegistry is a shared service across on- and off-the-record
contexts, which isn't really the purview of the ProcessManager to
verify.
* Moves the Create() method used by the ProcessManagerFactory to be
public. Since the ctor is public, there's no reason to have the
Create() method be protected and require a friend class of the
factory.
Bug: None
Change-Id: I14203330517de486d066cd5a912c4ccc9ca50665
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6044293
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1387768}
Extensions can inject in multiple user script worlds, which are
specified by different user script world (string) IDs. If an ID isn't
specified, the extension injects in the default user script world ID.
However, this creates an issue with trying to update a registered
script to inject in the default world after initially registered with
a non-default world:
* Specifying any non-null value indicates a non-default world
* Specifying a null value will be dropped by the extension bindings
To enable this, allow extensions to use the empty string ('') as an
indicator that a script should use the default world. The API layer
converts the empty string to nullopt for use elsewhere in the
extensions system.
Bug: 331680187
Change-Id: I4b0ef064884dc5fac814c0680134c278d0b28fcc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6036255
Reviewed-by: Kelvin Jiang <kelvinjiang@chromium.org>
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1387164}
