Skip to content

Commit

Permalink
Add ScopedAllowSyncCall to AndroidOverlay.
Browse files Browse the repository at this point in the history
Bug: 1181378
Change-Id: Id6b5ad5f5a94cdc26e95b544aeb9fb490142e401
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2720010
Auto-Submit: Frank Liberato <liberato@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#858427}
  • Loading branch information
liberato-at-chromium authored and Chromium LUCI CQ committed Feb 28, 2021
1 parent d2144eb commit 13bde9e
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
16 changes: 15 additions & 1 deletion content/browser/android/dialog_overlay_impl.cc
Expand Up @@ -13,6 +13,7 @@
#include "content/public/browser/web_contents_delegate.h"
#include "gpu/ipc/common/gpu_surface_tracker.h"
#include "media/mojo/mojom/android_overlay.mojom.h"
#include "mojo/public/cpp/bindings/sync_call_restrictions.h"
#include "ui/android/view_android_observer.h"
#include "ui/android/window_android.h"

Expand Down Expand Up @@ -253,6 +254,17 @@ void DialogOverlayImpl::RegisterWindowObserverIfNeeded(
}
}

// Helper class that has permission to talk to SyncCallRestrictions. Rather
// than friend the function directly, which has an odd signature, friend a class
// that knows how to do the work.
class AndroidOverlaySyncHelper {
public:
static void MakeSyncCall(media::mojom::AndroidOverlayClient* remote) {
mojo::SyncCallRestrictions::ScopedAllowSyncCall scoped_allow;
remote->OnSynchronouslyDestroyed();
}
};

static void JNI_DialogOverlayImpl_NotifyDestroyedSynchronously(
JNIEnv* env,
int message_pipe_handle) {
Expand All @@ -262,9 +274,11 @@ static void JNI_DialogOverlayImpl_NotifyDestroyedSynchronously(
mojo::PendingRemote<media::mojom::AndroidOverlayClient>(
std::move(scoped_handle),
media::mojom::AndroidOverlayClient::Version_));
// This prevents crashes, though it's unclear how we'd have a null remote.
// https://crbug.com/1155313 .
if (!remote.is_bound())
return;
remote->OnSynchronouslyDestroyed();
AndroidOverlaySyncHelper::MakeSyncCall(remote.get());
// Note that we don't take back the mojo message pipe. We let it close when
// `remote` goes out of scope.
}
Expand Down
4 changes: 4 additions & 0 deletions mojo/public/cpp/bindings/sync_call_restrictions.h
Expand Up @@ -21,6 +21,7 @@ class CastCdmOriginProvider;

namespace content {
class DesktopCapturerLacros;
class AndroidOverlaySyncHelper;
} // namespace content

namespace crosapi {
Expand Down Expand Up @@ -91,6 +92,9 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) SyncCallRestrictions {
// TODO(159346933) Remove once the origin isolation logic is moved outside of
// cast media service.
friend class chromecast::CastCdmOriginProvider;
// Android requires synchronous processing when overlay surfaces are
// destroyed, else behavior is undefined.
friend class content::AndroidOverlaySyncHelper;
// END ALLOWED USAGE.

#if ENABLE_SYNC_CALL_RESTRICTIONS
Expand Down

0 comments on commit 13bde9e

Please sign in to comment.