[chrome.dial] Adds chrome.dial.fetchDeviceDecription API.

chrome/browser/extensions/api/dial/dial_api.h

Index: chrome/browser/extensions/api/dial/dial_api.h
diff --git a/chrome/browser/extensions/api/dial/dial_api.h b/chrome/browser/extensions/api/dial/dial_api.h
index 3182af4b377f0446528c0dc3cd5e4c3781a78a90..770b876c67eab8cc5e8541b479588593d18b5209 100644
--- a/chrome/browser/extensions/api/dial/dial_api.h
+++ b/chrome/browser/extensions/api/dial/dial_api.h
@@ -10,6 +10,7 @@
 #include "base/macros.h"
 #include "chrome/browser/extensions/api/dial/dial_device_data.h"
 #include "chrome/browser/extensions/api/dial/dial_registry.h"
+#include "chrome/common/extensions/api/dial.h"
 #include "components/keyed_service/core/refcounted_keyed_service.h"
 #include "extensions/browser/api/async_api_function.h"
 #include "extensions/browser/event_router.h"
@@ -18,10 +19,12 @@ namespace extensions {
 
 namespace api {
 namespace dial {
-class DialRegistry;
+class DeviceDescriptionFetcher;
 }  // namespace dial
 }  // namespace api
 
+class DialFetchDeviceDescriptionFunction;
+
 // Dial API which is a ref-counted KeyedService that manages
 // the DIAL registry. It takes care of creating the registry on the IO thread
 // and is an observer of the registry. It makes sure devices events are sent out
@@ -30,6 +33,13 @@ class DialRegistry;
 // TODO(mfoltz): This should probably inherit from BrowserContextKeyedAPI
 // instead; ShutdownOnUIThread below is a no-op, which is the whole point of
 // RefcountedKeyedService.
+//
+// TODO(mfoltz): The threading model for this API needs to be rethought.  At a
+// minimum, DialRegistry should move to the UI thread to avoid extra thread hops
+// here, allow a straightforward GetDeviceList implementation
+// (crbug.com/576817), cleanup some long-tail crashes (crbug.com/640011) that
+// are likely due to lifetime issues, and simplify unit tests
+// (crbug.com/661457).
 class DialAPI : public RefcountedKeyedService,
                 public EventRouter::Observer,
                 public api::dial::DialRegistry::Observer {
@@ -45,9 +55,16 @@ class DialAPI : public RefcountedKeyedService,
   void SendEventOnUIThread(const api::dial::DialRegistry::DeviceList& devices);
   void SendErrorOnUIThread(const api::dial::DialRegistry::DialErrorCode type);
 
+  // Sets test device data.  For tests only.

[Wez, 2017/01/05 22:29:24]

nit: Suggest removing the "For tests only", since that is implicit from context
& naming :)

[mark a. foltz, 2017/01/09 21:38:07]

Done.

+  void SetDeviceForTest(
+      const api::dial::DialDeviceData& device_data,
+      const api::dial::DialDeviceDescriptionData& device_description);
+
  private:
   ~DialAPI() override;
 
+  friend class DialFetchDeviceDescriptionFunction;
+
   // RefcountedKeyedService:
   void ShutdownOnUIThread() override;
 
@@ -70,6 +87,11 @@ class DialAPI : public RefcountedKeyedService,
   // Created lazily on first access on the IO thread.
   std::unique_ptr<api::dial::DialRegistry> dial_registry_;
 
+  // Device data for testing.
+  std::unique_ptr<api::dial::DialDeviceData> test_device_data_;
+  std::unique_ptr<api::dial::DialDeviceDescriptionData>
+      test_device_description_;

[Wez, 2017/01/05 22:29:24]

Rather than have these fields and having to friend the FetchDescriptionFunction,
have you considered providing a SetDialRegistryForTest() on DialAPI?  Though
perhaps that would mean a load more hoop-jumping to feed in the device
description for the test, since you'd need to intercept URLRequests to the test
device to return the test description data?

[mark a. foltz, 2017/01/09 21:38:07]

I like that idea.  I'd rather take it on as part of fixing the threading of
DialRegistry and cleaning up that class in general.  Updated TODO in
dial_api.cc.

[Wez, 2017/01/09 22:12:58]

Acknowledged.

+
   DISALLOW_COPY_AND_ASSIGN(DialAPI);
 };
 
@@ -103,6 +125,34 @@ class DialDiscoverNowFunction : public AsyncApiFunction {
   DISALLOW_COPY_AND_ASSIGN(DialDiscoverNowFunction);
 };
 
+class DialFetchDeviceDescriptionFunction : public AsyncExtensionFunction {
+ public:
+  DialFetchDeviceDescriptionFunction();
+
+ protected:
+  ~DialFetchDeviceDescriptionFunction() override;
+
+  // AsyncExtensionFunction:
+  bool RunAsync() override;
+
+ private:
+  DECLARE_EXTENSION_FUNCTION("dial.fetchDeviceDescription",
+                             DIAL_FETCHDEVICEDESCRIPTION)
+
+  void GetDeviceDescriptionUrlOnIOThread(const std::string& label);
+  void MaybeStartFetch(const GURL& url);
+  void OnFetchComplete(const api::dial::DialDeviceDescriptionData& result);
+  void OnFetchError(const std::string& result);
+
+  std::unique_ptr<api::dial::FetchDeviceDescription::Params> params_;
+  std::unique_ptr<api::dial::DeviceDescriptionFetcher>
+      device_description_fetcher_;
+
+  DialAPI* dial_;
+
+  DISALLOW_COPY_AND_ASSIGN(DialFetchDeviceDescriptionFunction);
+};
+
 }  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_API_DIAL_DIAL_API_H_