[Media Router] Add DialMediaSinkService and DeviceDescriptionService

chrome/browser/media/router/discovery/dial/device_description_service.h

+// Copyright 2017 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.
+
+#ifndef CHROME_BROWSER_MEDIA_ROUTER_DISCOVERY_DIAL_DEVICE_DESCRIPTION_SERVICE_H_
+#define CHROME_BROWSER_MEDIA_ROUTER_DISCOVERY_DIAL_DEVICE_DESCRIPTION_SERVICE_H_
+
+#include <memory>
+#include <string>
+
+#include "base/callback.h"
+#include "base/threading/thread_checker.h"
+#include "chrome/browser/media/router/discovery/dial/dial_device_data.h"
+#include "chrome/browser/media/router/discovery/dial/parsed_dial_device_description.h"
+#include "chrome/common/media_router/dial_device_description_parser.mojom.h"
+
+namespace net {
+class URLRequestContextGetter;
+}
+
+namespace media_router {
+
+class DeviceDescriptionFetcher;
+
+// Represents cached device description data parsed from device description xml.
+struct CachedParsedDialDeviceDescription {

[mark a. foltz, 2017/04/12 00:17:17]

Creating this as a private struct below, e.g.
DeviceDescriptionService::CacheEntry, might save some typing.  I don't feel
strongly though.

[zhaobin, 2017/04/18 06:58:26]

Done.

+  CachedParsedDialDeviceDescription();
+  CachedParsedDialDeviceDescription(
+      const CachedParsedDialDeviceDescription& other);
+  ~CachedParsedDialDeviceDescription();
+
+  // The time the description was fetched, in ms after the epoch.
+  base::Time fetch_time_millis;

[imcheng, 2017/04/12 19:33:50]

Omit the millis / ms part in the name / comments since the unit is encapsulated
within base::Time? Here and below.

[zhaobin, 2017/04/18 06:58:27]

Done.

+
+  // The expiration time from the cache, in ms after the epoch.
+  base::Time expire_time_millis;
+
+  // The version number.
+  base::Optional<int32_t> config_id;

[mark a. foltz, 2017/04/12 00:17:18]

It might be simpler to provide a default of -1 vs. using base::Optional, as
valid config_id values are non-negative.  I don't feel strongly though.

[zhaobin, 2017/04/18 06:58:26]

Done.

+
+  // Underlying device description data from xml

[mark a. foltz, 2017/04/12 00:17:18]

s/xml/XML./

[zhaobin, 2017/04/18 06:58:26]

Done.

+  ParsedDialDeviceDescription description_data;
+};
+
+class DeviceDescriptionService {

[imcheng, 2017/04/12 19:33:50]

Please document threading assumptions when providing documentation for this
class.

[zhaobin, 2017/04/18 06:58:26]

Done.

+ public:
+  // Called if parsing device description xml in utility process succeeds, and

[mark a. foltz, 2017/04/12 00:17:18]

Nit: s/xml/XML/ throughout as it's an abbreviation

[zhaobin, 2017/04/18 06:58:26]

Done.

+  // all fields are valid.
+  // |device_data|: The device to look up.
+  // |description_data|: Device description data from device description xml.
+  using DeviceDescriptionParseSuccessCallback =
+      base::Callback<void(const DialDeviceData& device_data,
+                          const ParsedDialDeviceDescription& description_data)>;
+
+  // Called if parsing device description xml in utility process fails, or some
+  // parsed fields are missing or invalid.
+  using DeviceDescriptionParseErrorCallback =
+      base::Callback<void(const std::string& device_label,

[imcheng, 2017/04/12 19:33:50]

Why does this only return the device label only vs. DialDeviceData itself in the
success callback, or vice versa?

[zhaobin, 2017/04/18 06:58:26]

Done.

+                          const std::string& error_message)>;
+
+  DeviceDescriptionService(DeviceDescriptionParseSuccessCallback success_cb,

[imcheng, 2017/04/12 19:33:50]

const ref for both args

[zhaobin, 2017/04/18 06:58:26]

Done.

+                           DeviceDescriptionParseErrorCallback error_cb);
+  virtual ~DeviceDescriptionService();
+
+  // For each device in |devices|, if there is a valid cache entry for it, call
+  // |success_cb_| with cached device description; otherwise start fetching
+  // device description xml and parsing xml in utility process. Call
+  // |success_cb_| if both fetching and parsing succeeds; otherwise call
+  // |error_cb_|.
+  virtual void GetDeviceDescriptions(
+      const std::vector<DialDeviceData>& devices,
+      net::URLRequestContextGetter* request_context);

[mark a. foltz, 2017/04/12 00:17:18]

Document |request_context|

[zhaobin, 2017/04/18 06:58:26]

Done.

+
+ private:
+  // Issues a HTTP GET request for the device description. No-op if there is
+  // already a pending request.
+  // |device_data|: The device to look up.
+  void FetchDeviceDescription(const DialDeviceData& device_data,
+                              net::URLRequestContextGetter* request_context);
+
+  // Checks the cache for a valid device description. If the entry is found but
+  // is no longer valid, it is removed from the cache.
+  // |device_data|: The device to look up.
+  // |out_description|: Cached entry of parsed device description. Returns
+  // nullptr if cache entry does not exist or is not valid.
+  bool CheckAndUpdateCache(const DialDeviceData& device_data,
+                           CachedParsedDialDeviceDescription* out_description);
+
+  // Invoked when SafeDialDeviceDescriptionParser finishes parsing device
+  // description xml.
+  // |device_data|: Device data initiating xml parsing in utility process.
+  // |app_url|: The app Url.
+  // |device_description_ptr|: Parsed devcie description from utility process.

[mark a. foltz, 2017/04/12 00:17:17]

typo in device

[zhaobin, 2017/04/18 06:58:26]

Done.

+  // Returns nullptr if parsing fails.
+  void OnParsedDeviceDescription(
+      const DialDeviceData& device_data,
+      const GURL& app_url,
+      chrome::mojom::DialDeviceDescriptionPtr device_description_ptr);
+
+  // Invoked when HTTP GET request finishes.

[mark a. foltz, 2017/04/12 00:17:18]

Maybe this (and the following method) should be grouped below
FetchDeviceDescription.

[zhaobin, 2017/04/18 06:58:27]

Done.

+  // |device_data|: Device data initiating the HTTP request.
+  // |description_data|: Response from HTTP request.
+  void OnDeviceDescriptionFetchComplete(
+      const DialDeviceData& device_data,
+      const DialDeviceDescriptionData& description_data);
+
+  // Invoked when HTTP GET request fails.
+  // |device_data|: Device data initiating the HTTP request.
+  void OnDeviceDescriptionFetchError(const DialDeviceData& device_data,
+                                     const std::string& error_message);
+
+  // Map of <device label, DeviceDescriptionFetcher>
+  std::map<std::string, std::unique_ptr<DeviceDescriptionFetcher>>
+      device_description_fetcher_map_;
+
+  // Map of <device label, CachedParsedDialDeviceDescription>
+  std::map<std::string, CachedParsedDialDeviceDescription> description_map_;
+
+  // See comments for DeviceDescriptionParseSuccessCallback.
+  DeviceDescriptionParseSuccessCallback success_cb_;
+
+  // See comments for DeviceDescriptionParseErrorCallback.
+  DeviceDescriptionParseErrorCallback error_cb_;
+
+  base::ThreadChecker thread_checker_;
+};
+
+}  // namespace media_router
+
+#endif  // CHROME_BROWSER_MEDIA_ROUTER_DISCOVERY_DIAL_DEVICE_DESCRIPTION_SERVICE_H_