Reland "serial: Add option to switch enumeration methods"

This reverts commit 412cff1e.

Reason for revert: Figured out why the change had no effect.

Original change's description:
> Revert "serial: Add option to switch enumeration methods"
>
> This reverts commit fdbcb258.
>
> Reason for revert: The new flag didn't resolve the issue.
>
> Original change's description:
> > serial: Add option to switch enumeration methods
> > 
> > A potential solution for issue 1119497 is to switch from enumerating
> > devices implementing GUID_DEVINTERFACE_COMPORT to using the "bus
> > enumerator" identified by GUID_DEVINTERFACE_SERENUM_BUS_ENUMERATOR.
> > 
> > I'm putting this behind a flag so that developers can try it out in
> > canary-channel. If it works I can enable it by default and keep the flag
> > around for a couple releases in case it causes other problems.
> > 
> > Bug: 1119497
> > Change-Id: Icad9008799ed331c3da15e5a70e0184144c47a58
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2417460
> > Commit-Queue: Ovidio de Jesús Ruiz-Henríquez <odejesush@chromium.org>
> > Reviewed-by: Ovidio de Jesús Ruiz-Henríquez <odejesush@chromium.org>
> > Auto-Submit: Reilly Grant <reillyg@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#808368}
>
> TBR=reillyg@chromium.org,odejesush@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 1119497
> Change-Id: I3e39d5de932e94b980e7fb5f6de71a2cfa45b0f3
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2423053
> Reviewed-by: Reilly Grant <reillyg@chromium.org>
> Commit-Queue: Reilly Grant <reillyg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#809207}

Bug: 1119497
Change-Id: I39bb97d5e1cc5ca5dc971ba2dfad3e9eb987141d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2432005
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Ovidio de Jesús Ruiz-Henríquez <odejesush@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810925}

chrome/browser/about_flags.cc

@@ -6435,6 +6435,13 @@ const FeatureEntry kFeatureEntries[] = {
      FEATURE_VALUE_TYPE(printing::features::kEnableOopPrintDrivers)},
 #endif
 
+#if defined(OS_WIN)
+    {"use-serial-bus-enumerator",
+     flag_descriptions::kUseSerialBusEnumeratorName,
+     flag_descriptions::kUseSerialBusEnumeratorDescription, kOsWin,
+     FEATURE_VALUE_TYPE(features::kUseSerialBusEnumerator)},
+#endif
+
 #if defined(OS_CHROMEOS)
     {"omnibox-rich-entities-in-launcher",
      flag_descriptions::kOmniboxRichEntitiesInLauncherName,

chrome/browser/flag-metadata.json

@@ -4544,6 +4544,11 @@
     "owners": [ "khorimoto", "zentaro" ],
     "expiry_milestone": 85
   },
