Mojo Bindings: Support handles in native structs

Changes [Native] struct serialization to use an IPC::Message instead of a base::Pickle, allowing ParamTraits for [Native]-mapped types to be parameterized over IPC::Message once again. In order to support this, IPC::Message and related attachment support code has been moved into a separate leaf target in //ipc, avoiding circular dependencies with Mojo bindings. Also changes the wire representation of native structs to allow for typed Mojo handle attachments, and wires up native struct serialization to automatically convert between IPC::MessageAttachments and these typed mojom handles. The net result here is that [Native] mojom structs can be mapped to native types whose ParamTraits use message attachments. BUG=762025 Change-Id: Ib058eff2f32e0e7abfff9619da9f142113ad28ed Reviewed-on: https://chromium-review.googlesource.com/650219 Commit-Queue: Ken Rockot <rockot@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Yuzhu Shen <yzshen@chromium.org> Cr-Commit-Position: refs/heads/master@{#501866}
FairyWorld · Sep 14, 2017 · fd90763 · fd90763
1 parent b0ad6af
commit fd90763
Show file tree

Hide file tree

Showing 66 changed files with 1,035 additions and 654 deletions.
diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc
@@ -78,7 +78,7 @@
 #include "content/shell/browser/shell.h"
 #include "content/test/content_browser_test_utils_internal.h"
 #include "content/test/mock_overscroll_observer.h"
-#include "ipc/ipc.mojom.h"
+#include "ipc/constants.mojom.h"
 #include "ipc/ipc_security_test_util.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "net/dns/mock_host_resolver.h"

