Change how 3D API blocking is implemented

This moves the query for 3D API blocking policy back to the IO thread with the document URL provided by the renderer again. The IO thread is used to reduce sync IPC hangs in renderers using WebGL, and the origin is trusted from the renderer (a) to avoid racy coordination between IO and UI thread and (b) because this is not a security-sensitive decision. Fixed: 1119941 Change-Id: Ica47eb137c1e28a7e754b753f2356266e8b9d659 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2380345 Commit-Queue: Aaron Colwell <[email protected]> Auto-Submit: Ken Rockot <[email protected]> Reviewed-by: Aaron Colwell <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Reviewed-by: Kenneth Russell <[email protected]> Cr-Commit-Position: refs/heads/master@{#804339} GitOrigin-RevId: 2895839005437f978123678fea27b806d17c4c08
mjfroman · Sep 3, 2020 · 2b1731c · 2b1731c
1 parent 0eb44eb
commit 2b1731c
Show file tree

Hide file tree

Showing 9 changed files with 56 additions and 15 deletions.
diff --git a/blink/public/mojom/BUILD.gn b/blink/public/mojom/BUILD.gn
@@ -222,6 +222,7 @@ mojom("mojom_platform") {
     "//skia/public/mojom",
     "//third_party/blink/public/mojom/dom_storage",
     "//third_party/blink/public/mojom/frame",
+    "//third_party/blink/public/mojom/gpu",
     "//third_party/blink/public/mojom/tokens",
     "//third_party/blink/public/mojom/usb",
     "//ui/base/cursor/mojom",

diff --git a/blink/public/mojom/frame/frame.mojom b/blink/public/mojom/frame/frame.mojom
@@ -353,10 +353,6 @@ interface LocalFrameHost {
   [Sync]
   RunBeforeUnloadConfirm(bool is_reload) => (bool success);
 
-  // A request to check whether WebGL is explicitly blocked.
-  [Sync]
-  Are3DAPIsBlocked() => (bool blocked);
-
   // Notifies that the urls for the favicon of a site has been determined.
   UpdateFaviconURL(array<FaviconURL> favicon_urls);
 

diff --git a/blink/public/mojom/gpu/BUILD.gn b/blink/public/mojom/gpu/BUILD.gn
@@ -0,0 +1,21 @@
+# Copyright 2020 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import("//mojo/public/tools/bindings/mojom.gni")
+
+mojom_component("gpu") {
+  output_prefix = "blink_gpu_mojom"
+  macro_prefix = "BLINK_GPU_MOJOM"
+
+  sources = [ "gpu.mojom" ]
+  public_deps = [ "//url/mojom:url_mojom_gurl" ]
+
+  # It's important to specify these settings for Blink bindings, because the
+  # URL dependency above requires direct linkage into the platform library.
+  export_class_attribute_blink = "PLATFORM_EXPORT"
+  export_define_blink = "BLINK_PLATFORM_IMPLEMENTATION=1"
+  export_header_blink = "third_party/blink/renderer/platform/platform_export.h"
+
+  generate_java = true
+}
diff --git a/blink/public/mojom/gpu/OWNERS b/blink/public/mojom/gpu/OWNERS
@@ -0,0 +1,2 @@
+per-file *.mojom=set noparent
+per-file *.mojom=file://ipc/SECURITY_OWNERS
diff --git a/blink/public/mojom/gpu/gpu.mojom b/blink/public/mojom/gpu/gpu.mojom
@@ -0,0 +1,18 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+module blink.mojom;
+
+import "url/mojom/url.mojom";
+
+// An interface used by frames to query the browser about tracked GPU state.
+interface GpuDataManager {
+  // Asks whether or not `url` is allowed to use 3D APIs.
+  //
+  // Note that the renderer can ignore the result of this check, so this is
+  // currently not considered security-sensitive. As a temporary exception, pass
+  // the URL rather than routing this method via the frame.
+  [Sync]
+  Are3DAPIsBlockedForUrl(url.mojom.Url url) => (bool blocked);
+};
diff --git a/blink/renderer/core/html/canvas/html_canvas_element.cc b/blink/renderer/core/html/canvas/html_canvas_element.cc
@@ -44,6 +44,9 @@
 #include "third_party/blink/public/common/privacy_budget/identifiability_metrics.h"
 #include "third_party/blink/public/common/privacy_budget/identifiability_study_participation.h"
 #include "third_party/blink/public/common/privacy_budget/identifiable_surface.h"
+#include "third_party/blink/public/common/thread_safe_browser_interface_broker_proxy.h"
+#include "third_party/blink/public/mojom/gpu/gpu.mojom-blink.h"
+#include "third_party/blink/public/platform/platform.h"
 #include "third_party/blink/public/platform/task_type.h"
 #include "third_party/blink/public/resources/grit/blink_image_resources.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_controller.h"
@@ -457,12 +460,11 @@ bool HTMLCanvasElement::IsWebGL2Enabled() const {
 
 bool HTMLCanvasElement::IsWebGLBlocked() const {
   Document& document = GetDocument();
-  LocalFrame* frame = document.GetFrame();
-  if (!frame)
-    return false;
-
   bool blocked = false;
-  frame->GetLocalFrameHostRemote().Are3DAPIsBlocked(&blocked);
+  mojo::Remote<mojom::blink::GpuDataManager> gpu_data_manager;
+  Platform::Current()->GetBrowserInterfaceBroker()->GetInterface(
+      gpu_data_manager.BindNewPipeAndPassReceiver());
+  gpu_data_manager->Are3DAPIsBlockedForUrl(document.Url(), &blocked);
   return blocked;
 }
 

diff --git a/blink/renderer/core/testing/fake_local_frame_host.cc b/blink/renderer/core/testing/fake_local_frame_host.cc
@@ -152,10 +152,6 @@ void FakeLocalFrameHost::RunBeforeUnloadConfirm(
   std::move(callback).Run(true);
 }
 
-void FakeLocalFrameHost::Are3DAPIsBlocked(Are3DAPIsBlockedCallback callback) {
-  std::move(callback).Run(true);
-}
-
 void FakeLocalFrameHost::UpdateFaviconURL(
     WTF::Vector<blink::mojom::blink::FaviconURLPtr> favicon_urls) {}
 

diff --git a/blink/renderer/core/testing/fake_local_frame_host.h b/blink/renderer/core/testing/fake_local_frame_host.h
@@ -93,7 +93,6 @@ class FakeLocalFrameHost : public mojom::blink::LocalFrameHost {
                             RunModalPromptDialogCallback callback) override;
   void RunBeforeUnloadConfirm(bool is_reload,
                               RunBeforeUnloadConfirmCallback callback) override;
-  void Are3DAPIsBlocked(Are3DAPIsBlockedCallback callback) override;
   void UpdateFaviconURL(
       WTF::Vector<blink::mojom::blink::FaviconURLPtr> favicon_urls) override;
   void DownloadURL(mojom::blink::DownloadURLParamsPtr params) override;

diff --git a/blink/renderer/modules/webgl/webgl_rendering_context_base.cc b/blink/renderer/modules/webgl/webgl_rendering_context_base.cc
@@ -39,6 +39,8 @@
 #include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/common/privacy_budget/identifiability_metric_builder.h"
 #include "third_party/blink/public/common/privacy_budget/identifiability_study_participation.h"
+#include "third_party/blink/public/common/thread_safe_browser_interface_broker_proxy.h"
+#include "third_party/blink/public/mojom/gpu/gpu.mojom-blink.h"
 #include "third_party/blink/public/platform/platform.h"
 #include "third_party/blink/public/platform/task_type.h"
 #include "third_party/blink/renderer/bindings/modules/v8/html_canvas_element_or_offscreen_canvas.h"
@@ -8221,7 +8223,11 @@ void WebGLRenderingContextBase::MaybeRestoreContext(TimerBase*) {
       return;
 
     bool blocked = false;
-    frame->GetLocalFrameHostRemote().Are3DAPIsBlocked(&blocked);
+    mojo::Remote<mojom::blink::GpuDataManager> gpu_data_manager;
+    Platform::Current()->GetBrowserInterfaceBroker()->GetInterface(
+        gpu_data_manager.BindNewPipeAndPassReceiver());
+    gpu_data_manager->Are3DAPIsBlockedForUrl(canvas()->GetDocument().Url(),
+                                             &blocked);
     if (blocked)
       return;
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		per-file *.mojom=set noparent
		per-file *.mojom=file://ipc/SECURITY_OWNERS