[WebMIDI] Add connection error handler for midi_client_

We check |midi_client_| before calling EndSession in the destructor of MidiHost as it tells us if we've started a session before. |midi_client_| could potentially be false if there was a connection error and could skip the EndSession call. Bug: 913270, 582328 Change-Id: Iececf880fcd61f1cc8ad65673e3dcfc9724ea5db Reviewed-on: https://chromium-review.googlesource.com/c/1376089 Commit-Queue: Takashi Toyoshima <[email protected]> Reviewed-by: Takashi Toyoshima <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#617079}(cherry picked from commit ceb2f74) Reviewed-on: https://chromium-review.googlesource.com/c/1388053 Cr-Commit-Position: refs/branch-heads/3626@{#497} Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
Igalia · Dec 21, 2018 · 58e7b7e · 58e7b7e
1 parent acf38d0
commit 58e7b7e
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 2 deletions.
diff --git a/content/browser/media/midi_host.cc b/content/browser/media/midi_host.cc
@@ -79,8 +79,6 @@ void MidiHost::CompleteStartSession(Result result) {
         ChildProcessSecurityPolicyImpl::GetInstance()->CanSendMidiSysExMessage(
             renderer_process_id_);
     midi_session_.Bind(std::move(pending_session_request_));
-    midi_session_.set_connection_error_handler(
-        base::BindOnce(&MidiHost::EndSession, base::Unretained(this)));
   }
   midi_client_->SessionStarted(result);
 }
@@ -176,6 +174,8 @@ void MidiHost::StartSession(midi::mojom::MidiSessionRequest request,
 
   DCHECK(!midi_client_);
   midi_client_ = std::move(client);
+  midi_client_.set_connection_error_handler(
+      base::BindOnce(&MidiHost::EndSession, base::Unretained(this)));
 
   if (midi_service_)
     midi_service_->StartSession(this);

diff --git a/content/browser/media/midi_host_unittest.cc b/content/browser/media/midi_host_unittest.cc
@@ -16,6 +16,7 @@
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "media/midi/midi_manager.h"
 #include "media/midi/midi_service.h"
+#include "mojo/public/cpp/bindings/strong_binding.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
@@ -105,6 +106,22 @@ class MidiHostForTesting : public MidiHost {
   DISALLOW_COPY_AND_ASSIGN(MidiHostForTesting);
 };
 
+class MidiSessionClientForTesting : public midi::mojom::MidiSessionClient {
+ public:
+  MidiSessionClientForTesting() = default;
+  ~MidiSessionClientForTesting() override = default;
+
+  void AddInputPort(midi::mojom::PortInfoPtr info) override {}
+  void AddOutputPort(midi::mojom::PortInfoPtr info) override {}
+  void SetInputPortState(uint32_t port, PortState state) override {}
+  void SetOutputPortState(uint32_t port, PortState state) override {}
+  void SessionStarted(midi::mojom::Result result) override {}
+  void AcknowledgeSentData(uint32_t bytes) override {}
+  void DataReceived(uint32_t port,
+                    const std::vector<uint8_t>& data,
+                    base::TimeTicks timestamp) override {}
+};
+
 class MidiHostTest : public testing::Test {
  public:
   MidiHostTest() : data_(kNoteOn, kNoteOn + base::size(kNoteOn)), port_id_(0) {
@@ -117,6 +134,8 @@ class MidiHostTest : public testing::Test {
     host_ = std::make_unique<MidiHostForTesting>(rph_->GetID(), service_.get());
     midi::mojom::MidiSessionClientPtr ptr;
     midi::mojom::MidiSessionClientRequest request = mojo::MakeRequest(&ptr);
+    mojo::MakeStrongBinding(std::make_unique<MidiSessionClientForTesting>(),
+                            std::move(request));
     midi::mojom::MidiSessionRequest session_request =
         mojo::MakeRequest(&session_);
     host_->StartSession(std::move(session_request), std::move(ptr));