Enable the mojo-based serial API in the renderer behind a flag.

This enables the serial custom bindings that forward to the mojo serial
service behind the --enable-mojo-serial-service flag. This adds service
factory overrides to ServiceRegistrationManager for testing and updates
the serial api browsertest to test both serial implementations.

BUG=389016

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

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

chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc

@@ -532,6 +532,10 @@ void ChromeContentBrowserClientExtensionsPart::
 #if defined(ENABLE_WEBRTC)
     command_line->AppendSwitch(::switches::kEnableWebRtcHWH264Encoding);
 #endif
+    if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kEnableMojoSerialService)) {
+      command_line->AppendSwitch(switches::kEnableMojoSerialService);
+    }
   }
 }
 

extensions/BUILD.gn

@@ -94,6 +94,8 @@ source_set("test_support") {
     "test/test_extensions_client.h",
     "test/test_permission_message_provider.cc",
     "test/test_permission_message_provider.h",
+    "test/test_service_registration_manager.cc",
+    "test/test_service_registration_manager.h",
    ]
 
   deps = [

extensions/browser/api/serial/serial_apitest.cc

@@ -5,27 +5,24 @@
 #include <string>
 
 #include "chrome/browser/extensions/extension_apitest.h"
+#include "content/public/browser/browser_thread.h"
+#include "device/serial/serial_device_enumerator.h"
+#include "device/serial/serial_service_impl.h"
 #include "device/serial/test_serial_io_handler.h"
 #include "extensions/browser/api/serial/serial_api.h"
 #include "extensions/browser/api/serial/serial_connection.h"
 #include "extensions/browser/extension_function.h"
 #include "extensions/common/api/serial.h"
+#include "extensions/common/switches.h"
 #include "extensions/test/result_catcher.h"
+#include "extensions/test/test_service_registration_manager.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 using testing::_;
 using testing::Return;
 
-namespace {
-
-class SerialApiTest : public ExtensionApiTest {
- public:
-  SerialApiTest() {}
-};
-
-}  // namespace
-
 namespace extensions {
+namespace {
 
 class FakeSerialGetDevicesFunction : public AsyncExtensionFunction {
  public:
@@ -43,23 +40,44 @@ class FakeSerialGetDevicesFunction : public AsyncExtensionFunction {
   }
 
  protected:
-  ~FakeSerialGetDevicesFunction() override {}
+  ~FakeSerialGetDevicesFunction() {}
+};
+
+class FakeSerialDeviceEnumerator : public device::SerialDeviceEnumerator {
+ public:
+  ~FakeSerialDeviceEnumerator() override {}
+
+  mojo::Array<device::serial::DeviceInfoPtr> GetDevices() override {
+    mojo::Array<device::serial::DeviceInfoPtr> devices;
+    device::serial::DeviceInfoPtr device0(device::serial::DeviceInfo::New());
+    device0->path = "/dev/fakeserialmojo";
+    device::serial::DeviceInfoPtr device1(device::serial::DeviceInfo::New());
+    device1->path = "\\\\COM800\\";
+    devices.push_back(device0.Pass());
+    devices.push_back(device1.Pass());
+    return devices.Pass();
+  }
 };
 
 class FakeEchoSerialIoHandler : public device::TestSerialIoHandler {
  public:
-  explicit FakeEchoSerialIoHandler() {
+  FakeEchoSerialIoHandler() {
     device_control_signals()->dcd = true;
     device_control_signals()->cts = true;
     device_control_signals()->ri = true;
     device_control_signals()->dsr = true;
+    EXPECT_CALL(*this, SetControlSignals(_)).Times(1).WillOnce(Return(true));
+  }
+
+  static scoped_refptr<device::SerialIoHandler> Create() {
+    return new FakeEchoSerialIoHandler();
   }
 
   MOCK_METHOD1(SetControlSignals,
                bool(const device::serial::HostControlSignals&));
 
  protected:
-  virtual ~FakeEchoSerialIoHandler() {}
+  ~FakeEchoSerialIoHandler() override {}
 
  private:
   DISALLOW_COPY_AND_ASSIGN(FakeEchoSerialIoHandler);
@@ -72,8 +90,6 @@ class FakeSerialConnectFunction : public core_api::SerialConnectFunction {
       const std::string& owner_extension_id) const override {
     scoped_refptr<FakeEchoSerialIoHandler> io_handler =
         new FakeEchoSerialIoHandler;
-    EXPECT_CALL(*io_handler.get(), SetControlSignals(_)).Times(1).WillOnce(
-        Return(true));
     SerialConnection* serial_connection =
         new SerialConnection(port, owner_extension_id);
     serial_connection->SetIoHandlerForTest(io_handler);
@@ -81,19 +97,58 @@ class FakeSerialConnectFunction : public core_api::SerialConnectFunction {
   }
 
  protected:
-  ~FakeSerialConnectFunction() override {}
+  ~FakeSerialConnectFunction() {}
 };
 
-}  // namespace extensions
+class SerialApiTest : public ExtensionApiTest,
+                      public testing::WithParamInterface<bool> {
+ public:
+  SerialApiTest() {}
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    ExtensionApiTest::SetUpCommandLine(command_line);
+    if (GetParam())
+      command_line->AppendSwitch(switches::kEnableMojoSerialService);
+    test_service_registration_manager_.reset(
+        new TestServiceRegistrationManager);
+  }
+
+ protected:
+  scoped_ptr<TestServiceRegistrationManager> test_service_registration_manager_;
+};
 
 ExtensionFunction* FakeSerialGetDevicesFunctionFactory() {
-  return new extensions::FakeSerialGetDevicesFunction();
+  return new FakeSerialGetDevicesFunction();
 }
 
 ExtensionFunction* FakeSerialConnectFunctionFactory() {
-  return new extensions::FakeSerialConnectFunction();
+  return new FakeSerialConnectFunction();
+}
+
+void CreateTestSerialServiceOnFileThread(
+    mojo::InterfaceRequest<device::serial::SerialService> request) {
+  auto io_handler_factory = base::Bind(&FakeEchoSerialIoHandler::Create);
+  auto connection_factory = new device::SerialConnectionFactory(
+      io_handler_factory,
+      content::BrowserThread::GetMessageLoopProxyForThread(
+          content::BrowserThread::IO));
+  scoped_ptr<device::SerialDeviceEnumerator> device_enumerator(
+      new FakeSerialDeviceEnumerator);
+  mojo::BindToRequest(new device::SerialServiceImpl(connection_factory,
+                                                    device_enumerator.Pass()),
+                      &request);
 }
 
+void CreateTestSerialService(
+    mojo::InterfaceRequest<device::serial::SerialService> request) {
+  content::BrowserThread::PostTask(
+      content::BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(&CreateTestSerialServiceOnFileThread, base::Passed(&request)));
+}
+
+}  // namespace
+
 // Disable SIMULATE_SERIAL_PORTS only if all the following are true:
 //
 // 1. You have an Arduino or compatible board attached to your machine and
@@ -114,23 +169,32 @@ ExtensionFunction* FakeSerialConnectFunctionFactory() {
 // chrome/test/data/extensions/api_test/serial/api/serial_arduino_test.ino.
 //
 #define SIMULATE_SERIAL_PORTS (1)
-IN_PROC_BROWSER_TEST_F(SerialApiTest, SerialFakeHardware) {
-  extensions::ResultCatcher catcher;
+IN_PROC_BROWSER_TEST_P(SerialApiTest, SerialFakeHardware) {
+  ResultCatcher catcher;
   catcher.RestrictToBrowserContext(browser()->profile());
 
 #if SIMULATE_SERIAL_PORTS
-  ASSERT_TRUE(extensions::ExtensionFunctionDispatcher::OverrideFunction(
+  if (GetParam()) {
+    test_service_registration_manager_->OverrideServiceFactoryForTest(
+        base::Bind(&CreateTestSerialService));
+  } else {
+    ASSERT_TRUE(ExtensionFunctionDispatcher::OverrideFunction(
         "serial.getDevices", FakeSerialGetDevicesFunctionFactory));
-  ASSERT_TRUE(extensions::ExtensionFunctionDispatcher::OverrideFunction(
+    ASSERT_TRUE(ExtensionFunctionDispatcher::OverrideFunction(
         "serial.connect", FakeSerialConnectFunctionFactory));
+  }
 #endif
 
   ASSERT_TRUE(RunExtensionTest("serial/api")) << message_;
 }
 
-IN_PROC_BROWSER_TEST_F(SerialApiTest, SerialRealHardware) {
-  extensions::ResultCatcher catcher;
+IN_PROC_BROWSER_TEST_P(SerialApiTest, SerialRealHardware) {
+  ResultCatcher catcher;
   catcher.RestrictToBrowserContext(browser()->profile());
 
   ASSERT_TRUE(RunExtensionTest("serial/real_hardware")) << message_;
 }
+
+INSTANTIATE_TEST_CASE_P(SerialApiTest, SerialApiTest, testing::Bool());
+
+}  // namespace extensions

extensions/browser/mojo/service_registration_manager.cc

@@ -23,6 +23,8 @@ namespace {
 base::LazyInstance<ServiceRegistrationManager> g_lazy_instance =
     LAZY_INSTANCE_INITIALIZER;
 
+ServiceRegistrationManager* g_test_instance = nullptr;
+
 }  // namespace
 
 ServiceRegistrationManager::ServiceRegistrationManager() {
@@ -44,6 +46,8 @@ ServiceRegistrationManager::~ServiceRegistrationManager() {
 }
 
 ServiceRegistrationManager* ServiceRegistrationManager::GetSharedInstance() {
+  if (g_test_instance)
+    return g_test_instance;
   return g_lazy_instance.Pointer();
 }
 
@@ -72,9 +76,23 @@ void ServiceRegistrationManager::AddServicesToRenderFrame(
   for (const auto& factory : factories_) {
     auto availability = ExtensionAPI::GetSharedInstance()->IsAvailable(
         factory.first, extension, context_type, extension_url);
-    if (availability.is_available())
-      factory.second->Register(render_frame_host->GetServiceRegistry());
+    if (availability.is_available()) {
+      AddServiceToServiceRegistry(render_frame_host->GetServiceRegistry(),
+                                  factory.second.get());
+    }
   }
 }
 
+void ServiceRegistrationManager::AddServiceToServiceRegistry(
+    content::ServiceRegistry* service_registry,
+    internal::ServiceFactoryBase* factory) {
+  factory->Register(service_registry);
+}
+
+// static
+void ServiceRegistrationManager::SetServiceRegistrationManagerForTest(
+    ServiceRegistrationManager* service_registration_manager) {
+  g_test_instance = service_registration_manager;
+}
+
 }  // namespace extensions

extensions/browser/mojo/service_registration_manager.h

@@ -19,6 +19,7 @@ class RenderFrameHost;
 }
 
 namespace extensions {
+
 namespace internal {
 
 // A base class for forwarding calls to ServiceRegistry::AddService().
@@ -28,6 +29,8 @@ class ServiceFactoryBase {
 
   // Add this service factory to |service_registry|.
   virtual void Register(content::ServiceRegistry* service_registry) const = 0;
+
+  virtual std::string GetName() const = 0;
 };
 
 template <typename Interface>
@@ -42,6 +45,8 @@ class ServiceFactory : public ServiceFactoryBase {
     service_registry->AddService(factory_);
   }
 
+  std::string GetName() const override { return Interface::Name_; }
+
  private:
   const base::Callback<void(mojo::InterfaceRequest<Interface>)> factory_;
   DISALLOW_COPY_AND_ASSIGN(ServiceFactory);
@@ -57,7 +62,7 @@ class ServiceFactory : public ServiceFactoryBase {
 class ServiceRegistrationManager {
  public:
   ServiceRegistrationManager();
-  ~ServiceRegistrationManager();
+  virtual ~ServiceRegistrationManager();
 
   static ServiceRegistrationManager* GetSharedInstance();
 
@@ -84,7 +89,17 @@ class ServiceRegistrationManager {
   // ServiceRegistry.
   void AddServicesToRenderFrame(content::RenderFrameHost* render_frame_host);
 
+ protected:
+  virtual void AddServiceToServiceRegistry(
+      content::ServiceRegistry* service_registry,
+      internal::ServiceFactoryBase* service_factory);
+
  private:
+  friend class TestServiceRegistrationManager;
+
+  static void SetServiceRegistrationManagerForTest(
+      ServiceRegistrationManager* service_registration_manager);
+
   // All factories and their corresponding API permissions.
   std::vector<std::pair<std::string, linked_ptr<internal::ServiceFactoryBase>>>
       factories_;

extensions/extensions.gyp

@@ -1047,6 +1047,8 @@
         'test/test_extensions_client.h',
         'test/test_permission_message_provider.cc',
         'test/test_permission_message_provider.h',
+        'test/test_service_registration_manager.cc',
+        'test/test_service_registration_manager.h',
       ],
       # Disable c4267 warnings until we fix size_t to int truncations.
       'msvs_disabled_warnings': [ 4267, ],

extensions/renderer/api/serial/data_receiver_unittest.cc

@@ -18,12 +18,6 @@ class DataReceiverTest : public ApiTestBase {
 
   void SetUp() override {
     ApiTestBase::SetUp();
-    env()->RegisterModule("async_waiter", IDR_ASYNC_WAITER_JS);
-    env()->RegisterModule("data_receiver", IDR_DATA_RECEIVER_JS);
-    env()->RegisterModule("device/serial/data_stream.mojom",
-                          IDR_DATA_STREAM_MOJOM_JS);
-    env()->RegisterModule("device/serial/data_stream_serialization.mojom",
-                          IDR_DATA_STREAM_SERIALIZATION_MOJOM_JS);
     service_provider()->AddService(base::Bind(
         &DataReceiverTest::CreateDataSource, base::Unretained(this)));
   }

extensions/renderer/api/serial/data_sender_unittest.cc

@@ -18,12 +18,6 @@ class DataSenderTest : public ApiTestBase {
 
   void SetUp() override {
     ApiTestBase::SetUp();
-    env()->RegisterModule("async_waiter", IDR_ASYNC_WAITER_JS);
-    env()->RegisterModule("data_sender", IDR_DATA_SENDER_JS);
-    env()->RegisterModule("device/serial/data_stream.mojom",
-                          IDR_DATA_STREAM_MOJOM_JS);
-    env()->RegisterModule("device/serial/data_stream_serialization.mojom",
-                          IDR_DATA_STREAM_SERIALIZATION_MOJOM_JS);
     service_provider()->AddService(
         base::Bind(&DataSenderTest::CreateDataSink, base::Unretained(this)));
   }

extensions/renderer/api/serial/serial_api_unittest.cc

@@ -397,18 +397,7 @@ class SerialApiTest : public ApiTestBase {
 
   void SetUp() override {
     ApiTestBase::SetUp();
-    env()->RegisterModule("async_waiter", IDR_ASYNC_WAITER_JS);
-    env()->RegisterModule("data_receiver", IDR_DATA_RECEIVER_JS);
-    env()->RegisterModule("data_sender", IDR_DATA_SENDER_JS);
     env()->RegisterModule("serial", IDR_SERIAL_CUSTOM_BINDINGS_JS);
-    env()->RegisterModule("serial_service", IDR_SERIAL_SERVICE_JS);
-    env()->RegisterModule("device/serial/data_stream.mojom",
-                          IDR_DATA_STREAM_MOJOM_JS);
-    env()->RegisterModule("device/serial/data_stream_serialization.mojom",
-                          IDR_DATA_STREAM_SERIALIZATION_MOJOM_JS);
-    env()->RegisterModule("device/serial/serial.mojom", IDR_SERIAL_MOJOM_JS);
-    env()->RegisterModule("device/serial/serial_serialization.mojom",
-                          IDR_SERIAL_SERIALIZATION_MOJOM_JS);
     service_provider()->AddService<device::serial::SerialService>(base::Bind(
         &SerialApiTest::CreateSerialService, base::Unretained(this)));
     service_provider()->IgnoreServiceRequests<KeepAlive>();

extensions/renderer/dispatcher.cc

@@ -574,9 +574,17 @@ std::vector<std::pair<std::string, int> > Dispatcher::GetJsResources() {
       std::make_pair(mojo::kUnicodeModuleName, IDR_MOJO_UNICODE_JS));
   resources.push_back(
       std::make_pair(mojo::kValidatorModuleName, IDR_MOJO_VALIDATOR_JS));
+  resources.push_back(std::make_pair("async_waiter", IDR_ASYNC_WAITER_JS));
+  resources.push_back(std::make_pair("data_receiver", IDR_DATA_RECEIVER_JS));
+  resources.push_back(std::make_pair("data_sender", IDR_DATA_SENDER_JS));
   resources.push_back(std::make_pair("keep_alive", IDR_KEEP_ALIVE_JS));
   resources.push_back(std::make_pair("extensions/common/mojo/keep_alive.mojom",
                                      IDR_KEEP_ALIVE_MOJOM_JS));
+  resources.push_back(std::make_pair("device/serial/data_stream.mojom",
+                                     IDR_DATA_STREAM_MOJOM_JS));
+  resources.push_back(
+      std::make_pair("device/serial/data_stream_serialization.mojom",
+                     IDR_DATA_STREAM_SERIALIZATION_MOJOM_JS));
 
   // Custom bindings.
   resources.push_back(
@@ -601,6 +609,17 @@ std::vector<std::pair<std::string, int> > Dispatcher::GetJsResources() {
                      IDR_WEB_VIEW_REQUEST_CUSTOM_BINDINGS_JS));
   resources.push_back(std::make_pair("binding", IDR_BINDING_JS));
 
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableMojoSerialService)) {
+    resources.push_back(
+        std::make_pair("serial", IDR_SERIAL_CUSTOM_BINDINGS_JS));
+  }
+  resources.push_back(std::make_pair("serial_service", IDR_SERIAL_SERVICE_JS));
+  resources.push_back(
+      std::make_pair("device/serial/serial.mojom", IDR_SERIAL_MOJOM_JS));
+  resources.push_back(std::make_pair("device/serial/serial_serialization.mojom",
+                                     IDR_SERIAL_SERIALIZATION_MOJOM_JS));
+
   // Custom types sources.
   resources.push_back(std::make_pair("StorageArea", IDR_STORAGE_AREA_JS));
 

extensions/renderer/module_system.cc

@@ -10,6 +10,7 @@
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_view.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extensions_client.h"
@@ -144,6 +145,10 @@ ModuleSystem::ModuleSystem(ScriptContext* context, SourceMap* source_map)
                          v8::External::New(isolate, this));
 
   gin::ModuleRegistry::From(context->v8_context())->AddObserver(this);
+  if (context_->GetRenderFrame()) {
+    context_->GetRenderFrame()->EnsureMojoBuiltinsAreAvailable(
+        context->isolate(), context->v8_context());
+  }
 }
 
 ModuleSystem::~ModuleSystem() { Invalidate(); }

extensions/test/test_service_registration_manager.cc

+// Copyright 2014 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 "extensions/test/test_service_registration_manager.h"
+
+namespace extensions {
+
+TestServiceRegistrationManager::TestServiceRegistrationManager() {
+  ServiceRegistrationManager::SetServiceRegistrationManagerForTest(this);
+}
+
+TestServiceRegistrationManager::~TestServiceRegistrationManager() {
+  ServiceRegistrationManager::SetServiceRegistrationManagerForTest(nullptr);
+}
+
+void TestServiceRegistrationManager::AddServiceToServiceRegistry(
+    content::ServiceRegistry* service_registry,
+    internal::ServiceFactoryBase* factory) {
+  auto test_factory = test_factories_.find(factory->GetName());
+  if (test_factory != test_factories_.end()) {
+    test_factory->second->Register(service_registry);
+  } else {
+    ServiceRegistrationManager::AddServiceToServiceRegistry(service_registry,
+                                                            factory);
+  }
+}
+
+}  // namespace extensions

extensions/test/test_service_registration_manager.h

+// Copyright 2014 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.
+
+#ifndef EXTENSIONS_TEST_TEST_SERVICE_REGISTRATION_MANAGER_H_
+#define EXTENSIONS_TEST_TEST_SERVICE_REGISTRATION_MANAGER_H_
+
+#include <map>
+#include <string>
+
+#include "extensions/browser/mojo/service_registration_manager.h"
+
+namespace extensions {
+
+class TestServiceRegistrationManager : public ServiceRegistrationManager {
+ public:
+  TestServiceRegistrationManager();
+  ~TestServiceRegistrationManager() override;
+
+  // Overrides an existing service factory with |factory| for testing. This
+  // does not alter the permission checks used to determine whether a service
+  // is available.
+  template <typename Interface>
+  void OverrideServiceFactoryForTest(
+      const base::Callback<void(mojo::InterfaceRequest<Interface>)>& factory) {
+    linked_ptr<internal::ServiceFactoryBase> service_factory(
+        new internal::ServiceFactory<Interface>(factory));
+    auto result = test_factories_.insert(
+        std::make_pair(Interface::Name_, service_factory));
+    DCHECK(result.second);
+  }
+
+  void AddServiceToServiceRegistry(
+      content::ServiceRegistry* service_registry,
+      internal::ServiceFactoryBase* service_factory) override;
+
+ private:
+  std::map<std::string, linked_ptr<internal::ServiceFactoryBase>>
+      test_factories_;
+
+  ServiceRegistrationManager* service_registration_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestServiceRegistrationManager);
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_TEST_TEST_SERVICE_REGISTRATION_MANAGER_H_