Mojo C++ bindings: support sync methods - part 1.

This CL:
- adds support for [Sync] attribute;
- generates C++ sync method signatures;
- does the simplest thing to block and wait for results of sync calls. (Allows
  any messages of the same interface ptr to re-enter; disallows any messages of
  other message pipes to re-enter.)

The following will be in future CLs:
- Change the re-entrancy behavior.
- Support sync calls with associated interfaces.

BUG=577699

Review URL: https://codereview.chromium.org/1701423002

Cr-Commit-Position: refs/heads/master@{#376023}


CrOS-Libchrome-Original-Commit: 904b45f35fb925bab31d25e43732a947385460b6
diff --git a/mojo/mojo_edk_tests.gyp b/mojo/mojo_edk_tests.gyp
index 8879eb4..71489d6 100644
--- a/mojo/mojo_edk_tests.gyp
+++ b/mojo/mojo_edk_tests.gyp
@@ -94,6 +94,7 @@
         'public/cpp/bindings/tests/string_unittest.cc',
         'public/cpp/bindings/tests/struct_traits_unittest.cc',
         'public/cpp/bindings/tests/struct_unittest.cc',
+        'public/cpp/bindings/tests/sync_method_unittest.cc',
         'public/cpp/bindings/tests/type_conversion_unittest.cc',
         'public/cpp/bindings/tests/union_unittest.cc',
         'public/cpp/bindings/tests/validation_unittest.cc',
diff --git a/mojo/mojo_public.gyp b/mojo/mojo_public.gyp
index 69521ab..82cb600 100644
--- a/mojo/mojo_public.gyp
+++ b/mojo/mojo_public.gyp
@@ -366,6 +366,7 @@
           'public/interfaces/bindings/tests/test_constants.mojom',
           'public/interfaces/bindings/tests/test_native_types.mojom',
           'public/interfaces/bindings/tests/test_structs.mojom',
+          'public/interfaces/bindings/tests/test_sync_methods.mojom',
           'public/interfaces/bindings/tests/test_unions.mojom',
           'public/interfaces/bindings/tests/validation_test_interfaces.mojom',
         ],
diff --git a/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc b/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
index 79c25b5..0063bff 100644
--- a/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
+++ b/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
@@ -161,6 +161,13 @@
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(message->has_flag(kMessageExpectsResponse));
 
+  // TODO(yzshen): Sync method call using assoicated interfaces or master
+  // interfaces that serve associated interfaces hasn't been supported yet.
+  if (message->has_flag(kMessageIsSync)) {
+    NOTIMPLEMENTED();
+    return false;
+  }
+
   if (encountered_error_)
     return false;
 
diff --git a/mojo/public/cpp/bindings/lib/message_builder.h b/mojo/public/cpp/bindings/lib/message_builder.h
index a9c5330..e728492 100644
--- a/mojo/public/cpp/bindings/lib/message_builder.h
+++ b/mojo/public/cpp/bindings/lib/message_builder.h
@@ -48,6 +48,14 @@
                                     payload_size,
                                     kMessageExpectsResponse,
                                     0) {}
+
+  RequestMessageBuilder(uint32_t name,
+                        size_t payload_size,
+                        uint32_t extra_flags)
+      : MessageWithRequestIDBuilder(name,
+                                    payload_size,
+                                    kMessageExpectsResponse | extra_flags,
+                                    0) {}
 };
 
 class ResponseMessageBuilder : public MessageWithRequestIDBuilder {
@@ -59,6 +67,15 @@
                                     payload_size,
                                     kMessageIsResponse,
                                     request_id) {}
+
+  ResponseMessageBuilder(uint32_t name,
+                         size_t payload_size,
+                         uint64_t request_id,
+                         uint32_t extra_flags)
+      : MessageWithRequestIDBuilder(name,
+                                    payload_size,
+                                    kMessageIsResponse | extra_flags,
+                                    request_id) {}
 };
 
 }  // namespace internal