diff --git a/content/common/BUILD.gn b/content/common/BUILD.gn
@@ -651,7 +651,7 @@ mojom("mojo_bindings") {
   public_deps = [
     "//components/leveldb/public/interfaces",
     "//content/public/common:interfaces",
-    "//ipc:mojom",
+    "//ipc:mojom_constants",
     "//media/capture/mojo:capture_types",
     "//media/mojo/interfaces",
     "//mojo/common:common_custom_types",

diff --git a/content/common/renderer.mojom b/content/common/renderer.mojom
@@ -6,7 +6,7 @@ module content.mojom;
 
 import "content/common/native_types.mojom";
 import "content/common/service_worker/embedded_worker.mojom";
-import "ipc/ipc.mojom";
+import "ipc/constants.mojom";
 import "mojo/common/time.mojom";
 import "ui/gfx/geometry/mojo/geometry.mojom";
 import "ui/gfx/mojo/icc_profile.mojom";
@@ -171,7 +171,7 @@ interface Renderer {
   // EffectiveConnectionType is the connection type whose typical performance is
   // most similar to the measured performance of the network in use.
   // The downstream throughput is computed in kilobits per second. If an
-  // estimate of the HTTP or transport RTT is unavailable, it will be set to 
+  // estimate of the HTTP or transport RTT is unavailable, it will be set to
   // net::nqe::internal::InvalidRTT(). If the throughput estimate is
   // unavailable, it will be set to net::nqe::internal::kInvalidThroughput.
   OnNetworkQualityChanged(EffectiveConnectionType effective_connection_type,

diff --git a/ipc/BUILD.gn b/ipc/BUILD.gn
@@ -23,10 +23,6 @@ buildflag_header("ipc_features") {
 component("ipc") {
   sources = [
     "export_template.h",
-    "handle_attachment_win.cc",
-    "handle_attachment_win.h",
-    "handle_win.cc",
-    "handle_win.h",
     "ipc_channel.cc",
     "ipc_channel.h",
     "ipc_channel_common.cc",
@@ -45,12 +41,6 @@ component("ipc") {
     "ipc_listener.h",
     "ipc_logging.cc",
     "ipc_logging.h",
-    "ipc_message.cc",
-    "ipc_message.h",
-    "ipc_message_attachment.cc",
-    "ipc_message_attachment.h",
-    "ipc_message_attachment_set.cc",
-    "ipc_message_attachment_set.h",
     "ipc_message_macros.h",
     "ipc_message_pipe_reader.cc",
     "ipc_message_pipe_reader.h",
@@ -61,27 +51,13 @@ component("ipc") {
     "ipc_message_utils.h",
     "ipc_mojo_bootstrap.cc",
     "ipc_mojo_bootstrap.h",
-    "ipc_mojo_handle_attachment.cc",
-    "ipc_mojo_handle_attachment.h",
-    "ipc_mojo_message_helper.cc",
-    "ipc_mojo_message_helper.h",
     "ipc_mojo_param_traits.cc",
     "ipc_mojo_param_traits.h",
-    "ipc_platform_file.cc",
-    "ipc_platform_file.h",
-    "ipc_platform_file_attachment_posix.cc",
-    "ipc_platform_file_attachment_posix.h",
     "ipc_sender.h",
     "ipc_sync_channel.cc",
     "ipc_sync_channel.h",
-    "ipc_sync_message.cc",
-    "ipc_sync_message.h",
     "ipc_sync_message_filter.cc",
     "ipc_sync_message_filter.h",
-    "mach_port_attachment_mac.cc",
-    "mach_port_attachment_mac.h",
-    "mach_port_mac.cc",
-    "mach_port_mac.h",
     "message_filter.cc",
     "message_filter.h",
     "message_filter_router.cc",
@@ -105,20 +81,13 @@ component("ipc") {
     ]
   }
 
-  if (is_fuchsia) {
-    sources += [
-      "handle_attachment_fuchsia.cc",
-      "handle_attachment_fuchsia.h",
-      "handle_fuchsia.cc",
-      "handle_fuchsia.h",
-    ]
-  }
-
   defines = [ "IPC_IMPLEMENTATION" ]
 
   public_deps = [
     ":ipc_features",
+    ":message_support",
     ":mojom",
+    ":native_handle_type_converters",
     ":param_traits",
     "//mojo/public/cpp/bindings",
     "//mojo/public/cpp/system",
@@ -134,15 +103,76 @@ component("ipc") {
     "//base",
   ]
 
+  if (enable_ipc_fuzzer) {
+    public_configs = [ "//tools/ipc_fuzzer:ipc_fuzzer_config" ]
+  }
+}
+
+component("message_support") {
+  sources = [
+    "handle_attachment_win.cc",
+    "handle_attachment_win.h",
+    "handle_win.cc",
+    "handle_win.h",
+    "ipc_message.cc",
+    "ipc_message.h",
+    "ipc_message_attachment.cc",
+    "ipc_message_attachment.h",
+    "ipc_message_attachment_set.cc",
+    "ipc_message_attachment_set.h",
+    "ipc_message_support_export.h",
+    "ipc_mojo_handle_attachment.cc",
+    "ipc_mojo_handle_attachment.h",
+    "ipc_mojo_message_helper.cc",
+    "ipc_mojo_message_helper.h",
+    "ipc_platform_file.cc",
+    "ipc_platform_file.h",
+    "ipc_platform_file_attachment_posix.cc",
+    "ipc_platform_file_attachment_posix.h",
+    "ipc_sync_message.cc",
+    "ipc_sync_message.h",
+    "mach_port_attachment_mac.cc",
+    "mach_port_attachment_mac.h",
+    "mach_port_mac.cc",
+    "mach_port_mac.h",
+  ]
+
+  if (is_fuchsia) {
+    sources += [
+      "handle_attachment_fuchsia.cc",
+      "handle_attachment_fuchsia.h",
+      "handle_fuchsia.cc",
+      "handle_fuchsia.h",
+    ]
+  }
+
+  defines = [ "IPC_MESSAGE_SUPPORT_IMPL" ]
+
+  public_deps = [
+    ":ipc_features",
+    ":param_traits",
+    "//base",
+    "//mojo/public/cpp/system",
+  ]
+
   if (is_win || is_mac) {
     # On Windows HandleAttachmentWin needs to generate random IDs.
     # On Mac MachPortAttachmentMac needs to generate random IDs.
-    deps += [ "//crypto" ]
+    deps = [
+      "//crypto",
+    ]
   }
+}
 
-  if (enable_ipc_fuzzer) {
-    public_configs = [ "//tools/ipc_fuzzer:ipc_fuzzer_config" ]
-  }
+source_set("native_handle_type_converters") {
+  sources = [
+    "native_handle_type_converters.cc",
+    "native_handle_type_converters.h",
+  ]
+  public_deps = [
+    ":message_support",
+    "//mojo/public/interfaces/bindings:bindings_shared__generator",
+  ]
 }
 
 mojom_component("mojom") {
@@ -154,6 +184,16 @@ mojom_component("mojom") {
   public_deps = [
     "//mojo/common:read_only_buffer",
   ]
+
+  # Don't generate a variant sources since we depend on generated internal
+  # bindings types and we don't generate or build variants of those.
+  disable_variants = true
+}
+
+mojom("mojom_constants") {
+  sources = [
+    "constants.mojom",
+  ]
 }
 
 mojom("test_interfaces") {

diff --git a/ipc/OWNERS b/ipc/OWNERS
@@ -16,6 +16,8 @@ per-file *.mojom=set noparent
 per-file *.mojom=file://ipc/SECURITY_OWNERS
 per-file *_param_traits*.*=set noparent
 per-file *_param_traits*.*=file://ipc/SECURITY_OWNERS
+per-file *_type_converter*.*=set noparent
+per-file *_type_converter*.*=file://ipc/SECURITY_OWNERS
 
 per-file SECURITY_OWNERS=file://ipc/SECURITY_OWNERS
 

diff --git a/ipc/constants.mojom b/ipc/constants.mojom
@@ -0,0 +1,8 @@
+// Copyright 2017 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 IPC.mojom;
+
+// NOTE: This MUST match the value of MSG_ROUTING_NONE in src/ipc/ipc_message.h.
+const int32 kRoutingIdNone = -2;
diff --git a/ipc/handle_attachment_fuchsia.h b/ipc/handle_attachment_fuchsia.h
@@ -9,14 +9,15 @@
 
 #include "base/fuchsia/scoped_mx_handle.h"
 #include "ipc/handle_fuchsia.h"
-#include "ipc/ipc_export.h"
 #include "ipc/ipc_message_attachment.h"
+#include "ipc/ipc_message_support_export.h"
 
 namespace IPC {
 namespace internal {
 
 // This class represents a Fuchsia mx_handle_t attached to a Chrome IPC message.
-class IPC_EXPORT HandleAttachmentFuchsia : public MessageAttachment {
+class IPC_MESSAGE_SUPPORT_EXPORT HandleAttachmentFuchsia
+    : public MessageAttachment {
  public:
   // This constructor makes a copy of |handle| and takes ownership of the
   // result. Should only be called by the sender of a Chrome IPC message.

diff --git a/ipc/handle_attachment_win.h b/ipc/handle_attachment_win.h
@@ -9,14 +9,15 @@
 
 #include "base/win/scoped_handle.h"
 #include "ipc/handle_win.h"
-#include "ipc/ipc_export.h"
 #include "ipc/ipc_message_attachment.h"
+#include "ipc/ipc_message_support_export.h"
 
 namespace IPC {
 namespace internal {
 
 // This class represents a Windows HANDLE attached to a Chrome IPC message.
-class IPC_EXPORT HandleAttachmentWin : public MessageAttachment {
+class IPC_MESSAGE_SUPPORT_EXPORT HandleAttachmentWin
+    : public MessageAttachment {
  public:
   // This constructor makes a copy of |handle| and takes ownership of the
   // result. Should only be called by the sender of a Chrome IPC message.

diff --git a/ipc/handle_fuchsia.h b/ipc/handle_fuchsia.h
@@ -9,7 +9,7 @@
 
 #include <string>
 
-#include "ipc/ipc_export.h"
+#include "ipc/ipc_message_support_export.h"
 #include "ipc/ipc_param_traits.h"
 
 namespace base {
@@ -19,7 +19,7 @@ class PickleIterator;
 
 namespace IPC {
 
-class IPC_EXPORT HandleFuchsia {
+class IPC_MESSAGE_SUPPORT_EXPORT HandleFuchsia {
  public:
   // Default constructor makes an invalid mx_handle_t.
   HandleFuchsia();
@@ -33,7 +33,7 @@ class IPC_EXPORT HandleFuchsia {
 };
 
 template <>
-struct IPC_EXPORT ParamTraits<HandleFuchsia> {
+struct IPC_MESSAGE_SUPPORT_EXPORT ParamTraits<HandleFuchsia> {
   typedef HandleFuchsia param_type;
   static void Write(base::Pickle* m, const param_type& p);
   static bool Read(const base::Pickle* m,

diff --git a/ipc/handle_win.h b/ipc/handle_win.h
@@ -9,7 +9,7 @@
 
 #include <string>
 
-#include "ipc/ipc_export.h"
+#include "ipc/ipc_message_support_export.h"
 #include "ipc/ipc_param_traits.h"
 
 namespace base {
@@ -26,7 +26,7 @@ namespace IPC {
 // The ownership semantics for the underlying |handle_| are complex. See
 // ipc/mach_port_mac.h (the OSX analog of this class) for an extensive
 // discussion.
-class IPC_EXPORT HandleWin {
+class IPC_MESSAGE_SUPPORT_EXPORT HandleWin {
  public:
   // Default constructor makes an invalid HANDLE.
   HandleWin();
@@ -40,7 +40,7 @@ class IPC_EXPORT HandleWin {
 };
 
 template <>
-struct IPC_EXPORT ParamTraits<HandleWin> {
+struct IPC_MESSAGE_SUPPORT_EXPORT ParamTraits<HandleWin> {
   typedef HandleWin param_type;
   static void Write(base::Pickle* m, const param_type& p);
   static bool Read(const base::Pickle* m,

diff --git a/ipc/ipc.mojom b/ipc/ipc.mojom
@@ -5,23 +5,7 @@
 module IPC.mojom;
 
 import "mojo/common/read_only_buffer.mojom";
-
-// NOTE: This MUST match the value of MSG_ROUTING_NONE in src/ipc/ipc_message.h.
-const int32 kRoutingIdNone = -2;
-
-struct SerializedHandle {
-  handle the_handle;
-
-  enum Type {
-    MOJO_HANDLE,
-    PLATFORM_FILE,
-    WIN_HANDLE,
-    MACH_PORT,
-    FUCHSIA_HANDLE,
-  };
-
-  Type type;
-};
+import "mojo/public/interfaces/bindings/native_struct.mojom";
 
 // A placeholder interface type since we don't yet support generic associated
 // message pipe handles.
@@ -34,7 +18,7 @@ interface Channel {
 
   // Transmits a classical Chrome IPC message.
   Receive(mojo.common.mojom.ReadOnlyBuffer data,
-          array<SerializedHandle>? handles);
+          array<mojo.native.SerializedHandle>? handles);
 
   // Requests a Channel-associated interface.
   GetAssociatedInterface(string name, associated GenericInterface& request);