Clean up HID backend and API.

device/hid/hid_service.h

Index: device/hid/hid_service.h
diff --git a/device/hid/hid_service.h b/device/hid/hid_service.h
index 2589220121dac7cbc68308c53cf1dbae0cb39ea0..ff44992d554f23b0564ae66d623f17799b546431 100644
--- a/device/hid/hid_service.h
+++ b/device/hid/hid_service.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2014 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -9,25 +9,15 @@
 #include <string>
 #include <vector>
 
-#include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
-#include "base/strings/string16.h"
 #include "base/threading/thread_checker.h"
-#include "build/build_config.h"
 #include "device/hid/hid_device_info.h"
 
 namespace device {
 
-namespace {
-
-class HidServiceContainer;
-
-}  // namespace
-
 class HidConnection;
-class HidService;
 
 class HidService : public base::MessageLoop::DestructionObserver {
  public:
@@ -40,19 +30,15 @@ class HidService : public base::MessageLoop::DestructionObserver {
   // Fills in the device info struct of the given device_id.
   // Returns true if succeed.
   // Returns false if the device_id is invalid, with info untouched.
-  bool GetInfo(std::string device_id, HidDeviceInfo* info) const;
+  bool GetInfo(const std::string& device_id, HidDeviceInfo* info) const;

[Mark Mentovai, 2014/02/19 22:48:15]

Good call turning these into const refs.

 
   virtual scoped_refptr<HidConnection> Connect(
-      std::string platform_device_id) = 0;
+      const std::string& device_id) = 0;
 
   // Implements base::MessageLoop::DestructionObserver
   virtual void WillDestroyCurrentMessageLoop() OVERRIDE;
 
-  // Gets whether the HidService have been successfully initialized.
-  bool initialized() const { return initialized_; }
-
  protected:
-  friend class HidServiceContainer;
   friend struct base::DefaultDeleter<HidService>;
   friend class HidConnectionTest;
 
@@ -62,13 +48,11 @@ class HidService : public base::MessageLoop::DestructionObserver {
   static HidService* CreateInstance();
 
   virtual void AddDevice(HidDeviceInfo info);
-  virtual void RemoveDevice(std::string platform_device_id);
+  virtual void RemoveDevice(const std::string& device_id);
 
   typedef std::map<std::string, HidDeviceInfo> DeviceMap;
   DeviceMap devices_;

[Mark Mentovai, 2014/02/19 22:48:15]

Hmm, protected data. That’s not a good sign. Can this be private?

Using a protected data member as an interface between a superclass and its
subclasses gets hairy and out-of-control very quickly. Especially in cases like
this, where there isn’t even any documentation about what the base class does
with this and what it expects subclasses to do with it.

Most communication between a base class and its subclasses should be through
method calls, not shared data.

[Ken Rockot(use gerrit already), 2014/02/21 02:15:36]

Agreed. Moved to private.

 
-  bool initialized_;
-
   base::ThreadChecker thread_checker_;
 
   DISALLOW_COPY_AND_ASSIGN(HidService);