diff --git a/mojo/public/cpp/bindings/lib/message_internal.h b/mojo/public/cpp/bindings/lib/message_internal.h
index 53e8aae..b4fd334 100644
--- a/mojo/public/cpp/bindings/lib/message_internal.h
+++ b/mojo/public/cpp/bindings/lib/message_internal.h
@@ -14,7 +14,11 @@
 
 #pragma pack(push, 1)
 
-enum { kMessageExpectsResponse = 1 << 0, kMessageIsResponse = 1 << 1 };
+enum {
+  kMessageExpectsResponse = 1 << 0,
+  kMessageIsResponse = 1 << 1,
+  kMessageIsSync = 1 << 2
+};
 
 struct MessageHeader : internal::StructHeader {
   // Interface ID for identifying multiple interfaces running on the same
diff --git a/mojo/public/cpp/bindings/lib/router.cc b/mojo/public/cpp/bindings/lib/router.cc
index 44244aa..9bc9aa1 100644
--- a/mojo/public/cpp/bindings/lib/router.cc
+++ b/mojo/public/cpp/bindings/lib/router.cc
@@ -18,18 +18,17 @@
 
 class ResponderThunk : public MessageReceiverWithStatus {
  public:
-  explicit ResponderThunk(const SharedData<Router*>& router)
+  explicit ResponderThunk(const base::WeakPtr<Router>& router)
       : router_(router), accept_was_invoked_(false) {}
   ~ResponderThunk() override {
     if (!accept_was_invoked_) {
       // The Mojo application handled a message that was expecting a response
       // but did not send a response.
-      Router* router = router_.value();
-      if (router) {
+      if (router_) {
         // We raise an error to signal the calling application that an error
         // condition occurred. Without this the calling application would have
         // no way of knowing it should stop waiting for a response.
-        router->RaiseError();
+        router_->RaiseError();
       }
     }
   }
@@ -41,21 +40,19 @@
 
     bool result = false;
 
-    Router* router = router_.value();
-    if (router)
-      result = router->Accept(message);
+    if (router_)
+      result = router_->Accept(message);
 
     return result;
   }
 
   // MessageReceiverWithStatus implementation:
   bool IsValid() override {
-    Router* router = router_.value();
-    return router && !router->encountered_error() && router->is_valid();
+    return router_ && !router_->encountered_error() && router_->is_valid();
   }
 
  private:
-  SharedData<Router*> router_;
+  base::WeakPtr<Router> router_;
   bool accept_was_invoked_;
 };
 
@@ -84,18 +81,20 @@
       connector_(std::move(message_pipe),
                  Connector::SINGLE_THREADED_SEND,
                  waiter),
-      weak_self_(this),
       incoming_receiver_(nullptr),
       next_request_id_(0),
-      testing_mode_(false) {
+      testing_mode_(false),
+      weak_factory_(this) {
   filters_.SetSink(&thunk_);
   connector_.set_incoming_receiver(filters_.GetHead());
 }
 
 Router::~Router() {
-  weak_self_.set_value(nullptr);
+  weak_factory_.InvalidateWeakPtrs();
 
-  for (auto& pair : responders_)
+  for (auto& pair : async_responders_)
+    delete pair.second;
+  for (auto& pair : sync_responders_)
     delete pair.second;
 }
 
@@ -118,8 +117,32 @@
   if (!connector_.Accept(message))
     return false;
 
-  // We assume ownership of |responder|.
-  responders_[request_id] = responder;
+  if (!message->has_flag(kMessageIsSync)) {
+    // We assume ownership of |responder|.
+    async_responders_[request_id] = responder;
+    return true;
+  }
+
+  sync_responders_[request_id] = responder;
+  base::WeakPtr<Router> weak_self = weak_factory_.GetWeakPtr();
+  for (;;) {
+    // TODO(yzshen): Here we should allow incoming sync requests to re-enter and
+    // block async messages.
+    bool result = WaitForIncomingMessage(MOJO_DEADLINE_INDEFINITE);
+    // The message pipe has disconnected.
+    if (!result)
+      break;
+
+    // This instance has been destroyed.
+    if (!weak_self)
+      break;
+
+    // The corresponding response message has arrived.
+    if (sync_responders_.find(request_id) == sync_responders_.end())
+      break;
+  }
+
+  // Return true means that we take ownership of |responder|.
   return true;
 }
 
@@ -135,21 +158,25 @@
     if (!incoming_receiver_)
       return false;
 
-    MessageReceiverWithStatus* responder = new ResponderThunk(weak_self_);
+    MessageReceiverWithStatus* responder =
+        new ResponderThunk(weak_factory_.GetWeakPtr());
     bool ok = incoming_receiver_->AcceptWithResponder(message, responder);
     if (!ok)
       delete responder;
     return ok;
 
   } else if (message->has_flag(kMessageIsResponse)) {
+    ResponderMap& responder_map = message->has_flag(kMessageIsSync)
+                                      ? sync_responders_
+                                      : async_responders_;
     uint64_t request_id = message->request_id();
-    ResponderMap::iterator it = responders_.find(request_id);
-    if (it == responders_.end()) {
+    ResponderMap::iterator it = responder_map.find(request_id);
+    if (it == responder_map.end()) {
       DCHECK(testing_mode_);
       return false;
     }
     MessageReceiver* responder = it->second;
-    responders_.erase(it);
+    responder_map.erase(it);
     bool ok = responder->Accept(message);
     delete responder;
     return ok;
diff --git a/mojo/public/cpp/bindings/lib/router.h b/mojo/public/cpp/bindings/lib/router.h
index 1191202..f7ce513 100644
--- a/mojo/public/cpp/bindings/lib/router.h
+++ b/mojo/public/cpp/bindings/lib/router.h
@@ -10,11 +10,11 @@
 #include <map>
 
 #include "base/logging.h"
+#include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
 #include "mojo/public/cpp/bindings/callback.h"
 #include "mojo/public/cpp/bindings/lib/connector.h"
 #include "mojo/public/cpp/bindings/lib/filter_chain.h"
-#include "mojo/public/cpp/bindings/lib/shared_data.h"
 #include "mojo/public/cpp/environment/environment.h"
 
 namespace mojo {
@@ -104,10 +104,12 @@
   // Returns true if this Router has any pending callbacks.
   bool has_pending_responders() const {
     DCHECK(thread_checker_.CalledOnValidThread());
-    return !responders_.empty();
+    return !async_responders_.empty() || !sync_responders_.empty();
   }
 
  private:
+  // Maps from the id of a response to the MessageReceiver that handles the
+  // response.
   typedef std::map<uint64_t, MessageReceiver*> ResponderMap;
 
   class HandleIncomingMessageThunk : public MessageReceiver {
@@ -127,14 +129,13 @@
   HandleIncomingMessageThunk thunk_;
   FilterChain filters_;
   Connector connector_;
-  SharedData<Router*> weak_self_;
   MessageReceiverWithResponderStatus* incoming_receiver_;
-  // Maps from the id of a response to the MessageReceiver that handles the
-  // response.
-  ResponderMap responders_;
+  ResponderMap async_responders_;
+  ResponderMap sync_responders_;
   uint64_t next_request_id_;
   bool testing_mode_;
   base::ThreadChecker thread_checker_;
+  base::WeakPtrFactory<Router> weak_factory_;
 };
 
 }  // namespace internal
diff --git a/mojo/public/cpp/bindings/tests/sync_method_unittest.cc b/mojo/public/cpp/bindings/tests/sync_method_unittest.cc
new file mode 100644
index 0000000..fd519b0
--- /dev/null
+++ b/mojo/public/cpp/bindings/tests/sync_method_unittest.cc
@@ -0,0 +1,135 @@
+// Copyright 2016 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.
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/macros.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/threading/thread.h"
+#include "mojo/message_pump/message_pump_mojo.h"
+#include "mojo/public/cpp/bindings/binding.h"
+#include "mojo/public/interfaces/bindings/tests/test_sync_methods.mojom.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace mojo {
+namespace test {
+namespace {
+
+class SyncMethodTest : public testing::Test {
+ public:
+  SyncMethodTest() : loop_(common::MessagePumpMojo::Create()) {}
+  ~SyncMethodTest() override { loop_.RunUntilIdle(); }
+
+ private:
+  base::MessageLoop loop_;
+};
+
+class TestSyncImpl : public TestSync {
+ public:
+  TestSyncImpl(TestSyncRequest request) : binding_(this, std::move(request)) {}
+
+  void set_ping_notification(const Closure& closure) {
+    ping_notification_ = closure;
+  }
+
+  // TestSync implementation:
+  void Get(const GetCallback& callback) override { callback.Run(42); }
+  void Set(int32_t value, const SetCallback& callback) override {
+    callback.Run();
+  }
+  void Ping(const PingCallback& callback) override {
+    ping_notification_.Run();
+    callback.Run();
+  }
+  void Echo(int32_t value, const EchoCallback& callback) override {
+    callback.Run(value);
+  }
+
+ private:
+  Binding<TestSync> binding_;
+  Closure ping_notification_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestSyncImpl);
+};
+
+class TestSyncServiceThread {
+ public:
+  TestSyncServiceThread()
+      : thread_("TestSyncServiceThread"), ping_called_(false) {
+    base::Thread::Options thread_options;
+    thread_options.message_pump_factory =
+        base::Bind(&common::MessagePumpMojo::Create);
+    thread_.StartWithOptions(thread_options);
+  }
+
+  void SetUp(TestSyncRequest request) {
+    CHECK(thread_.task_runner()->BelongsToCurrentThread());
+    impl_.reset(new TestSyncImpl(std::move(request)));
+    impl_->set_ping_notification([this]() {
+      base::AutoLock locker(lock_);
+      ping_called_ = true;
+    });
+  }
+
+  void TearDown() {
+    CHECK(thread_.task_runner()->BelongsToCurrentThread());
+    impl_.reset();
+  }
+
+  base::Thread* thread() { return &thread_; }
+  bool ping_called() const {
+    base::AutoLock locker(lock_);
+    return ping_called_;
+  }
+
+ private:
+  base::Thread thread_;
+
+  scoped_ptr<TestSyncImpl> impl_;
+
+  mutable base::Lock lock_;
+  bool ping_called_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestSyncServiceThread);
+};
+
+TEST_F(SyncMethodTest, CallSyncMethodAsynchronously) {
+  TestSyncPtr ptr;
+  TestSyncImpl impl(GetProxy(&ptr));
+
+  base::RunLoop run_loop;
+  ptr->Echo(123, [&run_loop](int32_t result) {
+    EXPECT_EQ(123, result);
+    run_loop.Quit();
+  });
+  run_loop.Run();
+}
+
+TEST_F(SyncMethodTest, BasicSyncCalls) {
+  TestSyncPtr ptr;
+
+  TestSyncServiceThread service_thread;
+  service_thread.thread()->task_runner()->PostTask(
+      FROM_HERE, base::Bind(&TestSyncServiceThread::SetUp,
+                            base::Unretained(&service_thread),
+                            base::Passed(GetProxy(&ptr))));
+  ASSERT_TRUE(ptr->Ping());
+  ASSERT_TRUE(service_thread.ping_called());
+
+  int32_t output_value = -1;
+  ASSERT_TRUE(ptr->Echo(42, &output_value));
+  ASSERT_EQ(42, output_value);
+
+  base::RunLoop run_loop;
+  service_thread.thread()->task_runner()->PostTaskAndReply(
+      FROM_HERE, base::Bind(&TestSyncServiceThread::TearDown,
+                            base::Unretained(&service_thread)),
+      run_loop.QuitClosure());
+  run_loop.Run();
+}
+
+}  // namespace
+}  // namespace test
+}  // namespace mojo
diff --git a/mojo/public/interfaces/bindings/tests/test_sync_methods.mojom b/mojo/public/interfaces/bindings/tests/test_sync_methods.mojom
new file mode 100644
index 0000000..2df8a1c
--- /dev/null
+++ b/mojo/public/interfaces/bindings/tests/test_sync_methods.mojom
@@ -0,0 +1,19 @@
+// Copyright 2016 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 mojo.test;
+
+interface TestSync {
+  [Sync]
+  Get() => (int32 result);
+
+  [Sync]
+  Set(int32 value) => ();
+
+  [Sync]
+  Ping() => ();
+
+  [Sync]
+  Echo(int32 value) => (int32 result);
+};
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/interface_declaration.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/interface_declaration.tmpl
index 7b4bab2..7bc6d6c 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/interface_declaration.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/interface_declaration.tmpl
@@ -45,6 +45,15 @@
 
 {%- for method in interface.methods %}
 {%    if method.response_parameters != None %}
