New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 620733 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Need to disable sync mojo calls from the browser process

Project Member Reported by jam@chromium.org, Jun 16 2016

Issue description

For performance and stability (avoiding nested message loops) reasons, we don't allow sync IPCs from the browser process with legacy IPCs. We need to be able to configure the mojo bindings process-wide in the browser process to maintain this behavior.

Marking this as P1 because we don't want to inadvertently let in any sync calls in the meantime.
 

Comment 1 by yzshen@chromium.org, Jun 16 2016

Cc: -yzshen@chromium.org
Owner: yzshen@chromium.org

Comment 2 by jam@chromium.org, Jun 16 2016

Ken points out that the GPU code does sync IPCs from the browser.

We chatted with Yuzhu and came up with:
1) disable it process wide
2) have something like base::ScopedAllowWait to enable it. Like the base class, the new one should be in a private section of a class and we should use a small list of OWNERS files to ensure that we have careful review of any sync IPCs from the browser.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/519639829bd17eaae4b147f7a96cbd1406fc6244

commit 519639829bd17eaae4b147f7a96cbd1406fc6244
Author: yzshen <yzshen@chromium.org>
Date: Tue Jun 21 19:37:10 2016

Mojo: add support for disallowing sync calls for a process.

This CLs introduce a new system API to expose the configuration whether sync calls are allowed to mojo apps.
The bindings check this configuration before processing a sync call (if DCHECK is enabled).

This CL also disables sync calls for the browser process.

BUG= 620733 

Review-Url: https://codereview.chromium.org/2084993002
Cr-Commit-Position: refs/heads/master@{#401076}

[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/base/threading/thread_restrictions.h
[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/components/mus/common/gpu_service.cc
[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/content/browser/browser_main_loop.cc
[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/mojo/edk/embedder/embedder.cc
[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/mojo/edk/embedder/embedder.h
[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/mojo/edk/embedder/entrypoints.cc
[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/mojo/edk/system/core.cc
[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/mojo/edk/system/core.h
[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/mojo/mojo_public.gypi
[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/mojo/public/c/system/functions.h
[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/mojo/public/c/system/thunks.cc
[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/mojo/public/c/system/thunks.h
[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/mojo/public/c/system/types.h
[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/mojo/public/cpp/bindings/BUILD.gn
[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
[modify] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/mojo/public/cpp/bindings/lib/router.cc
[add] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/mojo/public/cpp/bindings/lib/sync_call_restrictions.cc
[add] https://crrev.com/519639829bd17eaae4b147f7a96cbd1406fc6244/mojo/public/cpp/bindings/sync_call_restrictions.h

Comment 4 by yzshen@chromium.org, Jun 22 2016

Status: Fixed (was: Available)

Sign in to add a comment