Web MIDI: fix a regression of r430234

components/test_runner/mock_web_midi_accessor.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 "components/test_runner/mock_web_midi_accessor.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/macros.h"
+#include "base/strings/stringprintf.h"
 #include "components/test_runner/test_interfaces.h"
 #include "components/test_runner/test_runner.h"
 #include "components/test_runner/web_test_delegate.h"
 #include "components/test_runner/web_test_runner.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/platform/modules/webmidi/WebMIDIAccessorClient.h"
 
 using midi::mojom::PortState;
 using midi::mojom::Result;
 
 namespace test_runner {
 
+namespace {
+
+const unsigned char kSysexHeader[] = {0xf0, 0x00, 0x02, 0x0d, 0x7f};

[yhirano, 2016/11/14 04:32:54]

constexpr

ditto below

[Takashi Toyoshima, 2016/11/14 06:25:28]

Done.

+const unsigned char kSysexFooter = 0xf7;
+const size_t kSysexMinimumLength =
+    arraysize(kSysexHeader) + sizeof(kSysexFooter) + 1;
+
+bool isSysexForTesting(const unsigned char* data, size_t length) {
+  // It should have five bytes header, one byte footer, and at least one byte
+  // payload.
+  if (length < kSysexMinimumLength)
+    return false;
+  if (memcmp(data, kSysexHeader, arraysize(kSysexHeader)))
+    return false;
+  return data[length - 1] == kSysexFooter;
+}
+
+}  // namespace
+
 MockWebMIDIAccessor::MockWebMIDIAccessor(blink::WebMIDIAccessorClient* client,
                                          TestInterfaces* interfaces)
-    : client_(client), interfaces_(interfaces), weak_factory_(this) {}
+    : client_(client),
+      interfaces_(interfaces),
+      max_input_port_index_(0),
+      max_output_port_index_(0),
+      weak_factory_(this) {}
 
 MockWebMIDIAccessor::~MockWebMIDIAccessor() {
 }
 
 void MockWebMIDIAccessor::startSession() {
   // Add a mock input and output port.
-  client_->didAddInputPort("MockInputID", "MockInputManufacturer",
-                           "MockInputName", "MockInputVersion",
-                           PortState::CONNECTED);
-  client_->didAddOutputPort("MockOutputID", "MockOutputManufacturer",
-                            "MockOutputName", "MockOutputVersion",
-                            PortState::CONNECTED);
+  addInputPort(PortState::CONNECTED);
+  addOutputPort(PortState::CONNECTED);
   interfaces_->GetDelegate()->PostTask(base::Bind(
-      &MockWebMIDIAccessor::ReportStartedSession, weak_factory_.GetWeakPtr(),
+      &MockWebMIDIAccessor::reportStartedSession, weak_factory_.GetWeakPtr(),
       interfaces_->GetTestRunner()->midiAccessorResult()));
 }
 
-void MockWebMIDIAccessor::ReportStartedSession(Result result) {
-  client_->didStartSession(result);
-}
-
 void MockWebMIDIAccessor::sendMIDIData(unsigned port_index,
                                        const unsigned char* data,
                                        size_t length,
                                        double timestamp) {
   // Emulate a loopback device for testing.
-  client_->didReceiveMIDIData(port_index, data, length, timestamp);
+  if (port_index <= max_output_port_index_)

[yhirano, 2016/11/14 04:32:54]

Is there any chance this condition doesn't hold?

[yhirano, 2016/11/14 04:32:54]

Shouldn't this '<=' be '<'?

[Takashi Toyoshima, 2016/11/14 06:25:27]

If it's only for this CL, I can change sysex commands to add both input and
output ports together so that we can omit this index check and send messages to
a relevant loop-back port safely, but I'd have different tests later with this
testing infra. E.g., I can test hot-plug functionalities in layout tests.

Anyway, this variable name is bad. I'd change it as next_xxxx_port_index_.

+    client_->didReceiveMIDIData(port_index, data, length, timestamp);
+
+  // Handle special sysex messages for testing.
+  // A special sequence is [0xf0, 0x00, 0x02, 0x0d, 0x7f, <function>, 0xf7].
+  // <function> should be one of following sequences.
+  //  - [0x00, 0x00]: Add an input port as connected.
+  //  - [0x00, 0x01]: Add an output port as connected.
+  //  - [0x00, 0x02]: Add an input port as opened.
+  //  - [0x00, 0x03]: Add an output port as opened.
+  if (!isSysexForTesting(data, length))
+    return;
+  size_t offset = arraysize(kSysexHeader);
+  if (data[offset++] != 0)
+    return;
+  switch (data[offset]) {
+    case 0:
+      addInputPort(PortState::CONNECTED);
+      break;
+    case 1:
+      addOutputPort(PortState::CONNECTED);
+      break;
+    case 2:
+      addInputPort(PortState::OPENED);
+      break;
+    case 3:
+      addOutputPort(PortState::OPENED);
+      break;

[yhirano, 2016/11/14 04:32:54]

default: break;

[Takashi Toyoshima, 2016/11/14 06:25:28]

Done.

+  }
+}
+
+void MockWebMIDIAccessor::addInputPort(PortState state) {
+  std::string id =
+      base::StringPrintf("MockInputID-%d", max_input_port_index_++);
+  client_->didAddInputPort(blink::WebString::fromUTF8(id),
+                           "MockInputManufacturer", "MockInputName",
+                           "MockInputVersion", state);
+}
+
+void MockWebMIDIAccessor::addOutputPort(PortState state) {
+  std::string id =
+      base::StringPrintf("MockOutputID-%d", max_output_port_index_++);
+  client_->didAddOutputPort(blink::WebString::fromUTF8(id),
+                            "MockOutputManufacturer", "MockOutputName",
+                            "MockOutputVersion", state);
+}
+
+void MockWebMIDIAccessor::reportStartedSession(Result result) {
+  client_->didStartSession(result);
 }
 
 }  // namespace test_runner