+{%-     if method.sync %}
+  virtual bool {{method.name}}({{interface_macros.declare_sync_method_params("", method)}}) {
+    // Sync method. This signature is used by the client side; the service side
+    // should implement the signature with callback below.
+    NOTREACHED();
+    return false;
+  }
+{%-     endif %}
+
   using {{method.name}}Callback = {{interface_macros.declare_callback(method)}};
 {%-   endif %}
   virtual void {{method.name}}({{interface_macros.declare_request_params("", method)}}) = 0;
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/interface_definition.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/interface_definition.tmpl
index edf2a0a..1031ba6 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/interface_definition.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/interface_definition.tmpl
@@ -26,8 +26,8 @@
 {%-   endfor %}
 {%- endmacro %}
 
-{%- macro build_message(struct, struct_display_name) -%}
-  {{struct_macros.serialize(struct, struct_display_name, "in_%s", "params", "builder.buffer()")}}
+{%- macro build_message(struct, input_pattern, struct_display_name) -%}
+  {{struct_macros.serialize(struct, struct_display_name, input_pattern, "params", "builder.buffer()")}}
   params->EncodePointersAndHandles(builder.message()->mutable_handles());
 {%- endmacro %}
 
@@ -47,6 +47,51 @@
 {#--- ForwardToCallback definition #}
 {%- for method in interface.methods -%}
 {%-   if method.response_parameters != None %}
+{%-     if method.sync %}
+class {{class_name}}_{{method.name}}_HandleSyncResponse
+    : public mojo::MessageReceiver {
+ public:
+  {{class_name}}_{{method.name}}_HandleSyncResponse(
+      scoped_refptr<mojo::internal::MultiplexRouter> router, bool* result
+{%-       for param in method.response_parameters -%}
+      , {{param.kind|cpp_wrapper_type}}* out_{{param.name}}
+{%-       endfor %})
+      : serialization_context_(std::move(router)), result_(result)
+{%-       for param in method.response_parameters -%}
+        , out_{{param.name}}_(out_{{param.name}})
+{%-       endfor %} {
+    DCHECK(!*result_);
+  }
+  bool Accept(mojo::Message* message) override;
+ private:
+  mojo::internal::SerializationContext serialization_context_;
+  bool* result_;
+{%-       for param in method.response_parameters %}
+  {{param.kind|cpp_wrapper_type}}* out_{{param.name}}_;
+{%-       endfor -%}
+  DISALLOW_COPY_AND_ASSIGN({{class_name}}_{{method.name}}_HandleSyncResponse);
+};
+bool {{class_name}}_{{method.name}}_HandleSyncResponse::Accept(
+    mojo::Message* message) {
+  internal::{{class_name}}_{{method.name}}_ResponseParams_Data* params =
+      reinterpret_cast<internal::{{class_name}}_{{method.name}}_ResponseParams_Data*>(
+          message->mutable_payload());
+
+  params->DecodePointersAndHandles(message->mutable_handles());
+  {{alloc_params(method.response_param_struct, "&serialization_context_")}}
+
+{%-       for param in method.response_parameters %}
+{%-         if param.kind|is_move_only_kind %}
+  *out_{{param.name}}_ = std::move(p_{{param.name}});
+{%-         else %}
+  *out_{{param.name}}_ = p_{{param.name}};
+{%-         endif %}
+{%-       endfor %}
+  *result_ = true;
+  return true;
+}
+{%-     endif %}
+
 class {{class_name}}_{{method.name}}_ForwardToCallback
     : public mojo::MessageReceiver {
  public:
@@ -59,7 +104,7 @@
  private:
   {{class_name}}::{{method.name}}Callback callback_;
   mojo::internal::SerializationContext serialization_context_;
-  MOJO_DISALLOW_COPY_AND_ASSIGN({{class_name}}_{{method.name}}_ForwardToCallback);
+  DISALLOW_COPY_AND_ASSIGN({{class_name}}_{{method.name}}_ForwardToCallback);
 };
 bool {{class_name}}_{{method.name}}_ForwardToCallback::Accept(
     mojo::Message* message) {
@@ -87,6 +132,29 @@
 {%-   set params_struct = method.param_struct %}
 {%-   set params_description =
           "%s.%s request"|format(interface.name, method.name) %}
+{%-   if method.sync %}
+bool {{proxy_name}}::{{method.name}}(
+    {{interface_macros.declare_sync_method_params("param_", method)}}) {
+  {{struct_macros.get_serialized_size(params_struct, "param_%s")}}
+
+  mojo::internal::RequestMessageBuilder builder({{message_name}}, size,
+                                                mojo::internal::kMessageIsSync);
+
+  {{build_message(params_struct, "param_%s", params_description)}}
+
+  bool result = false;
+  mojo::MessageReceiver* responder =
+      new {{class_name}}_{{method.name}}_HandleSyncResponse(
+          serialization_context_.router, &result
+{%-     for param in method.response_parameters -%}
+          , param_{{param.name}}
+{%-     endfor %});
+  if (!receiver_->AcceptWithResponder(builder.message(), responder))
+    delete responder;
+  return result;
+}
+{%-   endif %}
+
 void {{proxy_name}}::{{method.name}}(
     {{interface_macros.declare_request_params("in_", method)}}) {
   {{struct_macros.get_serialized_size(params_struct, "in_%s")}}
@@ -97,7 +165,7 @@
   mojo::internal::MessageBuilder builder({{message_name}}, size);
 {%- endif %}
 
-  {{build_message(params_struct, params_description)}}
+  {{build_message(params_struct, "in_%s", params_description)}}
 
 {%- if method.response_parameters != None %}
   mojo::MessageReceiver* responder =
@@ -138,8 +206,10 @@
 
   {{class_name}}_{{method.name}}_ProxyToResponder(
       uint64_t request_id,
+      bool is_sync,
       mojo::MessageReceiverWithStatus* responder)
       : request_id_(request_id),
+        is_sync_(is_sync),
         responder_(responder) {
   }
 
@@ -147,15 +217,17 @@
 
  private:
   uint64_t request_id_;
+  bool is_sync_;
   mutable mojo::MessageReceiverWithStatus* responder_;
-  MOJO_DISALLOW_COPY_AND_ASSIGN({{class_name}}_{{method.name}}_ProxyToResponder);
+  DISALLOW_COPY_AND_ASSIGN({{class_name}}_{{method.name}}_ProxyToResponder);
 };
 void {{class_name}}_{{method.name}}_ProxyToResponder::Run(
     {{interface_macros.declare_params("in_", method.response_parameters)}}) const {
   {{struct_macros.get_serialized_size(response_params_struct, "in_%s")}}
   mojo::internal::ResponseMessageBuilder builder(
-      {{message_name}}, size, request_id_);
-  {{build_message(response_params_struct, params_description)}}
+      {{message_name}}, size, request_id_,
+      is_sync_ ? mojo::internal::kMessageIsSync : 0);
+  {{build_message(response_params_struct, "in_%s", params_description)}}
   bool ok = responder_->Accept(builder.message());
   MOJO_ALLOW_UNUSED_LOCAL(ok);
   // TODO(darin): !ok returned here indicates a malformed message, and that may
@@ -222,7 +294,9 @@
       {{alloc_params(method.param_struct, "&serialization_context_")|indent(4)}}
       {{class_name}}::{{method.name}}Callback::Runnable* runnable =
           new {{class_name}}_{{method.name}}_ProxyToResponder(
-              message->request_id(), responder);
+              message->request_id(),
+              message->has_flag(mojo::internal::kMessageIsSync),
+              responder);
       {{class_name}}::{{method.name}}Callback callback(runnable);
       // A null |sink_| means no implementation was bound.
       assert(sink_);
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/interface_macros.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/interface_macros.tmpl
index 7644b2e..a6dc1cf 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/interface_macros.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/interface_macros.tmpl
@@ -21,3 +21,14 @@
 const {{method.name}}Callback& callback
 {%-   endif -%}
 {%- endmacro -%}
+
+{%- macro declare_sync_method_params(prefix, method) -%}
+{{declare_params(prefix, method.parameters)}}
+{%-   if method.response_parameters %}
+{%-     if method.parameters %}, {% endif %}
+{%-     for param in method.response_parameters -%}
+{{param.kind|cpp_wrapper_type}}* {{prefix}}{{param.name}}
+{%-       if not loop.last %}, {% endif %}
+{%-     endfor %}
+{%-   endif -%}
+{%- endmacro -%}
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/interface_proxy_declaration.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/interface_proxy_declaration.tmpl
index 91219f3..477116b 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/interface_proxy_declaration.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/interface_proxy_declaration.tmpl
@@ -6,9 +6,10 @@
   explicit {{interface.name}}Proxy(mojo::MessageReceiverWithResponder* receiver);
 
 {%- for method in interface.methods %}
-  void {{method.name}}(
-      {{interface_macros.declare_request_params("", method)}}
-  ) override;
+{%-   if method.sync %}
+  bool {{method.name}}({{interface_macros.declare_sync_method_params("", method)}}) override;
+{%-   endif %}
+  void {{method.name}}({{interface_macros.declare_request_params("", method)}}) override;
 {%- endfor %}
 
   mojo::internal::SerializationContext* serialization_context() {
diff --git a/mojo/public/tools/bindings/pylib/mojom/generate/data.py b/mojo/public/tools/bindings/pylib/mojom/generate/data.py
index 01b9749..331d218 100644
--- a/mojo/public/tools/bindings/pylib/mojom/generate/data.py
+++ b/mojo/public/tools/bindings/pylib/mojom/generate/data.py
@@ -322,6 +322,14 @@
         lambda parameter: ParameterFromData(module, parameter, interface),
                           data['response_parameters'])
   method.attributes = data.get('attributes')
+
+  # Enforce that only methods with response can have a [Sync] attribute.
+  if method.sync and method.response_parameters is None:
+    raise Exception("Only methods with response can include a [Sync] "
+                    "attribute. If no response parameters are needed, you "
+                    "could use an empty response parameter list, i.e., "
+                    "\"=> ()\".")
+
   return method
 
 def InterfaceToData(interface):
diff --git a/mojo/public/tools/bindings/pylib/mojom/generate/module.py b/mojo/public/tools/bindings/pylib/mojom/generate/module.py
index cb76084..860438d 100644
--- a/mojo/public/tools/bindings/pylib/mojom/generate/module.py
+++ b/mojo/public/tools/bindings/pylib/mojom/generate/module.py
@@ -135,6 +135,7 @@
 
 ATTRIBUTE_MIN_VERSION = 'MinVersion'
 ATTRIBUTE_EXTENSIBLE = 'Extensible'
+ATTRIBUTE_SYNC = 'Sync'
 
 
 class NamedValue(object):
@@ -376,6 +377,11 @@
     return self.attributes.get(ATTRIBUTE_MIN_VERSION) \
         if self.attributes else None
 
+  @property
+  def sync(self):
+    return self.attributes.get(ATTRIBUTE_SYNC) \
+        if self.attributes else None
+
 
 class Interface(ReferenceKind):
   ReferenceKind.AddSharedProperty('module')