Use bidirectional native message api for DriveFS

This ports the existing native message functionality to use
CreateNativeHostSession() if the corresponding flag is enabled.

Bug: 1105319
Change-Id: Iadb4544e39ebc818f5e0309dd8e98b5fd14c801e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2306508
Commit-Queue: Austin Tankiang <austinct@chromium.org>
Reviewed-by: Sergei Datsenko <dats@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790694}

chrome/browser/chromeos/BUILD.gn

@@ -3039,6 +3039,7 @@ source_set("unit_tests") {
     "customization/customization_document_unittest.cc",
     "dbus/proxy_resolution_service_provider_unittest.cc",
     "drive/drive_integration_service_unittest.cc",
+    "drive/drivefs_native_message_host_unittest.cc",
     "drive/file_system_util_unittest.cc",
     "eol_notification_unittest.cc",
     "events/event_rewriter_unittest.cc",

chrome/browser/chromeos/drive/drivefs_native_message_host.cc

@@ -5,13 +5,17 @@
 #include "chrome/browser/chromeos/drive/drivefs_native_message_host.h"
 
 #include "base/bind.h"
+#include "base/feature_list.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/chromeos/drive/drive_integration_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/extension_constants.h"
+#include "chromeos/constants/chromeos_features.h"
 #include "components/drive/file_errors.h"
+#include "mojo/public/cpp/bindings/receiver.h"
+#include "mojo/public/cpp/bindings/remote.h"
 
 namespace drive {
 
@@ -24,14 +28,36 @@ const char* const kDriveFsNativeMessageHostOrigins[] = {
 constexpr size_t kDriveFsNativeMessageHostOriginsSize =
     base::size(kDriveFsNativeMessageHostOrigins);
 
-class DriveFsNativeMessageHost : public extensions::NativeMessageHost {
+class DriveFsNativeMessageHost : public extensions::NativeMessageHost,
+                                 public drivefs::mojom::NativeMessagingPort {
  public:
+  // Used when the native messaging session is initiated by the extension.
   explicit DriveFsNativeMessageHost(Profile* profile)
       : drive_service_(DriveIntegrationServiceFactory::GetForProfile(profile)) {
   }
+
+  // Used when the native messaging session is initiated by DriveFS.
+  DriveFsNativeMessageHost(
+      mojo::PendingReceiver<drivefs::mojom::NativeMessagingPort>
+          extension_receiver,
+      mojo::PendingRemote<drivefs::mojom::NativeMessagingHost> drivefs_remote)
+      : pending_receiver_(std::move(extension_receiver)),
+        drivefs_remote_(std::move(drivefs_remote)) {
+    DCHECK(UseBidirectionalNativeMessaging());
+  }
+
+  explicit DriveFsNativeMessageHost(
+      drivefs::mojom::DriveFs* drivefs_for_testing)
+      : drivefs_for_testing_(drivefs_for_testing) {}
+
   ~DriveFsNativeMessageHost() override = default;
 
   void OnMessage(const std::string& message) override {
+    DCHECK(client_);
+
+    if (UseBidirectionalNativeMessaging()) {
+      drivefs_remote_->HandleMessageFromExtension(message);
+    } else {
       if (!drive_service_ || !drive_service_->GetDriveFsInterface()) {
         OnDriveFsResponse(FILE_ERROR_SERVICE_UNAVAILABLE, "");
         return;
@@ -41,8 +67,32 @@ class DriveFsNativeMessageHost : public extensions::NativeMessageHost {
           message, base::BindOnce(&DriveFsNativeMessageHost::OnDriveFsResponse,
                                   weak_ptr_factory_.GetWeakPtr()));
     }
+  }
 
-  void Start(Client* client) override { client_ = client; }
+  void Start(Client* client) override {
+    client_ = client;
+
+    if (!UseBidirectionalNativeMessaging()) {
+      return;
+    }
+
+    if (!pending_receiver_) {
+      // The session was initiated by the extension.
+      mojo::PendingRemote<drivefs::mojom::NativeMessagingPort> extension_port;
+      pending_receiver_ = extension_port.InitWithNewPipeAndPassReceiver();
+      drivefs::mojom::DriveFs* drivefs =
+          drivefs_for_testing_ ? drivefs_for_testing_
+                               : drive_service_->GetDriveFsInterface();
+      drivefs->CreateNativeHostSession(
+          drivefs::mojom::ExtensionConnectionParams::New(
+              kDriveFsNativeMessageHostOrigins[0]),
+          drivefs_remote_.BindNewPipeAndPassReceiver(),
+          std::move(extension_port));
+    }
+    receiver_.Bind(std::move(std::move(pending_receiver_)));
+    receiver_.set_disconnect_with_reason_handler(base::Bind(
+        &DriveFsNativeMessageHost::OnDisconnect, base::Unretained(this)));
+  }
 
   scoped_refptr<base::SingleThreadTaskRunner> task_runner() const override {
     return task_runner_;
@@ -58,7 +108,29 @@ class DriveFsNativeMessageHost : public extensions::NativeMessageHost {
     }
   }
 
-  DriveIntegrationService* drive_service_;
+  void PostMessageToExtension(const std::string& message) override {
+    client_->PostMessageFromNativeHost(message);
+  }
+
+  void OnDisconnect(uint32_t error, const std::string& reason) {
+    client_->CloseChannel(FileErrorToString(static_cast<FileError>(
+                              -static_cast<int32_t>(error))) +
+                          ": " + reason);
+    drivefs_remote_.reset();
+  }
+
+  bool UseBidirectionalNativeMessaging() {
+    return base::FeatureList::IsEnabled(
+        chromeos::features::kDriveFsBidirectionalNativeMessaging);
+  }
+
+  DriveIntegrationService* drive_service_ = nullptr;
+  drivefs::mojom::DriveFs* drivefs_for_testing_ = nullptr;
+
+  // Used to buffer messages until Start() has been called.
+  mojo::PendingReceiver<drivefs::mojom::NativeMessagingPort> pending_receiver_;
+  mojo::Receiver<drivefs::mojom::NativeMessagingPort> receiver_{this};
+  mojo::Remote<drivefs::mojom::NativeMessagingHost> drivefs_remote_;
 
   Client* client_ = nullptr;
 
@@ -76,4 +148,19 @@ std::unique_ptr<extensions::NativeMessageHost> CreateDriveFsNativeMessageHost(
       Profile::FromBrowserContext(browser_context));
 }
 
+std::unique_ptr<extensions::NativeMessageHost>
+CreateDriveFsInitiatedNativeMessageHost(
+    mojo::PendingReceiver<drivefs::mojom::NativeMessagingPort>
+        extension_receiver,
+    mojo::PendingRemote<drivefs::mojom::NativeMessagingHost> drivefs_remote) {
+  return std::make_unique<DriveFsNativeMessageHost>(
+      std::move(extension_receiver), std::move(drivefs_remote));
+}
+
+std::unique_ptr<extensions::NativeMessageHost>
+CreateDriveFsNativeMessageHostForTesting(
+    drivefs::mojom::DriveFs* drivefs_for_testing) {
+  return std::make_unique<DriveFsNativeMessageHost>(drivefs_for_testing);
+}
+
 }  // namespace drive

chrome/browser/chromeos/drive/drivefs_native_message_host.h

@@ -7,8 +7,11 @@
 
 #include <memory>
 
+#include "chromeos/components/drivefs/mojom/drivefs.mojom.h"
 #include "content/public/browser/browser_context.h"
 #include "extensions/browser/api/messaging/native_message_host.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
 
 namespace drive {
 
@@ -21,6 +24,16 @@ extern const size_t kDriveFsNativeMessageHostOriginsSize;
 std::unique_ptr<extensions::NativeMessageHost> CreateDriveFsNativeMessageHost(
     content::BrowserContext* browser_context);
 
+std::unique_ptr<extensions::NativeMessageHost>
+CreateDriveFsInitiatedNativeMessageHost(
+    mojo::PendingReceiver<drivefs::mojom::NativeMessagingPort>
+        extension_receiver,
+    mojo::PendingRemote<drivefs::mojom::NativeMessagingHost> drivefs_remote);
+
+std::unique_ptr<extensions::NativeMessageHost>
+CreateDriveFsNativeMessageHostForTesting(
+    drivefs::mojom::DriveFs* drivefs_for_testing);
+
 }  // namespace drive
 
 #endif  //  CHROME_BROWSER_CHROMEOS_DRIVE_DRIVEFS_NATIVE_MESSAGE_HOST_H_

chrome/browser/chromeos/drive/drivefs_native_message_host_unittest.cc

+// Copyright 2019 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 "chrome/browser/chromeos/drive/drivefs_native_message_host.h"
+
+#include <memory>
+
+#include "base/run_loop.h"
+#include "base/test/gmock_callback_support.h"
+#include "base/test/scoped_feature_list.h"
+#include "base/test/task_environment.h"
+#include "chromeos/components/drivefs/mojom/drivefs.mojom-test-utils.h"
+#include "chromeos/components/drivefs/mojom/drivefs.mojom.h"
+#include "chromeos/constants/chromeos_features.h"
+#include "extensions/browser/api/messaging/native_message_host.h"
+#include "mojo/public/cpp/bindings/receiver.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace drive {
+namespace {
+
+using base::test::RunOnceClosure;
+using testing::_;
+using testing::InvokeWithoutArgs;
+
+class MockClient : public extensions::NativeMessageHost::Client {
+ public:
+  MockClient() {}
+
+  MOCK_METHOD(void,
+              PostMessageFromNativeHost,
+              (const std::string& message),
+              (override));
+  MOCK_METHOD(void,
+              CloseChannel,
+              (const std::string& error_message),
+              (override));
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockClient);
+};
+
+class DriveFsNativeMessageHostTest
+    : public testing::Test,
+      public drivefs::mojom::DriveFsInterceptorForTesting,
+      public drivefs::mojom::NativeMessagingHost {
+ public:
+  DriveFsNativeMessageHostTest() {
+    scoped_feature_list_.InitAndEnableFeature(
+        chromeos::features::kDriveFsBidirectionalNativeMessaging);
+  }
+
+  DriveFs* GetForwardingInterface() override {
+    NOTREACHED();
+    return nullptr;
+  }
+
+  void CreateNativeHostSession(
+      drivefs::mojom::ExtensionConnectionParamsPtr params,
+      mojo::PendingReceiver<drivefs::mojom::NativeMessagingHost> session,
+      mojo::PendingRemote<drivefs::mojom::NativeMessagingPort> port) {
+    params_ = std::move(params);
+    extension_port_.Bind(std::move(port));
+    receiver_.Bind(std::move(session));
+  }
+
+  MOCK_METHOD(void,
+              HandleMessageFromExtension,
+              (const std::string& message),
+              (override));
+
+  base::test::ScopedFeatureList scoped_feature_list_;
+  base::test::TaskEnvironment task_environment_;
+  drivefs::mojom::ExtensionConnectionParamsPtr params_;
+  mojo::Receiver<drivefs::mojom::NativeMessagingHost> receiver_{this};
+  mojo::Remote<drivefs::mojom::NativeMessagingPort> extension_port_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DriveFsNativeMessageHostTest);
+};
+
+TEST_F(DriveFsNativeMessageHostTest, DriveFsInitiatedMessaging) {
+  base::RunLoop run_loop;
+
+  std::unique_ptr<extensions::NativeMessageHost> host =
+      CreateDriveFsInitiatedNativeMessageHost(
+          extension_port_.BindNewPipeAndPassReceiver(),
+          receiver_.BindNewPipeAndPassRemote());
+  MockClient client;
+  EXPECT_CALL(client, PostMessageFromNativeHost("foo"))
+      .WillOnce(InvokeWithoutArgs([&host]() { host->OnMessage("bar"); }));
+  EXPECT_CALL(*this, HandleMessageFromExtension("bar"))
+      .WillOnce(InvokeWithoutArgs(
+          [this]() { extension_port_->PostMessageToExtension("baz"); }));
+  EXPECT_CALL(client, PostMessageFromNativeHost("baz"))
+      .WillOnce(RunOnceClosure(run_loop.QuitClosure()));
+
+  host->Start(&client);
+  extension_port_->PostMessageToExtension("foo");
+  run_loop.Run();
+}
+
+TEST_F(DriveFsNativeMessageHostTest, ExtensionInitiatedMessaging) {
+  base::RunLoop run_loop;
+
+  std::unique_ptr<extensions::NativeMessageHost> host =
+      CreateDriveFsNativeMessageHostForTesting(this);
+  MockClient client;
+  host->Start(&client);
+  EXPECT_CALL(*this, HandleMessageFromExtension("foo"))
+      .WillOnce(InvokeWithoutArgs(
+          [this]() { extension_port_->PostMessageToExtension("bar"); }));
+  EXPECT_CALL(client, PostMessageFromNativeHost("bar"))
+      .WillOnce(RunOnceClosure(run_loop.QuitClosure()));
+  host->OnMessage("foo");
+  run_loop.Run();
+}
+
+TEST_F(DriveFsNativeMessageHostTest, NativeHostSendsMessageBeforeStart) {
+  std::unique_ptr<extensions::NativeMessageHost> host =
+      CreateDriveFsInitiatedNativeMessageHost(
+          extension_port_.BindNewPipeAndPassReceiver(),
+          receiver_.BindNewPipeAndPassRemote());
+  MockClient client;
+
+  EXPECT_CALL(client, PostMessageFromNativeHost(_)).Times(0);
+  extension_port_->PostMessageToExtension("foo");
+  base::RunLoop().RunUntilIdle();
+
+  {
+    base::RunLoop run_loop;
+    EXPECT_CALL(client, PostMessageFromNativeHost("foo"))
+        .WillOnce(InvokeWithoutArgs([&host]() { host->OnMessage("bar"); }));
+    EXPECT_CALL(*this, HandleMessageFromExtension("bar"))
+        .WillOnce(RunOnceClosure(run_loop.QuitClosure()));
+    host->Start(&client);
+    run_loop.Run();
+  }
+}
+
+TEST_F(DriveFsNativeMessageHostTest, Error) {
+  base::RunLoop run_loop;
+
+  std::unique_ptr<extensions::NativeMessageHost> host =
+      CreateDriveFsInitiatedNativeMessageHost(
+          extension_port_.BindNewPipeAndPassReceiver(),
+          receiver_.BindNewPipeAndPassRemote());
+  MockClient client;
+  EXPECT_CALL(client, PostMessageFromNativeHost).Times(0);
+  EXPECT_CALL(client, CloseChannel("FILE_ERROR_FAILED: foo"));
+  receiver_.set_disconnect_handler(run_loop.QuitClosure());
+
+  host->Start(&client);
+  extension_port_.ResetWithReason(1u, "foo");
+
+  run_loop.Run();
+}
+
+}  // namespace
+}  // namespace drive

chromeos/components/drivefs/fake_drivefs.cc

@@ -446,4 +446,9 @@ void FakeDriveFs::DumpAccountSettings() {}
 
 void FakeDriveFs::LoadAccountSettings() {}
 
+void FakeDriveFs::CreateNativeHostSession(
+    drivefs::mojom::ExtensionConnectionParamsPtr params,
+    mojo::PendingReceiver<drivefs::mojom::NativeMessagingHost> session,
+    mojo::PendingRemote<drivefs::mojom::NativeMessagingPort> port) {}
+
 }  // namespace drivefs

