Allow tests to swap Mojo interface implementations.

This CL adds support to Binding and BindingSet to allow 
tests to swap the existing interface implementation with
a different one. It allows tests to interpose an implementation
that can change the interface behavior or inputs to it. 

Bug: 
Change-Id: If22a4d0124ab0d961519de9cefb2dab4bebf939f
Reviewed-on: https://chromium-review.googlesource.com/644187
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499267}

content/browser/isolated_origin_browsertest.cc

@@ -508,28 +508,42 @@ IN_PROC_BROWSER_TEST_F(IsolatedOriginTest, IsolatedOriginWithSubdomain) {
 // This class allows intercepting the OpenLocalStorage method and changing
 // the parameters to the real implementation of it.
 class StoragePartitonInterceptor
-    : public mojom::StoragePartitionServiceInterceptorForTesting {
+    : public mojom::StoragePartitionServiceInterceptorForTesting,
+      public RenderProcessHostObserver {
  public:
-  StoragePartitonInterceptor(RenderProcessHostImpl* rph) {
-    // Install a custom callback for handling errors during interface calls.
-    // This is needed because the passthrough calls that this object makes
-    // to the real implementaion of the service don't have the correct
-    // context to invoke the real error callback.
-    mojo::edk::SetDefaultProcessErrorCallback(base::Bind(
-        [](int process_id, const std::string& s) {
-          bad_message::ReceivedBadMessage(process_id,
-                                          bad_message::DSMF_LOAD_STORAGE);
-        },
-        rph->GetID()));
-
-    static_cast<StoragePartitionImpl*>(rph->GetStoragePartition())
-        ->Bind(rph->GetID(), mojo::MakeRequest(&storage_partition_service_));
+  StoragePartitonInterceptor(RenderProcessHostImpl* rph,
+                             mojom::StoragePartitionServiceRequest request) {
+    StoragePartitionImpl* storage_partition =
+        static_cast<StoragePartitionImpl*>(rph->GetStoragePartition());
+
+    // Bind the real StoragePartitionService implementation.
+    mojo::BindingId binding_id =
+        storage_partition->Bind(rph->GetID(), std::move(request));
+
+    // Now replace it with this object and keep a pointer to the real
+    // implementation.
+    storage_partition_service_ =
+        storage_partition->bindings_for_testing().SwapImplForTesting(binding_id,
+                                                                     this);
+
+    // Register the |this| as a RenderProcessHostObserver, so it can be
+    // correctly cleaned up when the process exits.
+    rph->AddObserver(this);
+  }
+
+  // Ensure this object is cleaned up when the process goes away, since it
+  // is not owned by anyone else.
+  void RenderProcessExited(RenderProcessHost* host,
+                           base::TerminationStatus status,
+                           int exit_code) override {
+    host->RemoveObserver(this);
+    delete this;
   }
 
   // Allow all methods that aren't explicitly overriden to pass through
   // unmodified.
   mojom::StoragePartitionService* GetForwardingInterface() override {
-    return storage_partition_service_.get();
+    return storage_partition_service_;
   }
 
   // Override this method to allow changing the origin. It simulates a
@@ -545,17 +559,15 @@ class StoragePartitonInterceptor
  private:
   // Keep a pointer to the original implementation of the service, so all
   // calls can be forwarded to it.
-  // Note: When making calls through this object, they are in-process calls,
-  // so state on the receiving side of the call will be missing the real
-  // information of which process has made the real method call.
-  mojom::StoragePartitionServicePtr storage_partition_service_;
+  mojom::StoragePartitionService* storage_partition_service_;
 };
 
 void CreateTestStoragePartitionService(
     RenderProcessHostImpl* rph,
     mojom::StoragePartitionServiceRequest request) {
-  mojo::MakeStrongBinding(base::MakeUnique<StoragePartitonInterceptor>(rph),
-                          std::move(request));
+  // This object will register as RenderProcessHostObserver, so it will
+  // clean itself automatically on process exit.
+  new StoragePartitonInterceptor(rph, std::move(request));
 }
 
 // Verify that an isolated renderer process cannot read localStorage of an

content/browser/storage_partition_impl.cc

@@ -689,7 +689,7 @@ void StoragePartitionImpl::OpenLocalStorage(
   int process_id = bindings_.dispatch_context();
   if (!ChildProcessSecurityPolicy::GetInstance()->CanAccessDataForOrigin(
           process_id, origin.GetURL())) {
-    mojo::ReportBadMessage("Access denied for localStorage request");
+    bindings_.ReportBadMessage("Access denied for localStorage request");
     return;
   }
   dom_storage_context_->OpenLocalStorage(origin, std::move(request));
@@ -1003,10 +1003,10 @@ BrowserContext* StoragePartitionImpl::browser_context() const {
   return browser_context_;
 }
 
-void StoragePartitionImpl::Bind(
+mojo::BindingId StoragePartitionImpl::Bind(
     int process_id,
     mojo::InterfaceRequest<mojom::StoragePartitionService> request) {
-  bindings_.AddBinding(this, std::move(request), process_id);
+  return bindings_.AddBinding(this, std::move(request), process_id);
 }
 
 void StoragePartitionImpl::OverrideQuotaManagerForTesting(

content/browser/storage_partition_impl.h

@@ -135,9 +135,13 @@ class CONTENT_EXPORT StoragePartitionImpl
   // Can return nullptr while |this| is being destroyed.
   BrowserContext* browser_context() const;
 
-  // Called by each renderer process once.
-  void Bind(int process_id,
-            mojo::InterfaceRequest<mojom::StoragePartitionService> request);
+  // Called by each renderer process once. Returns the id of the created
+  // binding.
+  mojo::BindingId Bind(
+      int process_id,
+      mojo::InterfaceRequest<mojom::StoragePartitionService> request);
+
+  auto& bindings_for_testing() { return bindings_; }
 
   struct DataDeletionHelper;
   struct QuotaManagedDataDeletionHelper;

mojo/public/cpp/bindings/binding.h

@@ -229,6 +229,11 @@ class Binding {
     return internal_state_.RouterForTesting();
   }
 
+  // Allows test code to swap the interface implementation.
+  ImplPointerType SwapImplForTesting(ImplPointerType new_impl) {
+    return internal_state_.SwapImplForTesting(new_impl);
+  }
+
  private:
   internal::BindingState<Interface, ImplRefTraits> internal_state_;
 

mojo/public/cpp/bindings/binding_set.h

@@ -123,6 +123,18 @@ class BindingSetBase {
     return true;
   }
 
+  // Swaps the interface implementation with a different one, to allow tests
+  // to modify behavior.
+  //
+  // Returns the existing interface implementation to the caller.
+  ImplPointerType SwapImplForTesting(BindingId id, ImplPointerType new_impl) {
+    auto it = bindings_.find(id);
+    if (it == bindings_.end())
+      return nullptr;
+
+    return it->second->SwapImplForTesting(new_impl);
+  }
+
   void CloseAllBindings() { bindings_.clear(); }
 
   bool empty() const { return bindings_.empty(); }
@@ -216,6 +228,10 @@ class BindingSetBase {
 
     void FlushForTesting() { binding_.FlushForTesting(); }
 
+    ImplPointerType SwapImplForTesting(ImplPointerType new_impl) {
+      return binding_.SwapImplForTesting(new_impl);
+    }
+
    private:
     class DispatchFilter : public MessageReceiver {
      public:

mojo/public/cpp/bindings/lib/binding_state.h

@@ -121,6 +121,11 @@ class BindingState : public BindingStateBase {
   }
 
   Interface* impl() { return ImplRefTraits::GetRawPointer(&stub_.sink()); }
+  ImplPointerType SwapImplForTesting(ImplPointerType new_impl) {
+    Interface* old_impl = impl();
+    stub_.set_sink(std::move(new_impl));
+    return old_impl;
+  }
 
  private:
   typename Interface::template Stub_<ImplRefTraits> stub_;

mojo/public/tools/bindings/generators/cpp_templates/interface_declaration.tmpl

@@ -69,6 +69,6 @@ class {{export_attribute}} {{interface.name}}InterceptorForTesting : public {{in
   virtual {{interface.name}}* GetForwardingInterface() = 0;
 
 {%- for method in interface.methods %}
-  void {{method.name}}({{interface_macros.declare_request_params("", method, use_once_callback)}});
+  void {{method.name}}({{interface_macros.declare_request_params("", method, use_once_callback)}}) override;
 {%- endfor %}
 };