NavigationClient is now on by default, so removing an unused branch in tests.
Also removing a test that does not make sense anymore.
Bug: 784904
Change-Id: Ibf91fdfbdc23d03a9f5a966ca02c82314fcdca97
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1821558
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708177}
In order to remove the dependency, the web_triggering_event_info.h file was
moved to public/common/navigation. Since the file is no longer in the public/web
directory we are also renaming it to triggering_event_info.h (and the enum
defined in that file to TriggeringEventInfo).
In order to avoid clashes between class and method names the
TriggeringEventInfo() method in FrameLoader had to be renamed to
GetTriggeringEventInfo().
Bug: 1008303
Change-Id: I0daba0ea71c1ebe879d78db39844d41db727ee0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1860417
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Sergio Villar <svillar@igalia.com>
Cr-Commit-Position: refs/heads/master@{#706835}
In RenderFrameHostManager, the speculative RenderFrameHost may be
deleted by using CleanupNavigation(). The speculative RenderFrameHost
may contain multiple pending NavigationRequests. When they are deleted,
their matching pending NavigationEntry must be discarded.
This is now done automatically in NavigationRequest's destructor by
releasing the PendingEntryRef, thanks to patch:
https://chromium-review.googlesource.com/c/chromium/src/+/1815129
Bug: None.
Change-Id: I5f88e5dafa5188eeab29d5c5a55b8f8306bd83d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1860031
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705956}
Site Isolation and process allocation should work correctly even in
absence of a cooperating renderer process - there should be no need for
the renderer-side ChromeContentRendererClient::ShouldFork. This CL
removes the view-source: and file: related ShouldFork considerations.
Bug: 1003957, 883549
Change-Id: I5598807c358bf81f9c9e827383d0d1dd51d5ba17
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1841667
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705329}
The FrameHostInterceptor observe RenderFrameHost creation and
destruction. In its constructor, it simulate observing RenderFrameHost
creation for every active RenderFrameHost. It misses the ones in the
BackForwardCache. This causes a mismatch later when it observe a
RenderFrameHost deletion.
Change-Id: Ifabf5eab92f3275d71e5a7d760f44e4178e00d6a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1816509
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705020}
ScopedFeatureList is unsafe to use after browser threads have been
started. This constraint will imminently be enforced by DCHECK to
prevent further erroneous usage from landing.
This CL corrects usage within miscellaneous Content browser tests
as well as Chrome tests related to site islation.
This is split from a larger CL where in some rare cases, correction
was too complex to resolve before landing the DCHECK, so corresponding
test(s) may be disabled instead of fixed.
Bug: 846380
Change-Id: Idf6fcaabc37d09b49acb70ad3bd177985bed91cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1850736
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704749}
This CL converts NavigationClientAssociatedPtrInfo,
NavigationClientAssociatedPtr, NavigationClientPtr,
and NavigationClientAssociatedRequest to new
Mojo types.
It also updates BeginNavigation from frame.mojom
and methods and members which implements it with
new Mojo types.
Bug: 955171
Change-Id: I2ed4aa7cdf57361c982e3adc696cc9ead797630b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1797823
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Julie Kim <jkim@igalia.com>
Cr-Commit-Position: refs/heads/master@{#695984}
This CL converts WidgetPtr to new Mojo types.
It updates CreateNewWidget() and CreateFullscreenWidget()
from render_message_filter.mojom and methods and members
which implement these interfaces.
Bug: 955171
Change-Id: Ibc1a871b182716c216e90d4a9d041917cf35e278
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1792471
Commit-Queue: Julie Kim <jkim@igalia.com>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695903}
To enforce using main Document's origin
for subresources of HTML imported Documents,
this CL disables loading using HTML imported Documents' ResourceFetcher,
and switches the following subresources to use context document's
ResourceFetcher and origin:
- classic <script>s,
- Styleheets loaded via <link rel="stylesheet">, and
- Styleheets loaded via @import
(context documents should be the main Documents for HTML imported Documents)
This CL doesn't modify the following subresources:
- XHR and Fetch API (because they get the ResourceFetcher from
the current execution context obtained from the current ScriptState,
which is already the same as the context document)
- <img>s and other subresource types; they seem disabled
in HTML imported Documents, or not covered by tests.
We'll add a UMA to see if there are any subresources missed by this CL.
This CL doesn't modify base URLs,
i.e. keeps using HTML imported Documents' base URLs.
[The changes outside third_party/blink/renderer/ and
the following paragraphs are originally by lukasza@chromium.org from
https://chromium-review.googlesource.com/c/chromium/src/+/1750084]
This change affects some scenarios where HTML imports can end up using a
request_initiator that doesn't match request_initiator_site_lock. We
think that this breaking changes is acceptable, because:
A) The old behavior leads to a security bug that can be exploited even
without compromising a renderer - see https://crbug.com/961614.
B) The behavior is behind the features::kTrustworthyRequestInitiator
kill switch, so we can quickly disable the new behavior if any
trouble is spotted on the Beta or Stable channel. See also
blink::features::kHtmlImportsRequestInitiatorLock.
C) HTML Imports are deprecated at M70, and will be removed in M80.
See also https://www.chromestatus.com/feature/5144752345317376
D) The behavior affects only a subset of fetches related to HTML
imports. In particular, most fetches will use request_initiator
set to the importer of the module. Only some fetches will use
request_initiator set to the module origin. For more specific
examples see:
https://docs.google.com/document/d/153oiCNNMksDWuvzY36WF1iD_i_ahs9X6RXxsZcpfuyg
(listing of how "bar.com/module.html requests of “/resource.json”
in 3 different ways")
E) It seems unlikely that the module author / the module server
would CORS-allow the module importer to request D1) the module
itself but D2) not the subresources fetched by the module.
For reference, this aspect was discussed in May 2019 in an internal
email thread titled "HTML Imports and enforcing security properties".
Bug: 961614
Change-Id: Ifefeb786a0d8773c51533b2b16bd7643b83466db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1753330
Auto-Submit: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693544}
This CL converts DocumentInterfaceBroker{Ptr, Request} in content,
blink, extension and media to the new Mojo types and uses
pending_remote<blink.mojom.DocumentInterfaceBroker> in
document_scoped_interface_bundle.mojom and
pending_receiver<blink.mojom.DocumentInterfaceBroker> in
frame_messages.mojom.
Bug: 955171, 978694
Change-Id: Ic0ccabb445febbf8b2bd81aa754fc0abecfb77b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1781900
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693066}
This CL converts NavigationInitiator{Ptr, Request} in content and
blink to the new Mojo types, and uses
pending_remote<blink.mojom.NavigationInitiator> in frame.mojom.
Bug: 955171, 978694
Change-Id: I89d54747b61cc1cfa4e0ee46c1013b560150c07e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1776157
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#692429}
This CL converts BlobURLTokenPtr and BlobURLTokenPtrInfo
to new Mojo types.
It updates BeginNavigation from frame.mojom,
CreateWorkerHostAndStartScriptLoad from
dedicated_worker_host_factory.mojom, and Connect from
shared_worker_connector.mojom
Bug: 955171, 978694
Change-Id: If9cbac3e3500803e9f9bb8914c59c8ba14ea2149
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1774422
Commit-Queue: Julie Kim <jkim@igalia.com>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691955}
This converts a few more files to expect the right result from
NavigateToURL(), fixing behavior in several tests in the process.
Most of these files had tests that would fail if we simply added
EXPECT_TRUE to their use of NavigateToURL(). Some of the causes of
that were:
- Navigating to invalid/broken URLs
(MHTMLGenerationTest.InvalidPath, TracingControllerTest.*)
- Navigating to javascript URLs that actually ended up navigating
the page (or some other frame) somewhere else.
(BookmarkletTest.Redirect*, SessionHistoryTest.*)
- Navigating to URLs that redirect to other URLs
(VerifyCrossSiteRedirectURL, RedirectAndRespondWithNavigationPreload)
- Navigating to URLs with a broken test server that was incapable
of actually serving the right files.
(NetworkServiceBrowserTest.SyncXHROnCrash)
- Navigating to URLs that result in a crash
(SecurityExploitBrowserTest.DidCommitInvalidURL)
- Navigating to a webui URL that doesn't exist in content/
(WorkerNetworkIsolationKeyBrowserTest). Bonus points for
then relying on cross-process behavior of error page
isolation to accidentally satisfy the test's requirements.
Bug: 425335
Change-Id: I471479b5811f89793f105dce429fa13f342e0df5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1759900
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688577}
*** Note: There is no behavior change from this patch. ***
The PostTask APIs will shortly be changed to require all tasks to explicitly
specify their thread affinity, i.e., whether the task should run on the thread
pool or a specific named thread such as a BrowserThread. This patch updates all
call sites with thread affinity annotation. We also remove the "WithTraits"
suffix to make the call sites more readable.
Before:
// Thread pool task.
base::PostTaskWithTraits(FROM_HERE, {...}, ...);
// UI thread task.
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI, ...}, ...);
After:
// Thread pool task.
base::PostTask(FROM_HERE, {base::ThreadPool(), ...}, ...);
// UI thread task.
base::PostTask(FROM_HERE, {BrowserThread::UI, ...}, ...);
This patch was semi-automatically prepared with these steps:
1. Patch in https://chromium-review.googlesource.com/c/chromium/src/+/1635827
to make thread affinity a build-time requirement.
2. Run an initial pass with a clang rewriter:
https://chromium-review.googlesource.com/c/chromium/src/+/1635623
3. ninja -C out/Debug | grep 'requested here' | cut -d: -f1-3 | sort | \
uniq > errors.txt
4. while read line; do
f=$(echo $line | cut -d: -f 1)
r=$(echo $line | cut -d: -f 2)
c=$(echo $line | cut -d: -f 3)
sed -i "${r}s/./&base::ThreadPool(),/$c" $f
done < errors.txt
5. GOTO 3 until build succeeds.
6. Remove the "WithTraits" suffix from task API call sites:
$ tools/git/mffr.py -i <(cat <<EOF
[
["PostTaskWithTraits", "PostTask"],
["PostDelayedTaskWithTraits", "PostDelayedTask"],
["PostTaskWithTraitsAndReply", "PostTaskAndReply"],
["CreateTaskRunnerWithTraits", "CreateTaskRunner"],
["CreateSequencedTaskRunnerWithTraits", "CreateSequencedTaskRunner"],
["CreateUpdateableSequencedTaskRunnerWithTraits", "CreateUpdateableSequencedTaskRunner"],
["CreateSingleThreadTaskRunnerWithTraits", "CreateSingleThreadTaskRunner"],
["CreateCOMSTATaskRunnerWithTraits", "CreateCOMSTATaskRunner"]
]
EOF
)
This CL was uploaded by git cl split.
R=boliu@chromium.org, tsepez@chromium.org
Bug: 968047
Change-Id: I346372d16a3856186ea74d14e0dd8a12f7cacae5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1729589
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683554}
- "cross-origin-embedder-policy" set's the policy for frame.
- dedicated worker's policy is set to the ancestor frame's policy.
- nested frames with a conflicting policy is blocked.
SharedWorker / ServiceWorker are not yet supported. All the
implementation is behind the flag, and tested manually with
work in progress WPTs[1].
1: https://github.com/web-platform-tests/wpt/pull/17606
Bug: 887967
Change-Id: I70ed24841afde1b3c72dad40288744bb92a6f5dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1715378
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682609}
This CL adds the mojom interface that the renderer will use to get
interfaces from the browser, as well as its implementation, functions
for registering interface handlers and all the necessary plumbing.
This CL also converts AudioContext to use BrowserInterfaceBroker.
Bug: 718652
Change-Id: I6b24bc802ca482feac1d8ae5fff7e5bf44215fa4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1688547
Commit-Queue: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#681341}
This change allows a top-level empty SiteInstance to become the default
SiteInstance if the URL for the top-level frame does not require a
dedicated process. This allows the top-level frame and any subframes
that do not require a dedicated process to share the same default
SiteInstance process.
Bug: 958060,78757
Change-Id: I920860b822474157897a0f3e3534cca87984599c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1693778
Commit-Queue: Aaron Colwell <acolwell@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680472}
|allow_universal_access_from_file_urls| is one of
content::WebPreferences, that used to affect some layout tests and that
is exposed publicly via
WebSettings.setAllowUniversalAccessFromFileURLs [1].
Note that allow_universal_access_from_file_urls needs to be set *before*
a document is committed, because this setting is only inspected by
blink::Document::InitSecurityContext. In other words, it is not
possible to change this setting during the lifetime of a frame (or
during the lifetime of a URLLoaderFactory).
Before this CL:
1. CORB would unconditionally ignore requests initiated from file:
origins (in the past some layout tests would fail + blocking
requests from file: origin might break Android WebView cases
that have opted into |allow_universal_access_from_file_urls|).
2. OOR-CORS would ignore |allow_universal_access_from_file_urls|
(possibly breaking some Android WebView scenarios - before this CL
the new CorsFileOriginBrowserTest.UniversalAccessFromFileUrls test
would succeed in InBlink mode and fail in InNetworkService mode).
After this CL:
3. RenderProcessHostImpl::CreateURLLoaderFactory takes an optional
|content::WebPreferences*| argument and based on that argument
may disable CORB and OOR-CORS by setting appropriate
URLLoaderFactoryParams.
4. CORB in NetworkService no longer unconditionally skips processing of
requests initiated from file: origins
(//services/network/cross_origin_read_blocking.cc changes). OTOH,
this behavior is preserved in pre-NetworkService code
(//content/browser/loader/cross_site_document_resource_handler.cc
changes).
5. Regression tests have been added:
- CorsFileOriginBrowserTest.UniversalAccessFromFileUrls
- fails before this CL in OOR-CORS/InNetworkService mode
- shows that before this CL OOR-CORS might have broken Android
WebView; note that this step would fail if item3 above would
not disable *both* OOR-CORS *and* CORB.
- CrossSiteDocumentBlockingTest.BlockImages
- now also covers requests from file: origin (this part fails
before this CL)
[1] https://developer.android.com/reference/android/webkit/WebSettings#setAllowUniversalAccessFromFileURLs(boolean)
Bug: 873483, 980629
Tbr: anthonyvd@chromium.org for //components/translate
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I7f71461a5ddd8225939df1e414689d58f7d99bda
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1172935
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#674692}
This CL adds a network isolation key as a member in URLLoaderFactoryParams
which is used as the key for the Http cache and is propagated to the cache
in every URLRequest.
This CL is scoped to create and fill the key for subresources
which is done in RenderFrameHostImpl when a navigation is being committed.
RenderProcessHost::CreateURLLoaderFactory() interface is changed to accept
the network isolation key as an argument.
Follow up CLs would fill it up for navigation requests and worker
subresources.
Added unit tests in url_loader_unittests.cc and for the new mojom traits.
The existing browser test WebContentsSplitCacheBrowserTestEnabled.SplitCache
tests this change and the helper function is modified to also test the http cache
key in the RenderFrameHostImpl.
Bug: 910708
Change-Id: I066d53df5c1ad1abe84ecce67697ddd2bb5ee11c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1610218
Commit-Queue: Shivani Sharma <shivanisha@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: anthonyvd <anthonyvd@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#668057}
Rather than having the browser and renderer processes try to allocate
non conflicting integers, and keying everything of process id +
host id, instead identify all appcache hosts by a unguessable token.
This hopefully fixes some race conditions when a process id gets
reused for a new process before the old state has fully torn down, and
should make the code slightly more robust against compromised renderers
trying to claim pre-registered browser created AppCacheHosts that were
meant for a different process/frame.
Bug: 963661
Change-Id: I4fbf1835c3ca22ee26e7d1b9a6bd142d0152465d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1620697
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662283}
This is a small code change with a lot of test changes. The two most
common reasons for those changes are:
* Cases where the ordering of messages reaching the browser process
changed in ways that browser_tests were sensitive to.
* A navigation scheduled by NavigationScheduler doesn't prevent a
DidStopLoading() callback, but starting the navigation
immediately does prevent DidStopLoading(). So location API
navigations inside onload used to result in two pairs of
DidStartLoading/DidStopLoading callbacks, where there is now only
one pair.
Bug: 914587
Change-Id: I83b016041c4579a64cf9ccfd7876dba335b799d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1526426
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#653352}
This check was initially added in r629679 and then reverted in r633938
due to unexpected renderer process terminations. Some bugs were discovered
and fixed in the meantime, so this is an attempt to add the check back.
The CL includes crash keys to capture more information about the state
when the process is terminated to aid debugging. The instrumentation
will be removed separately if all goes smoothly.
Bug: 918565, 931895
Change-Id: I4de255731ee8251714132fdbea56777da4e4b440
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1549318
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652724}
This allows us to be more explicit when content::ResourceType needs to
be smuggled in an untyped int. It has the effect of introducing more
static_cast<int>, but this makes the conversion sites explicit.
This also allows several other incidental fixes:
- the Mojo version of the enum no longer defines a sentinel value (which
violates best practices in the security guidelines)
- the C++ version of the enum can now define a compiler-enforced
kMaxValue enumerator.
- Since kMaxValue is no longer a distinct value, dummy handling for this
value can be removed. This revealed a quirk in
chrome/browser/predictors/loading_data_collector.cc
Tbr: avi@chromium.org
Tbr: blundell@chromium.org
Tbr: droger@chromium.org
Tbr: rdevlin.cronin@chromium.org
Change-Id: I7203a029b1fcb06426c957d208e9278a25d4a45b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1569345
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652395}
This patch verifies that incoming navigation requests from renderer processes
can only have `ui::PageTransition` values that meet the requirements of
`PageTransitionIsWebTriggerable()`. The process will be terminated if that
invariant is violated.
Bug: 946976
Change-Id: Ia1446964020455a05e824fbfb2456ea2d36cea6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1543255
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#646284}
If a renderer process is compromised, and it calls both of
FileChooserImpl::OpenFileChooser() and EnumerateChosenDirectory() via
Mojo, the browser process could crash because ResetOwner() for
the first FileChooserImpl::proxy_ instance was not called. We
should check nullness of proxy_ before updating it.
Bug: 941008
Change-Id: Ie0c1895ec46ce78d40594b543e49e43b420af675
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1520509
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#640580}
All renderer-initiated navigations should specify an |initiator_origin|
(CommonNavigationParams::initiator_origin is a base::Optional to
accomodate browser-initiated navigations and some extension APIs which
need to distinguish between no origin VS a unique origin).
Bug: 919144
Change-Id: I91ad5a2ecc1ac81c7f13f16f6eb281f814866411
Reviewed-on: https://chromium-review.googlesource.com/c/1474091
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635164}
This CL makes the following changes:
Product code changes:
- Moving most of verification of CommonNavigationParams that used to be
done in RenderFrameHostImpl::BeginNavigation into a new helper
function in frame_host/ipc_utils.cc - VerifyCommonNavigationParams.
- Making sure that VerifyCommonNavigationParams verifies things that used
to be covered by RenderFrameHostImpl::BeginNavigation and *also*
covers verification of |initiator_origin| (reusing other verification
helpers present in frame_host/ipc_utils.cc).
- Auditing callers of SetRequestorOrigin and making sure they use the right
origin - they should use the initiating frame's origin. The following
functions have been tweaked:
1) blink::FrameLoader::ResourceRequestForReload
2) blink::HTMLAnchorElement::HandleClick
- Without tweak #1 some browser tests would have failed
(e.g. the existing DNSErrorPageTest.StaleCacheStatus or the new
ErrorPageNavigationReload_InSubframe_BlockedByClient in the
RenderFrameHostManagerTest suite).
- I also note that the tweaked line in
blink::FrameLoader::ResourceRequestForReload was added in r529246
which also added a regression test which still passes:
RenderFrameDevToolsAgentHostBrowserTest.ReloadDinoPage
Test changes:
- Moving IsolateOrigin[ForTesting] from security_exploit_browsertest.cc
into content/public/test/content_browser_test_utils.h
- Renaming DidCommitProvisionalLoadInterceptor into FrameHostInterceptor
and supporting intercepting one more method of FrameHost -
BeginNavigation (in addition to the old support for intercepting
DidCommitProvisionalLoad).
- Adding test coverage for terminating a renderer that provides an
invalid value in the |initiator_origin| argument of BeginNavigation.
- Adding test coverage for renderer-initiated reloading of an
erorred-out subframe.
Bug: 919144
Change-Id: I2de74a634e6691b490cd94df1ac8494248f7c363
Reviewed-on: https://chromium-review.googlesource.com/c/1455470
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634486}
This CL partially reverts r629679 to avoid incorrectly terminating
renderers in the wild - see the bugs listed below.
Bug: 918565, 931895
Change-Id: I6e0e7db87db4596c1f21520ceb942f5a34333e45
Reviewed-on: https://chromium-review.googlesource.com/c/1479077
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#633938}
This CL adds simple check for URLs reported during navigation commit
to ensure that the URL is allowed to commit in the process based on
ChildProcessSecurityPolicy::CanAccessDataForOrigin.
Bug: 918565
Change-Id: I7b24cbf160d5ae109e578177377db6a2f1ce667f
Reviewed-on: https://chromium-review.googlesource.com/c/1412689
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629679}
