chrome.hid: enrich model with report IDs

device/hid/hid_connection.h

Index: device/hid/hid_connection.h
diff --git a/device/hid/hid_connection.h b/device/hid/hid_connection.h
index 5c08fbc5bbdf5733e1e3a3c63149bed7044d768b..91b8a18e575dc700f1c1da79088fa6313de6e78f 100644
--- a/device/hid/hid_connection.h
+++ b/device/hid/hid_connection.h
@@ -7,8 +7,11 @@
 
 #include <stdint.h>
 
+#include <queue>
+
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
+#include "base/threading/thread_checker.h"
 #include "device/hid/hid_device_info.h"
 #include "net/base/io_buffer.h"
 
@@ -18,29 +21,53 @@ class HidConnection : public base::RefCountedThreadSafe<HidConnection> {
  public:
   typedef base::Callback<void(bool success, size_t size)> IOCallback;
 
-  virtual void Read(scoped_refptr<net::IOBufferWithSize> buffer,
-                    const IOCallback& callback) = 0;
-  virtual void Write(uint8_t report_id,
-                     scoped_refptr<net::IOBufferWithSize> buffer,
-                     const IOCallback& callback) = 0;
-  virtual void GetFeatureReport(uint8_t report_id,
-                                scoped_refptr<net::IOBufferWithSize> buffer,
-                                const IOCallback& callback) = 0;
-  virtual void SendFeatureReport(uint8_t report_id,
-                                 scoped_refptr<net::IOBufferWithSize> buffer,
-                                 const IOCallback& callback) = 0;
-
   const HidDeviceInfo& device_info() const { return device_info_; }
+  bool has_protected_collection() const { return has_protected_collection_; }
+  bool has_report_id() const { return has_report_id_; }
+  const base::ThreadChecker& thread_checker() const { return thread_checker_; }
+
+  void Read(scoped_refptr<net::IOBufferWithSize> buffer,

[jracle (use Gerrit), 2014/06/11 11:20:04]

I made those methods the single entry-point, and create abstract Platform*
methods called within them. Purpose is to concentrate report validation logic
here, as a must-do step.

[Ken Rockot(use gerrit already), 2014/06/19 19:29:57]

Great!

+            const IOCallback& callback);
+  void Write(uint8_t report_id,
+             scoped_refptr<net::IOBufferWithSize> buffer,
+             const IOCallback& callback);
+  void GetFeatureReport(uint8_t report_id,
+                        scoped_refptr<net::IOBufferWithSize> buffer,
+                        const IOCallback& callback);
+  void SendFeatureReport(uint8_t report_id,
+                         scoped_refptr<net::IOBufferWithSize> buffer,
+                         const IOCallback& callback);
 
  protected:
   friend class base::RefCountedThreadSafe<HidConnection>;
-  friend struct HidDeviceInfo;
 
   explicit HidConnection(const HidDeviceInfo& device_info);
   virtual ~HidConnection();
 
+  virtual void PlatformRead(scoped_refptr<net::IOBufferWithSize> buffer,
+                            const IOCallback& callback) = 0;
+  virtual void PlatformWrite(uint8_t report_id,
+                             scoped_refptr<net::IOBufferWithSize> buffer,
+                             const IOCallback& callback) = 0;
+  virtual void PlatformGetFeatureReport(
+      uint8_t report_id,
+      scoped_refptr<net::IOBufferWithSize> buffer,
+      const IOCallback& callback) = 0;
+  virtual void PlatformSendFeatureReport(
+      uint8_t report_id,
+      scoped_refptr<net::IOBufferWithSize> buffer,
+      const IOCallback& callback) = 0;
+
+  bool FilterInputReport(scoped_refptr<net::IOBufferWithSize> buffer,

[Ken Rockot(use gerrit already), 2014/06/19 19:29:56]

Can this be private?

+                         const IOCallback& callback);
+
  private:
+  bool IsReportIdProtected(const uint8_t report_id);
+
   const HidDeviceInfo device_info_;
+  bool has_report_id_;
+  bool has_protected_collection_;
+  base::ThreadChecker thread_checker_;
 
   DISALLOW_COPY_AND_ASSIGN(HidConnection);
 };
@@ -60,6 +87,25 @@ struct PendingHidRead {
   HidConnection::IOCallback callback;
 };
 
+class HidConnection2 : public HidConnection {

[jracle (use Gerrit), 2014/06/11 11:20:04]

This is of course 'temporary code' before refactoring.
We can try to move this code in parent class, or find relevant name for this
class, which is base class used by HidConnectionLinux and HidConnectionMac.

[Ken Rockot(use gerrit already), 2014/06/19 19:29:56]

I am very confused by this.

[jracle (use Gerrit), 2014/06/19 21:39:36]

Code duplication removal.

On 2014/06/19 19:29:56, Ken Rockot wrote:

+ protected:
+  explicit HidConnection2(const HidDeviceInfo& device_info);
+  virtual ~HidConnection2();
+
+  // HidConnection implementation.
+  virtual void PlatformRead(scoped_refptr<net::IOBufferWithSize> buffer,
+                            const IOCallback& callback) OVERRIDE;
+
+  void ProcessInputReport(scoped_refptr<net::IOBufferWithSize> buffer);
+  void Flush();
+
+ private:
+  void ProcessReadQueue();
+
+  std::queue<PendingHidReport> pending_reports_;

[jracle (use Gerrit), 2014/06/11 11:20:04]

see above comments

+  std::queue<PendingHidRead> pending_reads_;
+};
+
 }  // namespace device
 
 #endif  // DEVICE_HID_HID_CONNECTION_H_