serial: Filter ports detected after UI is shown

This change adds a missing check that a new serial port matches the
filter specified by the developer when it is detected after the chooser
UI is shown.

Matching tests are added to exercise this case.

Bug: 1130674
Change-Id: I3f7a5086b98971cafd69fe5e630500932944af98
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2422431
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Auto-Submit: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Vincent Scheib <scheib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809117}

chrome/browser/ui/serial/serial_chooser_controller.cc

@@ -114,6 +114,9 @@ void SerialChooserController::OpenHelpCenterUrl() const {
 
 void SerialChooserController::OnPortAdded(
     const device::mojom::SerialPortInfo& port) {
+  if (!FilterMatchesAny(port))
+    return;
+
   ports_.push_back(port.Clone());
   if (view())
     view()->OnOptionAdded(ports_.size() - 1);

chrome/browser/ui/serial/serial_chooser_controller_unittest.cc

@@ -39,6 +39,28 @@ class SerialChooserControllerTest : public ChromeRenderViewHostTestHarness {
         ->SetPortManagerForTesting(std::move(port_manager));
   }
 
+  base::UnguessableToken AddPort(
+      const std::string& display_name,
+      const base::FilePath& path,
+      base::Optional<uint16_t> vendor_id = base::nullopt,
+      base::Optional<uint16_t> product_id = base::nullopt) {
+    auto port = device::mojom::SerialPortInfo::New();
+    port->token = base::UnguessableToken::Create();
+    port->display_name = display_name;
+    port->path = path;
+    if (vendor_id) {
+      port->has_vendor_id = true;
+      port->vendor_id = *vendor_id;
+    }
+    if (product_id) {
+      port->has_product_id = true;
+      port->product_id = *product_id;
+    }
+    base::UnguessableToken port_token = port->token;
+    port_manager().AddPort(std::move(port));
+    return port_token;
+  }
+
   device::FakeSerialPortManager& port_manager() { return port_manager_; }
 
  private:
@@ -111,11 +133,7 @@ TEST_F(SerialChooserControllerTest, PortsAddedAndRemoved) {
   EXPECT_EQ(base::ASCIIToUTF16("Test Port 1 (ttyS0)"),
             controller->GetOption(0));
 
-  port = device::mojom::SerialPortInfo::New();
-  port->token = base::UnguessableToken::Create();
-  port->display_name = "Test Port 2";
-  port->path = base::FilePath(FILE_PATH_LITERAL("/dev/ttyS1"));
-  port_manager().AddPort(std::move(port));
+  AddPort("Test Port 2", base::FilePath(FILE_PATH_LITERAL("/dev/ttyS1")));
   {
     base::RunLoop run_loop;
     EXPECT_CALL(view, OnOptionAdded(_)).WillOnce(Invoke([&](size_t index) {
@@ -151,12 +169,8 @@ TEST_F(SerialChooserControllerTest, PortsAddedAndRemoved) {
 TEST_F(SerialChooserControllerTest, PortSelected) {
   base::HistogramTester histogram_tester;
 
-  auto port = device::mojom::SerialPortInfo::New();
-  port->token = base::UnguessableToken::Create();
-  port->display_name = "Test Port";
-  port->path = base::FilePath(FILE_PATH_LITERAL("/dev/ttyS0"));
-  base::UnguessableToken port_token = port->token;
-  port_manager().AddPort(std::move(port));
+  base::UnguessableToken port_token =
+      AddPort("Test Port", base::FilePath(FILE_PATH_LITERAL("/dev/ttyS0")));
 
   base::MockCallback<content::SerialChooser::Callback> callback;
   std::vector<blink::mojom::SerialPortFilterPtr> filters;
@@ -192,3 +206,65 @@ TEST_F(SerialChooserControllerTest, PortSelected) {
       "Permissions.Serial.ChooserClosed",
       SerialChooserOutcome::kEphemeralPermissionGranted, 1);
 }
+
+TEST_F(SerialChooserControllerTest, PortFiltered) {
+  base::HistogramTester histogram_tester;
+
+  // Create two ports from the same vendor with different product IDs.
+  base::UnguessableToken port_1 =
+      AddPort("Test Port 1", base::FilePath(FILE_PATH_LITERAL("/dev/ttyS0")),
+              0x1234, 0x0001);
+  base::UnguessableToken port_2 =
+      AddPort("Test Port 2", base::FilePath(FILE_PATH_LITERAL("/dev/ttyS1")),
+              0x1234, 0x0002);
+
+  // Create a filter which will select only the first port.
+  std::vector<blink::mojom::SerialPortFilterPtr> filters;
+  auto filter = blink::mojom::SerialPortFilter::New();
+  filter->has_vendor_id = true;
+  filter->vendor_id = 0x1234;
+  filter->has_product_id = true;
+  filter->product_id = 0x0001;
+  filters.push_back(std::move(filter));
+
+  auto controller = std::make_unique<SerialChooserController>(
+      main_rfh(), std::move(filters), base::DoNothing());
+
+  MockChooserControllerView view;
+  controller->set_view(&view);
+
+  {
+    base::RunLoop run_loop;
+    EXPECT_CALL(view, OnOptionsInitialized).WillOnce(Invoke([&] {
+      // Expect that only the first port is shown thanks to the filter.
+      EXPECT_EQ(1u, controller->NumOptions());
+      EXPECT_EQ(base::ASCIIToUTF16("Test Port 1 (ttyS0)"),
+                controller->GetOption(0));
+      run_loop.Quit();
+    }));
+    run_loop.Run();
+  }
+
+  // Removing the second port should be a no-op since it is filtered out.
+  EXPECT_CALL(view, OnOptionRemoved).Times(0);
+  port_manager().RemovePort(port_2);
+  base::RunLoop().RunUntilIdle();
+
+  // Adding it back should be a no-op as well.
+  EXPECT_CALL(view, OnOptionAdded).Times(0);
+  AddPort("Test Port 2", base::FilePath(FILE_PATH_LITERAL("/dev/ttyS1")),
+          0x1234, 0x0002);
+  base::RunLoop().RunUntilIdle();
+
+  // Removing the first port should trigger a change in the UI. This also acts
+  // as a synchronization point to make sure that the changes above were
+  // processed.
+  {
+    base::RunLoop run_loop;
+    EXPECT_CALL(view, OnOptionRemoved(0)).WillOnce(Invoke([&]() {
+      run_loop.Quit();
+    }));
+    port_manager().RemovePort(port_1);
+    run_loop.Run();
+  }
+}