chromeos/components/drivefs/fake_drivefs.h

@@ -117,6 +117,11 @@ class FakeDriveFs : public drivefs::mojom::DriveFs,
 
   void LoadAccountSettings() override;
 
+  void CreateNativeHostSession(
+      drivefs::mojom::ExtensionConnectionParamsPtr params,
+      mojo::PendingReceiver<drivefs::mojom::NativeMessagingHost> session,
+      mojo::PendingRemote<drivefs::mojom::NativeMessagingPort> port) override;
+
   const base::FilePath mount_path_;
 
   std::map<base::FilePath, FileMetadata> metadata_;

chromeos/components/drivefs/mojom/drivefs.mojom

@@ -95,6 +95,11 @@ interface DriveFs {
   // Loads account settings (including feature flags) from
   // |data_dir_path/account_settings. Should only be called in developer mode.
   LoadAccountSettings();
+
+  // Establish communication session between extension and DriveFS.
+  CreateNativeHostSession(ExtensionConnectionParams params,
+      pending_receiver<NativeMessagingHost> session,
+      pending_remote<NativeMessagingPort> port);
 };
 
 // Implemented by Chrome, used from DriveFS.
@@ -422,3 +427,27 @@ struct FetchChangeLogOptions {
   // fetch the My Drive changelog instead.
   string team_drive_id;
 };
+
+// Implemented in the browser and used from DriveFS.
+// Roughly corresponds to a native messaging port to the extension and allows
+// sending messages from the DriveFS process to the extension.
+interface NativeMessagingPort {
+  // Send a native |message| to the extension. |message| is an
+  // opaque string in an extension-specific format.
+  PostMessageToExtension(string message);
+};
+
+// Implemented in DriveFS and used from the browser.
+// Roughly corresponds to a native messaging host instance and allows
+// processing of requests from the extension by the DriveFS process.
+// It's up to the extension and the host to agree to message ordering.
+interface NativeMessagingHost {
+  // Process |message| from the extension. |message| is an
+  // opaque string in an extension-specific format.
+  HandleMessageFromExtension(string message);
+};
+
+// Configuration of the connection between extension and DriveFS.
+struct ExtensionConnectionParams {
+  string extension_id;
+};