NOTREACHED() and NOTREACHED_IN_MIGRATION() are both CHECK-fatal now.
The former is [[noreturn]] so this CL also performs dead-code removal
after the NOTREACHED().
This CL does not attempt to do additional rewrites of any surrounding
code, like:
if (!foo) {
NOTREACHED();
}
to CHECK(foo);
Those transforms take a non-trivial amount of time (and there are
thousands of instances). Cleanup can be left as an exercise for the
reader.
This does clean up kCrashOnDanglingBrowserContext as both paths of the
kill switch are currently fatal. This has been rolled out for a long
time.
Bug: 40580068, 40062641
Change-Id: Ib88e710d003e2e48df3fc502ca54d2341d157a0e
Cq-Include-Trybots: luci.chromium.try:linux-dcheck-off-rel
Low-Coverage-Reason: OTHER Should-be-unreachable code
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5974816
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Auto-Submit: Peter Boström <pbos@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1376522}
This s/NOTREACHED()/NOTREACHED_IN_MIGRATION() for most of remaining
src/. Then I went through all changes and made sure that we didn't do
this for comments or "NOTREACHED(): " log strings.
Rolling in the corresponding change for remaining third_party/crashpad
is separately up for review.
Bug: 40580068
Low-Coverage-Reason: TRIVIAL_CHANGE Unreachable code is not reachable.
Change-Id: I707e582b710bf65474db2e3477fa06f5b8d8ea06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5545807
Owners-Override: Lei Zhang <thestig@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1302866}
First, this patch moves the capability control related test utilities
into content/public/test so that the new test in chrome/ can reuse it.
Then, it adds a browser test for LinkPreview to check basic capability
control behaviors on kDefer and kGrant policies.
Bug: b/299240273
Change-Id: I0283a2840e7b13c33ad7d7cfecc4d591812fd009
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5017840
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Ken Okada <kenoss@chromium.org>
Auto-Submit: Takashi Toyoshima <toyoshim@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1223445}
Mojo message calls
This CL adds a DCHECK into MojoBinderPolicyApplier::GrantAll() to ensure
this method is called within Mojo message call stacks.
We should guarantee it because MojoBinderPolicyApplier::GrantAll()
invokes BrowserInterfaceBrokerImpl::BindInterface(), and the
BindInterface() method may call mojo::ReportBadMessage(), which should
be called within a stack frame of a Mojo message dispatch.
(Now mojom::LocalFrameHost::DidActivateForPrerendering() invokes
MojoBinderPolicyApplier::GrantAll(). See the issue description for more
information.)
Bug: 1217977
Change-Id: Ib63811a753243756b778b1a1f02f5aa616dedeba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2952745
Commit-Queue: Lingqi Chi <lingqi@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#892439}
prerenderingchange"
This relands commit Ib3da39f9cae6b1a46d08a64d8f41b5f5b50460c0.
The added test failed on the LaCrOS builds because of the use of the
Gamepad API. This CL relands the test except for the LaCrOS builds.
Also this adds a new test using the NotificationService interface to
keep the test coverage on the LaCrOS builds.
Original change's description:
> For prerendering, MojoBinderPolicyMapApplier wants to loosen the
> kCancel policy slightly before the activation is complete.
>
> The activation of a prerendered page follows the steps below.
> 1. The activation navigation to the prerendered page is committed.
> 2. PrerenderHostRegistry knows that it can activate the prerendered
> page.
> 3. It informs every RenderFrameHostImpl(RFHI) of activation, and they
> will inform the corresponding documents on renderer sides.
> 4. These documents update their prerendering states and dispatch
> prerenderingchange events.
> 5. These documents inform the browser process that they finished
> activating.
> 6. The browser process releases binder policies. This should be done
> after step 5 because the renderer isn't meant to get these deferred
> messages before the prerenderingchange event is diepatched. But these
> Mojo message pipes are not channel-associated, once we grant those
> deferred binders and the interfaces are bound to the renderer, it
> might receives some deferred messages before CommitActivation arrives.
>
> The prerendered page is allowed to access kCancel and kUnexpected APIs
> in the prerenderingchange event, so RFHI may receive kCancel or
> kUnexpected binding requests between 4 and 6, which means we should
> loosen the Mojo capability control before firing this event.
>
> The decision of when to loose control has not been finalized, but at
> this point we can loosen the kCancel and kUnexpected polices in
> RenderFrameHostImpl::ActivateForPrerendering() for starting early
> testing.
>
> This CL implements this logic and adds a browser test for the
> GamepadMonitor interface, whose policy is kCancel, to ensure the
> change works.
>
> Bug: 1132752, 1186796
> Change-Id: Ib3da39f9cae6b1a46d08a64d8f41b5f5b50460c0
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2810422
> Commit-Queue: Lingqi Chi <lingqi@chromium.org>
> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
> Reviewed-by: Matt Falkenhagen <falken@chromium.org>
> Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#874138}
Bug: 1132752, 1186796, 1198110
Change-Id: I8be5c672e7a81db261329bc807146c9d13f92bc9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2842625
Commit-Queue: Lingqi Chi <lingqi@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#875615}
This reverts commit a728ed6052.
Reason for revert:
All/PrerenderBrowserTest.GamepadMonitorAfterNavigation/WebContents
All/PrerenderBrowserTest.GamepadMonitorAfterNavigation/MPArch
are failing on linux-lacros-tester-rel
Example: https://ci.chromium.org/ui/p/chromium/builders/ci/linux-lacros-tester-rel/9034/overview
Original change's description:
> Prerender: Loosen Mojo capability control before firing
> prerenderingchange
>
> For prerendering, MojoBinderPolicyMapApplier wants to loosen the kCancel
> policy slightly before the activation is complete.
>
> The activation of a prerendered page follows the steps below.
> 1. The activation navigation to the prerendered page is committed.
> 2. PrerenderHostRegistry knows that it can activate the prerendered page.
> 3. It informs every RenderFrameHostImpl(RFHI) of activation, and they
> will inform the corresponding documents on renderer sides.
> 4. These documents update their prerendering states and dispatch
> prerenderingchange events.
> 5. These documents inform the browser process that they finished
> activating.
> 6. The browser process releases binder policies. This should be done
> after step 5 because the renderer isn't meant to get these deferred
> messages before the prerenderingchange event is diepatched. But these
> Mojo message pipes are not channel-associated, once we grant those
> deferred binders and the interfaces are bound to the renderer, it might
> receives some deferred messages before CommitActivation arrives.
>
> The prerendered page is allowed to access kCancel and kUnexpected APIs
> in the prerenderingchange event, so RFHI may receive kCancel or
> kUnexpected binding requests between 4 and 6, which means we should
> loosen the Mojo capability control before firing this event.
>
> The decision of when to loose control has not been finalized, but at
> this point we can loosen the kCancel and kUnexpected polices in
> RenderFrameHostImpl::ActivateForPrerendering() for starting early
> testing.
>
> This CL implements this logic and adds a browser test for the
> GamepadMonitor interface, whose policy is kCancel, to ensure the change
> works.
>
> Bug: 1132752, 1186796
> Change-Id: Ib3da39f9cae6b1a46d08a64d8f41b5f5b50460c0
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2810422
> Commit-Queue: Lingqi Chi <lingqi@chromium.org>
> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
> Reviewed-by: Matt Falkenhagen <falken@chromium.org>
> Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#874138}
Bug: 1132752
Bug: 1186796
Change-Id: Icb14aa78708d99d93079ee29bfede536432d2caf
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2837979
Auto-Submit: Christos Froussios <cfroussios@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Owners-Override: Christos Froussios <cfroussios@chromium.org>
Cr-Commit-Position: refs/heads/master@{#874205}
prerenderingchange
For prerendering, MojoBinderPolicyMapApplier wants to loosen the kCancel
policy slightly before the activation is complete.
The activation of a prerendered page follows the steps below.
1. The activation navigation to the prerendered page is committed.
2. PrerenderHostRegistry knows that it can activate the prerendered page.
3. It informs every RenderFrameHostImpl(RFHI) of activation, and they
will inform the corresponding documents on renderer sides.
4. These documents update their prerendering states and dispatch
prerenderingchange events.
5. These documents inform the browser process that they finished
activating.
6. The browser process releases binder policies. This should be done
after step 5 because the renderer isn't meant to get these deferred
messages before the prerenderingchange event is diepatched. But these
Mojo message pipes are not channel-associated, once we grant those
deferred binders and the interfaces are bound to the renderer, it might
receives some deferred messages before CommitActivation arrives.
The prerendered page is allowed to access kCancel and kUnexpected APIs
in the prerenderingchange event, so RFHI may receive kCancel or
kUnexpected binding requests between 4 and 6, which means we should
loosen the Mojo capability control before firing this event.
The decision of when to loose control has not been finalized, but at
this point we can loosen the kCancel and kUnexpected polices in
RenderFrameHostImpl::ActivateForPrerendering() for starting early
testing.
This CL implements this logic and adds a browser test for the
GamepadMonitor interface, whose policy is kCancel, to ensure the change
works.
Bug: 1132752, 1186796
Change-Id: Ib3da39f9cae6b1a46d08a64d8f41b5f5b50460c0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2810422
Commit-Queue: Lingqi Chi <lingqi@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#874138}
This CL includes the following changes.
- Updates the final state of PrerenderHost when MojoBinderPolicyApplier
cancels prerendering.
- Records the interface that causes the cancellation and reports it to
UMA.
Bug: 1132752, 1126305
Change-Id: Ibc156d588424917c2a8f05478e386b272c6cffe8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2796850
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Commit-Queue: Lingqi Chi <lingqi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#870322}
For kUnexpected interface binders, we want to understand the situation
by checking crash reports.
When receiving an unexpected binding request, MojoBinderPolicyApplier
- Reports a bad mojom message to crash the corresponding renderer.
- Cancels the prerender.
Bug: 1141364
Change-Id: Ia332ccb14b723ce160c6514a7c8128e96c466c7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2784660
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Lingqi Chi <lingqi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#867577}
This CL allows Prerender2 to partially loosen its binding policies by
switching MojoBinderPolicyApplier(MBPA) into the kPrepareToGrantAll
mode.
The Prerender activation flow requires Mojo capability control to
loosen kCancel and kUnexpected policies before Blink dispatches the
`prerenderingchange` event.
(https://docs.google.com/document/d/1_l1LDUALf8PbZIz5y_UYNGZmYCMKNpePDLBHn-gTsTM/)
Mojo capability control controls when and whether to bind a interface
with MBPA. Before this CL, MBPA only runs in two modes, one applies
pre-set policies strictly, and will cancel the current prerender if it
receives a kCancel/kUnexpected interface; the other ignores these
policies and runs all binders immediately.
To meet the new requirement, this CL introduces a new
mode, kPrepareToGrantAll. When running in this mode, MBPA only defers
non-kGrant binding requests but not cancels prerendering.
Bug: 1132752
Change-Id: Ie93eb7aba458f3e687649d6c3859feb44ccdfe20
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2726016
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Commit-Queue: Lingqi Chi <lingqi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#863697}
This CL delays the timing to release the mojo capability control in the
browser process. Releasing the mojo capability control corresponds to
running the "post-prerendering activation steps list" defined in the
draft spec:
https://jeremyroman.github.io/alternate-loading-modes/#document-post-prerendering-activation-steps-list
This change makes it possible to write tests for deferred features using
the web-exposed features (i.e., `document.prerendering` and the
`prerenderingchange` event)
> Details
Before this CL, the mojo capability control was released in the browser
process before the prerenderingchange event was dispatched in the Blink.
This violated the spec steps in the Activate algorithm. The step 3 and
step 4 could be reversed.
https://jeremyroman.github.io/alternate-loading-modes/#prerendering-browsing-context-activate
1. Set bc's loading mode to "default".
2. Let doc be bc’s active document.
3. Fire an event named prerenderingchange at doc.
4. For each steps in doc's post-prerendering activation steps list:
1. Run steps.
After this CL, to obey the spec steps, the mojo capability control is
released after the browser process (RenderFrameHostImpl) receives a
DidActivateForPrerendering message sent by the Blink (LocalFrame) after
the prerenderingchange event is dispatched.
Design Doc:
https://docs.google.com/document/d/1_l1LDUALf8PbZIz5y_UYNGZmYCMKNpePDLBHn-gTsTM/edit?usp=sharing
> Future plans
This causes unfavorable period between when RenderFrameHostImpl gets
activated and when RenderFrameHostImpl receives the
DidActivateForPrerendering message. The BrowserInterfaceBrokerImpl may
receive a mojo request that causes cancellation of prerendering during
the period, while the frame host already gets activated. This will be
supported by introducing the "mode" of the BrowserInterfaceBrokerImpl
in follow-up CLs. See the discussion in the design doc for details.
Change-Id: I52c33d5350733cfb8d990312d665e36d97970fec
Bug: 1183320, 1186796
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2738911
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Carlos Caballero <carlscab@google.com>
Reviewed-by: Lingqi Chi <lingqi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#863673}
RenderFrameHostImpl
After https://chromium-review.googlesource.com/c/chromium/src/+/2689551,
RenderFrameHostImpl(RFHI) can access the correct prerendering state in
its constructor. So RFHI can decide whether to enable Mojo capability
control here according to the prerendering state.
This CL sets MojoBinderPolicyApplier in RFHI::ctor. It brings the
following benefits:
1. In prerendering pages, we can ensure that
BrowserInterfaceBrokerImpl(BIB) receives interface binders after
RFHI sets MojoBinderPolicyApplier, i.e., Mojo capability control is
enabled before BIB receives the first binding request.
2. It unifies the logic of setting MojoBinderPolicyApplier; RFHI sets it
in three different methods before this CL.
3. It guarantees every RenderFrameHostImpl in the prerendering frame
tree enables Mojo capability control. Before this CL, activating the
prerendering page could happen before RFHI::CommitNavigation(), which
fails `CHECK(policy_applier_);` in ReleaseMojoBinderPolicies().
Bug: 1132752, 1160611
Change-Id: Ie1bb0366fc01d3018ad7e6b52cff31152d000f78
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2738156
Commit-Queue: Lingqi Chi <lingqi@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#861494}
Some new features, such as prerendering, need to defer some binding
interface requests or stop the renderer when an unexpected mojo pipe is
requested to set up.
This CL introduces MojoBinderPolicyApplier that manages binding requests
and integrates it with BrowserInterfaceBroker.
Regarding the MojoBinderPolicyApplier, it should be initialized with a
predefined policy map. It decide the actions the applier takes when
handling incoming requests. Based on the predefined policies and the
current state, a MojoBinderPolicyApplier instance may take the following
actions:
1. Bind the given interface immediately. (kGrant or
`ResolveDeferredBinders` has been invoked)
2. Delay binding the interface until `ResolveDeferredBinders` is called.
(kDefer)
3. Call the cancelling closure. (kCancel)
4. Stop the renderer and collect some metrics for analysis. (kUnexpected)
Bug: 1132752
Change-Id: Ifdc2f0b27d2f4613274fe5c6e44ddb056a199509
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2513960
Commit-Queue: Lingqi Chi <lingqi@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828609}