+  {
+    "name": "use-serial-bus-enumerator",
+    "owners": [ "reillyg" ],
+    "expiry_milestone": 89
+  },
   {
     "name": "use-sync-sandbox",
     "owners": [ "//components/sync/OWNERS" ],

chrome/browser/flag_descriptions.cc

@@ -3425,6 +3425,11 @@ const char kUseAngleD3D11[] = "D3D11";
 const char kUseAngleD3D9[] = "D3D9";
 const char kUseAngleD3D11on12[] = "D3D11on12";
 
+const char kUseSerialBusEnumeratorName[] = "Use system serial port enumerator";
+const char kUseSerialBusEnumeratorDescription[] =
+    "Use the system-supplied enumerator for serial port devices instead of "
+    "enumerating devices implementing the COM port interface.";
+
 const char kUseWinrtMidiApiName[] = "Use Windows Runtime MIDI API";
 const char kUseWinrtMidiApiDescription[] =
     "Use Windows Runtime MIDI API for WebMIDI (effective only on Windows 10 or "

chrome/browser/flag_descriptions.h

@@ -1963,6 +1963,9 @@ extern const char kUseAngleD3D11[];
 extern const char kUseAngleD3D9[];
 extern const char kUseAngleD3D11on12[];
 
+extern const char kUseSerialBusEnumeratorName[];
+extern const char kUseSerialBusEnumeratorDescription[];
+
 extern const char kUseWinrtMidiApiName[];
 extern const char kUseWinrtMidiApiDescription[];
 

services/device/public/cpp/device_features.cc

@@ -23,4 +23,12 @@ const base::Feature kWinrtGeolocationImplementation{
 const base::Feature kMacCoreLocationImplementation{
     "kMacCoreLocationImplementation", base::FEATURE_ENABLED_BY_DEFAULT};
 
+#if defined(OS_WIN)
+// Switches from enumerating serial ports using GUID_DEVINTERFACE_SERIALPORT to
+// GUID_DEVINTERFACE_SERENUM_BUS_ENUMERATOR. This is an experimental solution to
+// https://crbug.com/1119497.
+const base::Feature kUseSerialBusEnumerator{"UseSerialBusEnumerator",
+                                            base::FEATURE_DISABLED_BY_DEFAULT};
+#endif  // defined(OS_WIN)
+
 }  // namespace features

services/device/public/cpp/device_features.h

@@ -9,6 +9,7 @@
 #define SERVICES_DEVICE_PUBLIC_CPP_DEVICE_FEATURES_H_
 
 #include "base/feature_list.h"
+#include "build/build_config.h"
 #include "services/device/public/cpp/device_features_export.h"
 
 namespace features {
@@ -22,6 +23,10 @@ DEVICE_FEATURES_EXPORT extern const base::Feature
 DEVICE_FEATURES_EXPORT extern const base::Feature
     kMacCoreLocationImplementation;
 
+#if defined(OS_WIN)
+DEVICE_FEATURES_EXPORT extern const base::Feature kUseSerialBusEnumerator;
+#endif  // defined(OS_WIN)
+
 }  // namespace features
 
 #endif  // SERVICES_DEVICE_PUBLIC_CPP_DEVICE_FEATURES_H_

services/device/serial/BUILD.gn

@@ -62,6 +62,7 @@ if (is_win || ((is_linux || is_chromeos) && use_udev) || is_mac) {
       "//device/bluetooth/public/cpp",
       "//mojo/public/cpp/bindings",
       "//net",
+      "//services/device/public/cpp:device_features",
       "//services/device/public/cpp/bluetooth:bluetooth",
       "//services/device/public/cpp/serial:switches",
     ]

services/device/serial/serial_device_enumerator_win.cc

@@ -19,6 +19,7 @@
 #include <string>
 #include <utility>
 
+#include "base/feature_list.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/numerics/ranges.h"
 #include "base/scoped_generic.h"
@@ -30,6 +31,7 @@
 #include "base/threading/scoped_blocking_call.h"
 #include "base/win/registry.h"
 #include "base/win/scoped_devinfo.h"
+#include "services/device/public/cpp/device_features.h"
 #include "third_party/re2/src/re2/re2.h"
 
 namespace device {
@@ -130,6 +132,10 @@ class SerialDeviceEnumeratorWin::UiThreadHelper
   void Initialize(base::WeakPtr<SerialDeviceEnumeratorWin> enumerator) {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     enumerator_ = std::move(enumerator);
+    // Note that this uses GUID_DEVINTERFACE_COMPORT regardless of the state of
+    // features::kUseSerialBusEnumerator because it doesn't seem to make a
+    // difference and ports which aren't enumerable by device interface GUID
+    // don't generate WM_DEVICECHANGE events.
     device_observer_.Add(
         DeviceMonitorWin::GetForDeviceInterface(GUID_DEVINTERFACE_COMPORT));
   }
@@ -238,9 +244,18 @@ void SerialDeviceEnumeratorWin::DoInitialEnumeration() {
   base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
                                                 base::BlockingType::MAY_BLOCK);
   // Make a device interface query to find all serial devices.
-  base::win::ScopedDevInfo dev_info(
-      SetupDiGetClassDevs(&GUID_DEVINTERFACE_COMPORT, nullptr, 0,
-                          DIGCF_DEVICEINTERFACE | DIGCF_PRESENT));
+  base::win::ScopedDevInfo dev_info;
+  if (base::FeatureList::IsEnabled(features::kUseSerialBusEnumerator)) {
+    // By using this GUID without passing DIGCF_DEVICEINTERFACE we get to
+    // enumerate all of the devices matching this GUID as a class, which is
+    // different from an interface and seems to find some otherwise unenumerable
+    // devices.  https://crbug.com/1119497
+    dev_info.reset(SetupDiGetClassDevs(
+        &GUID_DEVINTERFACE_SERENUM_BUS_ENUMERATOR, nullptr, 0, DIGCF_PRESENT));
+  } else {
+    dev_info.reset(SetupDiGetClassDevs(&GUID_DEVINTERFACE_COMPORT, nullptr, 0,
+                                       DIGCF_DEVICEINTERFACE | DIGCF_PRESENT));
+  }
   if (!dev_info.is_valid())
     return;