Use unions for interface control messages.

Interface control messages were added before union support was ready so
the structs used for messages manually imitate unions. Now that union
support is ready, this workaround is no longer necessary and the structs
used for interface control messages can contain unions of the possible
message types.

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

mojo/public/cpp/bindings/lib/control_message_handler.cc

@@ -18,8 +18,8 @@ namespace internal {
 
 // static
 bool ControlMessageHandler::IsControlMessage(const Message* message) {
-  return message->header()->name == kRunMessageId ||
-         message->header()->name == kRunOrClosePipeMessageId;
+  return message->header()->name == interface_control::kRunMessageId ||
+         message->header()->name == interface_control::kRunOrClosePipeMessageId;
 }
 
 ControlMessageHandler::ControlMessageHandler(uint32_t interface_version)
@@ -30,7 +30,7 @@ ControlMessageHandler::~ControlMessageHandler() {
 }
 
 bool ControlMessageHandler::Accept(Message* message) {
-  if (message->header()->name == kRunOrClosePipeMessageId)
+  if (message->header()->name == interface_control::kRunOrClosePipeMessageId)
     return RunOrClosePipe(message);
 
   NOTREACHED();
@@ -40,7 +40,7 @@ bool ControlMessageHandler::Accept(Message* message) {
 bool ControlMessageHandler::AcceptWithResponder(
     Message* message,
     MessageReceiverWithStatus* responder) {
-  if (message->header()->name == kRunMessageId)
+  if (message->header()->name == interface_control::kRunMessageId)
     return Run(message, responder);
 
   NOTREACHED();
@@ -49,19 +49,33 @@ bool ControlMessageHandler::AcceptWithResponder(
 
 bool ControlMessageHandler::Run(Message* message,
                                 MessageReceiverWithStatus* responder) {
-  RunResponseMessageParamsPtr response_params_ptr(
-      RunResponseMessageParams::New());
-  response_params_ptr->reserved0 = 16u;
-  response_params_ptr->reserved1 = 0u;
-  response_params_ptr->query_version_result = QueryVersionResult::New();
-  response_params_ptr->query_version_result->version = interface_version_;
-
-  size_t size = PrepareToSerialize<RunResponseMessageParamsDataView>(
-      response_params_ptr, &context_);
-  ResponseMessageBuilder builder(kRunMessageId, size, message->request_id());
-
-  RunResponseMessageParams_Data* response_params = nullptr;
-  Serialize<RunResponseMessageParamsDataView>(
+  interface_control::internal::RunMessageParams_Data* params =
+      reinterpret_cast<interface_control::internal::RunMessageParams_Data*>(
+          message->mutable_payload());
+  interface_control::RunMessageParamsPtr params_ptr;
+  Deserialize<interface_control::RunMessageParamsDataView>(params, &params_ptr,
+                                                           &context_);
+  auto& input = *params_ptr->input;
+  interface_control::RunOutputPtr output = interface_control::RunOutput::New();
+  if (input.is_query_version()) {
+    output->set_query_version_result(
+        interface_control::QueryVersionResult::New());
+    output->get_query_version_result()->version = interface_version_;
+  } else {
+    output.reset();
+  }
+
+  auto response_params_ptr = interface_control::RunResponseMessageParams::New();
+  response_params_ptr->output = std::move(output);
+  size_t size =
+      PrepareToSerialize<interface_control::RunResponseMessageParamsDataView>(
+          response_params_ptr, &context_);
+  ResponseMessageBuilder builder(interface_control::kRunMessageId, size,
+                                 message->request_id());
+
+  interface_control::internal::RunResponseMessageParams_Data* response_params =
+      nullptr;
+  Serialize<interface_control::RunResponseMessageParamsDataView>(
       response_params_ptr, builder.buffer(), &response_params, &context_);
   bool ok = responder->Accept(builder.message());
   ALLOW_UNUSED_LOCAL(ok);
@@ -71,14 +85,18 @@ bool ControlMessageHandler::Run(Message* message,
 }
 
 bool ControlMessageHandler::RunOrClosePipe(Message* message) {
-  RunOrClosePipeMessageParams_Data* params =
-      reinterpret_cast<RunOrClosePipeMessageParams_Data*>(
+  interface_control::internal::RunOrClosePipeMessageParams_Data* params =
+      reinterpret_cast<
+          interface_control::internal::RunOrClosePipeMessageParams_Data*>(
           message->mutable_payload());
-  RunOrClosePipeMessageParamsPtr params_ptr;
-  Deserialize<RunOrClosePipeMessageParamsDataView>(params, &params_ptr,
-                                                   &context_);
+  interface_control::RunOrClosePipeMessageParamsPtr params_ptr;
+  Deserialize<interface_control::RunOrClosePipeMessageParamsDataView>(
+      params, &params_ptr, &context_);
+  auto& input = *params_ptr->input;
+  if (input.is_require_version())
+    return interface_version_ >= input.get_require_version()->version;
 
-  return interface_version_ >= params_ptr->require_version->version;
+  return false;
 }
 
 }  // namespace internal

mojo/public/cpp/bindings/lib/control_message_proxy.cc

@@ -20,11 +20,12 @@ namespace internal {
 
 namespace {
 
-using RunCallback = base::Callback<void(QueryVersionResultPtr)>;
+using RunCallback =
+    base::Callback<void(interface_control::RunResponseMessageParamsPtr)>;
 
 class RunResponseForwardToCallback : public MessageReceiver {
  public:
-  RunResponseForwardToCallback(const RunCallback& callback)
+  explicit RunResponseForwardToCallback(const RunCallback& callback)
       : callback_(callback) {}
   bool Accept(Message* message) override;
 
@@ -34,60 +35,64 @@ class RunResponseForwardToCallback : public MessageReceiver {
 };
 
 bool RunResponseForwardToCallback::Accept(Message* message) {
-  RunResponseMessageParams_Data* params =
-      reinterpret_cast<RunResponseMessageParams_Data*>(
+  interface_control::internal::RunResponseMessageParams_Data* params =
+      reinterpret_cast<
+          interface_control::internal::RunResponseMessageParams_Data*>(
           message->mutable_payload());
-  RunResponseMessageParamsPtr params_ptr;
+  interface_control::RunResponseMessageParamsPtr params_ptr;
   SerializationContext context;
-  Deserialize<RunResponseMessageParamsDataView>(params, &params_ptr, &context);
+  Deserialize<interface_control::RunResponseMessageParamsDataView>(
+      params, &params_ptr, &context);
 
-  callback_.Run(std::move(params_ptr->query_version_result));
+  callback_.Run(std::move(params_ptr));
   return true;
 }
 
 void SendRunMessage(MessageReceiverWithResponder* receiver,
-                    QueryVersionPtr query_version,
+                    interface_control::RunInputPtr input_ptr,
                     const RunCallback& callback,
                     SerializationContext* context) {
-  RunMessageParamsPtr params_ptr(RunMessageParams::New());
-  params_ptr->reserved0 = 16u;
-  params_ptr->reserved1 = 0u;
-  params_ptr->query_version = std::move(query_version);
-
-  size_t size =
-      PrepareToSerialize<RunMessageParamsDataView>(params_ptr, context);
-  RequestMessageBuilder builder(kRunMessageId, size);
-
-  RunMessageParams_Data* params = nullptr;
-  Serialize<RunMessageParamsDataView>(params_ptr, builder.buffer(), &params,
-                                      context);
+  auto params_ptr = interface_control::RunMessageParams::New();
+  params_ptr->input = std::move(input_ptr);
+  size_t size = PrepareToSerialize<interface_control::RunMessageParamsDataView>(
+      params_ptr, context);
+  RequestMessageBuilder builder(interface_control::kRunMessageId, size);
+
+  interface_control::internal::RunMessageParams_Data* params = nullptr;
+  Serialize<interface_control::RunMessageParamsDataView>(
+      params_ptr, builder.buffer(), &params, context);
   MessageReceiver* responder = new RunResponseForwardToCallback(callback);
   if (!receiver->AcceptWithResponder(builder.message(), responder))
     delete responder;
 }
 
-void SendRunOrClosePipeMessage(MessageReceiverWithResponder* receiver,
-                               RequireVersionPtr require_version,
-                               SerializationContext* context) {
-  RunOrClosePipeMessageParamsPtr params_ptr(RunOrClosePipeMessageParams::New());
-  params_ptr->reserved0 = 16u;
-  params_ptr->reserved1 = 0u;
-  params_ptr->require_version = std::move(require_version);
-
-  size_t size = PrepareToSerialize<RunOrClosePipeMessageParamsDataView>(
-      params_ptr, context);
-  MessageBuilder builder(kRunOrClosePipeMessageId, size);
-
-  RunOrClosePipeMessageParams_Data* params = nullptr;
-  Serialize<RunOrClosePipeMessageParamsDataView>(params_ptr, builder.buffer(),
-                                                 &params, context);
+void SendRunOrClosePipeMessage(
+    MessageReceiverWithResponder* receiver,
+    interface_control::RunOrClosePipeInputPtr input_ptr,
+    SerializationContext* context) {
+  auto params_ptr = interface_control::RunOrClosePipeMessageParams::New();
+  params_ptr->input = std::move(input_ptr);
+
+  size_t size = PrepareToSerialize<
+      interface_control::RunOrClosePipeMessageParamsDataView>(params_ptr,
+                                                              context);
+  MessageBuilder builder(interface_control::kRunOrClosePipeMessageId, size);
+
+  interface_control::internal::RunOrClosePipeMessageParams_Data* params =
+      nullptr;
+  Serialize<interface_control::RunOrClosePipeMessageParamsDataView>(
+      params_ptr, builder.buffer(), &params, context);
   bool ok = receiver->Accept(builder.message());
   ALLOW_UNUSED_LOCAL(ok);
 }
 
-void RunVersionCallback(const base::Callback<void(uint32_t)>& callback,
-                        QueryVersionResultPtr query_version_result) {
-  callback.Run(query_version_result->version);
+void RunVersionCallback(
+    const base::Callback<void(uint32_t)>& callback,
+    interface_control::RunResponseMessageParamsPtr run_response) {
+  uint32_t version = 0u;
+  if (run_response->output && run_response->output->is_query_version_result())
+    version = run_response->output->get_query_version_result()->version;
+  callback.Run(version);
 }
 
 }  // namespace
@@ -98,14 +103,18 @@ ControlMessageProxy::ControlMessageProxy(MessageReceiverWithResponder* receiver)
 
 void ControlMessageProxy::QueryVersion(
     const base::Callback<void(uint32_t)>& callback) {
-  SendRunMessage(receiver_, QueryVersion::New(),
+  auto input_ptr = interface_control::RunInput::New();
+  input_ptr->set_query_version(interface_control::QueryVersion::New());
+  SendRunMessage(receiver_, std::move(input_ptr),
                  base::Bind(&RunVersionCallback, callback), &context_);
 }
 
 void ControlMessageProxy::RequireVersion(uint32_t version) {
-  RequireVersionPtr require_version(RequireVersion::New());
+  auto require_version = interface_control::RequireVersion::New();
   require_version->version = version;
-  SendRunOrClosePipeMessage(receiver_, std::move(require_version), &context_);
+  auto input_ptr = interface_control::RunOrClosePipeInput::New();
+  input_ptr->set_require_version(std::move(require_version));
+  SendRunOrClosePipeMessage(receiver_, std::move(input_ptr), &context_);
 }
 
 }  // namespace internal

mojo/public/cpp/bindings/lib/validation_util.cc

@@ -103,16 +103,18 @@ bool ValidateMessageIsResponse(const Message* message,
 bool ValidateControlRequest(const Message* message,
                             ValidationContext* validation_context) {
   switch (message->header()->name) {
-    case kRunMessageId:
+    case interface_control::kRunMessageId:
       return ValidateMessageIsRequestExpectingResponse(message,
                                                        validation_context) &&
-             ValidateMessagePayload<RunMessageParams_Data>(message,
-                                                           validation_context);
-    case kRunOrClosePipeMessageId:
+             ValidateMessagePayload<
+                 interface_control::internal::RunMessageParams_Data>(
+                 message, validation_context);
+    case interface_control::kRunOrClosePipeMessageId:
       return ValidateMessageIsRequestWithoutResponse(message,
                                                      validation_context) &&
-          ValidateMessagePayload<RunOrClosePipeMessageParams_Data>(
-              message, validation_context);
+             ValidateMessagePayload<
+                 interface_control::internal::RunOrClosePipeMessageParams_Data>(
+                 message, validation_context);
   }
   return false;
 }
@@ -122,8 +124,9 @@ bool ValidateControlResponse(const Message* message,
   if (!ValidateMessageIsResponse(message, validation_context))
     return false;
   switch (message->header()->name) {
-    case kRunMessageId:
-      return ValidateMessagePayload<RunResponseMessageParams_Data>(
+    case interface_control::kRunMessageId:
+      return ValidateMessagePayload<
+          interface_control::internal::RunResponseMessageParams_Data>(
           message, validation_context);
   }
   return false;

mojo/public/interfaces/bindings/interface_control_messages.mojom

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-[JavaPackage="org.chromium.mojo.bindings"]
-module mojo;
+[JavaPackage="org.chromium.mojo.bindings.interfacecontrol"]
+module mojo.interface_control;
 
 // For each user-defined interface, some control functions are provided at the
 // same end of the message pipe as the user-defined interface, providing
@@ -15,38 +15,20 @@ module mojo;
 // This control function runs the input command. If the command is not
 // supported, |output| is set to null; otherwise |output| stores the result,
 // whose type depends on the input.
-//
-// TODO(yzshen): Once union support is ready, switch the following definition
-// to:
-// struct RunMessageParams {
-//   RunInput input;
-// };
-// union RunInput {
-//   QueryVersion query_version;
-// };
-//
-// struct RunResponseMessageParams {
-//   RunOutput? output;
-// };
-// union RunOutput {
-//   QueryVersionResult query_version_result;
-// };
 
 const uint32 kRunMessageId = 0xFFFFFFFF;
 
 struct RunMessageParams {
-  // The reserved fields make the layout compatible with the RunInput union
-  // described above.
-  uint32 reserved0;  // Must be set to 16.
-  uint32 reserved1;  // Must be set to 0;
+  RunInput input;
+};
+union RunInput {
   QueryVersion query_version;
 };
 
 struct RunResponseMessageParams {
-  // The reserved fields make the layout compatible with the RunOutput union
-  // described above.
-  uint32 reserved0;  // Must be set to 16.
-  uint32 reserved1;  // Must be set to 0.
+  RunOutput? output;
+};
+union RunOutput {
   QueryVersionResult query_version_result;
 };
 
@@ -62,23 +44,13 @@ struct QueryVersionResult {
 //
 // This control function runs the input command. If the operation fails or the
 // command is not supported, the message pipe is closed.
-//
-// TODO(yzshen): Once union support is ready, switch the following definition
-// to:
-// struct RunOrClosePipeMessageParams {
-//   RunOrClosePipeInput input;
-// };
-// union RunOrClosePipeInput {
-//   RequireVersion require_version;
-// };
 
 const uint32 kRunOrClosePipeMessageId = 0xFFFFFFFE;
 
 struct RunOrClosePipeMessageParams {
-  // The reserved fields make the layout compatible with the RunOrClosePipeInput
-  // union described above.
-  uint32 reserved0;  // Must be set to 16.
-  uint32 reserved1;  // Must be set to 0.
+  RunOrClosePipeInput input;
+};
+union RunOrClosePipeInput {
   RequireVersion require_version;
 };
 

mojo/public/java/bindings/src/org/chromium/mojo/bindings/Interface.java

@@ -6,6 +6,14 @@ package org.chromium.mojo.bindings;
 
 import org.chromium.mojo.bindings.Callbacks.Callback1;
 import org.chromium.mojo.bindings.Interface.AbstractProxy.HandlerImpl;
+import org.chromium.mojo.bindings.interfacecontrol.QueryVersion;
+import org.chromium.mojo.bindings.interfacecontrol.RequireVersion;
+import org.chromium.mojo.bindings.interfacecontrol.RunInput;
+import org.chromium.mojo.bindings.interfacecontrol.RunMessageParams;
+import org.chromium.mojo.bindings.interfacecontrol.RunOrClosePipeInput;
+import org.chromium.mojo.bindings.interfacecontrol.RunOrClosePipeMessageParams;
+import org.chromium.mojo.bindings.interfacecontrol.RunOutput;
+import org.chromium.mojo.bindings.interfacecontrol.RunResponseMessageParams;
 import org.chromium.mojo.system.Core;
 import org.chromium.mojo.system.MessagePipeHandle;
 import org.chromium.mojo.system.MojoException;
@@ -186,15 +194,18 @@ public interface Interface extends ConnectionErrorHandler, Closeable {
             @Override
             public void queryVersion(final Callback1<Integer> callback) {
                 RunMessageParams message = new RunMessageParams();
-                message.reserved0 = 16;
-                message.reserved1 = 0;
-                message.queryVersion = new QueryVersion();
+                message.input = new RunInput();
+                message.input.setQueryVersion(new QueryVersion());
 
                 InterfaceControlMessagesHelper.sendRunMessage(getCore(), mMessageReceiver, message,
                         new Callback1<RunResponseMessageParams>() {
                             @Override
                             public void call(RunResponseMessageParams response) {
-                                mVersion = response.queryVersionResult.version;
+                                if (response.output != null
+                                        && response.output.which()
+                                                == RunOutput.Tag.QueryVersionResult) {
+                                    mVersion = response.output.getQueryVersionResult().version;
+                                }
                                 callback.call(mVersion);
                             }
                         });
@@ -210,10 +221,9 @@ public interface Interface extends ConnectionErrorHandler, Closeable {
                 }
                 mVersion = version;
                 RunOrClosePipeMessageParams message = new RunOrClosePipeMessageParams();
-                message.reserved0 = 16;
-                message.reserved1 = 0;
-                message.requireVersion = new RequireVersion();
-                message.requireVersion.version = version;
+                message.input = new RunOrClosePipeInput();
+                message.input.setRequireVersion(new RequireVersion());
+                message.input.getRequireVersion().version = version;
                 InterfaceControlMessagesHelper.sendRunOrClosePipeMessage(
                         getCore(), mMessageReceiver, message);
             }

mojo/public/java/bindings/src/org/chromium/mojo/bindings/InterfaceControlMessagesHelper.java

@@ -7,6 +7,14 @@ package org.chromium.mojo.bindings;
 import org.chromium.mojo.bindings.Callbacks.Callback1;
 import org.chromium.mojo.bindings.Interface.Manager;
 import org.chromium.mojo.bindings.Interface.Proxy;
+import org.chromium.mojo.bindings.interfacecontrol.InterfaceControlMessagesConstants;
+import org.chromium.mojo.bindings.interfacecontrol.QueryVersionResult;
+import org.chromium.mojo.bindings.interfacecontrol.RunInput;
+import org.chromium.mojo.bindings.interfacecontrol.RunMessageParams;
+import org.chromium.mojo.bindings.interfacecontrol.RunOrClosePipeInput;
+import org.chromium.mojo.bindings.interfacecontrol.RunOrClosePipeMessageParams;
+import org.chromium.mojo.bindings.interfacecontrol.RunOutput;
+import org.chromium.mojo.bindings.interfacecontrol.RunResponseMessageParams;
 import org.chromium.mojo.system.Core;
 
 /**
@@ -64,11 +72,16 @@ public class InterfaceControlMessagesHelper {
      */
     public static <I extends Interface, P extends Proxy> boolean handleRun(
             Core core, Manager<I, P> manager, ServiceMessage message, MessageReceiver responder) {
+        Message payload = message.getPayload();
+        RunMessageParams query = RunMessageParams.deserialize(payload);
         RunResponseMessageParams response = new RunResponseMessageParams();
-        response.reserved0 = 16;
-        response.reserved1 = 0;
-        response.queryVersionResult = new QueryVersionResult();
-        response.queryVersionResult.version = manager.getVersion();
+        response.output = new RunOutput();
+        if (query.input.which() == RunInput.Tag.QueryVersion) {
+            response.output.setQueryVersionResult(new QueryVersionResult());
+            response.output.getQueryVersionResult().version = manager.getVersion();
+        } else {
+            response.output = null;
+        }
 
         return responder.accept(response.serializeWithHeader(
                 core, new MessageHeader(InterfaceControlMessagesConstants.RUN_MESSAGE_ID,
@@ -84,6 +97,9 @@ public class InterfaceControlMessagesHelper {
             Manager<I, P> manager, ServiceMessage message) {
         Message payload = message.getPayload();
         RunOrClosePipeMessageParams query = RunOrClosePipeMessageParams.deserialize(payload);
-        return query.requireVersion.version <= manager.getVersion();
+        if (query.input.which() == RunOrClosePipeInput.Tag.RequireVersion) {
+            return query.input.getRequireVersion().version <= manager.getVersion();
+        }
+        return false;
     }
 }

mojo/public/tools/bindings/generators/java_templates/interface_definition.tmpl

@@ -91,11 +91,11 @@ try {
     }
     switch(header.getType()) {
 {%   if with_response %}
-        case org.chromium.mojo.bindings.InterfaceControlMessagesConstants.RUN_MESSAGE_ID:
+        case org.chromium.mojo.bindings.interfacecontrol.InterfaceControlMessagesConstants.RUN_MESSAGE_ID:
             return org.chromium.mojo.bindings.InterfaceControlMessagesHelper.handleRun(
                     getCore(), {{interface|name}}_Internal.MANAGER, messageWithHeader, receiver);
 {%   else %}
-        case org.chromium.mojo.bindings.InterfaceControlMessagesConstants.RUN_OR_CLOSE_PIPE_MESSAGE_ID:
+        case org.chromium.mojo.bindings.interfacecontrol.InterfaceControlMessagesConstants.RUN_OR_CLOSE_PIPE_MESSAGE_ID:
             return org.chromium.mojo.bindings.InterfaceControlMessagesHelper.handleRunOrClosePipe(
                     {{interface|name}}_Internal.MANAGER, messageWithHeader);
 {%   endif %}