Quirks Client for downloading and providing display profiles

components/quirks_client/quirks_client.h

+// Copyright (c) 2015 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 COMPONENTS_QUIRKS_CLIENT_QUIRKS_CLIENT_H_
+#define COMPONENTS_QUIRKS_CLIENT_QUIRKS_CLIENT_H_
+
+#include "base/files/file_path.h"
+#include "base/macros.h"
+#include "base/time/time.h"
+#include "base/timer/timer.h"
+#include "components/quirks_client/quirks_client_export.h"
+#include "net/url_request/url_fetcher_delegate.h"
+
+class PrefRegistrySimple;
+class PrefService;
+
+namespace base {
+class SequencedWorkerPool;
+class MessageLoopForUI;
+}
+
+namespace net {
+class URLRequestContextGetter;
+}
+
+namespace quirks_client {
+
+// Handles providing data from icc and other display data files.  This may
+// involve downloading and storing files from the Quirks Server, or serving
+// these files from local, writable storage.
+class QUIRKS_CLIENT_EXPORT QuirksClient : public net::URLFetcherDelegate {
+ public:
+  typedef base::Callback<void(base::FilePath)> DownloadFinishedCallback;
+
+  enum RequestReason {
+    FIRST,  // Device's first server request.
+    RETRY,  // Has server file been added since last check?
+    UPDATE  // Has server file been updated since last check?
+  };
+
+  QuirksClient(int64_t product_id,
+               DownloadFinishedCallback* on_download_finished);
+  ~QuirksClient() override;
+
+  // Returns path to icc file, if it exists; creates and starts QuirksServer if
+  // it doesn't.
+  static base::FilePath RequestIccProfilePath(
+      int64_t product_id,
+      DownloadFinishedCallback* on_download_finished);
+
+  // Start download.
+  void Start();

[oshima, 2016/01/28 20:16:31]

or StartDownload() and remove comment.

[Greg Levin, 2016/02/01 23:17:44]

Done.

+
+  // TODO(glevin): Copied from wallpaper downloader.  Are they needed?  TBD.
+  // This is called in tests to modify (lower) retry delay.

[stevenjb, 2016/01/28 21:05:58]

If we don't use these yet we should remove them until we do (but it would be
preferable to have at least basic tests with the initial CL).

[Greg Levin, 2016/02/01 23:17:44]

Done.

+  void set_retry_delay_for_testing(base::TimeDelta value) {
+    retry_delay_ = value;
+  }
+
+  base::TimeDelta retry_current_delay_for_testing() const {
+    return retry_current_delay_;
+  }
+
+ private:
+  // Self deletion when done.
+  void Shutdown();
+
+  // Schedules retry.
+  void Retry();
+
+  // net::URLFetcherDelegate:
+  void OnURLFetchComplete(const net::URLFetcher* source) override;
+
+  // Write |data| to |file_path|.
+  static bool WriteIccFile(const base::FilePath file_path,
+                           const std::string& data);
+
+  // Callback after icc write is finished.
+  void OnWriteIccFileFinished(bool success);
+
+  // Translate json with base64-encoded data (|result|) into raw |data|.
+  bool ParseResult(const std::string& result, std::string* data);
+
+  // ID of display to request from Quirks Server.
+  const int64_t product_id_;
+
+  // Callback supplied by caller.
+  const DownloadFinishedCallback* on_download_finished_;
+
+  // Full path to icc file.
+  const base::FilePath icc_path_;
+
+  // This fetcher is used to download icc file.
+  scoped_ptr<net::URLFetcher> url_fetcher_;
+
+  // Why are we making this server call?
+  RequestReason request_reason_;
+
+  // Pending retry.
+  base::OneShotTimer request_scheduled_;
+
+  // Number of download retries (first attempt is not counted as retry).
+  size_t retries_;
+
+  // TODO(glevin): Copied from wallpaper downloader.  Are they needed?  TBD.
+  // Sleep between retry requests (increasing, see Retry() method for details).
+  // Non-constant value for tests.
+  base::TimeDelta retry_delay_;
+
+  // Retry delay of the last attempt. For testing only.
+  base::TimeDelta retry_current_delay_;
+
+  // Factory for callbacks.
+  base::WeakPtrFactory<QuirksClient> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(QuirksClient);
+};
+
+// Handles needed components from browser (local preferences, url context
+// getter, message loops).
+class QUIRKS_CLIENT_EXPORT QuirksClientManager {

[oshima, 2016/01/28 20:16:31]

define this in a separate file.

[Greg Levin, 2016/02/01 23:17:43]

Done.

+ public:
+  // Delegate class, so implementation can access browser functionality.
+  class Delegate {
+   public:
+    virtual ~Delegate() {};
+    virtual std::string api_key_quirks() = 0;

[oshima, 2016/01/28 20:16:31]

GetApiKeyQuirks()

[Greg Levin, 2016/02/01 23:17:44]

Done.

+    virtual base::FilePath GetDisplayProfileDirectory() = 0;
+    virtual int GetDaysSinceOobe() = 0;

[oshima, 2016/01/28 20:16:31]

can/should these be const?(just asking)

[Greg Levin, 2016/02/01 23:17:44]

Done.

+  };

[stevenjb, 2016/01/28 21:05:59]

+ private: DISALLOW_ASSIGN

[Greg Levin, 2016/02/01 23:17:44]

Is this needed / desirable in a pure virtual class?  I added it to
QuirksClientDelegateImpl... is that sufficient?

+
+  QuirksClientManager(Delegate* delegate,
+                      base::MessageLoopForUI* message_loop_ui,
+                      base::SequencedWorkerPool* blocking_pool,
+                      PrefService* local_state,
+                      net::URLRequestContextGetter* url_context_getter);
+  virtual ~QuirksClientManager();
+
+  static void Initialize(QuirksClientManager* manager);
+  static void Shutdown();
+  static void RegisterPrefs(PrefRegistrySimple* registry);
+
+  void RunClient(int64_t product_id,
+                 QuirksClient::DownloadFinishedCallback* on_download_finished);
+
+  base::Time GetLastServerCheck(int64_t product_id);
+  void SetLastServerCheck(int64_t product_id, base::Time last_check);

[oshima, 2016/01/28 20:16:31]

const base::Time&

[Greg Levin, 2016/02/01 23:17:44]

Done.

+  void RecordReasonUmaStat(QuirksClient::RequestReason reason);
+  void RecordFileFoundUmaStat(bool success);
+
+  Delegate* delegate() { return delegate_; }
+  base::MessageLoopForUI* message_loop_ui() { return message_loop_ui_; }
+  base::SequencedWorkerPool* blocking_pool() { return blocking_pool_; }
+  net::URLRequestContextGetter* url_context_getter() {
+    return url_context_getter_.get();
+  }
+
+  // Check that ui thread and message loop are ready.
+  bool Validate();
+
+ private:
+  // browser/ui thread components needed for client.
+  Delegate* delegate_;                        // Impl runs from browser.
+  base::MessageLoopForUI* message_loop_ui_;   // To run QC on ui thread.

[oshima, 2016/01/28 20:16:31]

do you need this? can't you just use ::current() ?

[Greg Levin, 2016/02/01 23:17:44]

Do you mean base::MessageLoop::current()?  From this call stack:

ParseDisplayProfile() (in display_color_manager_chromeos.cc)
  QuirksClient::RequestIccProfilePath()
    QuirksClientManager::RunClient()

I call
  message_loop_ui_->PostTask(...RunClientOnUIThread...)

All the above is run on the io thread, and from there,
base::MessageLoop::current() returns NULL.

+  base::SequencedWorkerPool* blocking_pool_;  // For url getter and file io.
+  PrefService* local_state_;                  // For local prefs.
+  bool is_new_device_;                        // Is within 30 days of OOBE?
+  // For URLFetcher.
+  scoped_refptr<net::URLRequestContextGetter> url_context_getter_;
+
+  DISALLOW_COPY_AND_ASSIGN(QuirksClientManager);
+};
+
+}  // namespace chromeos
+
+#endif  // COMPONENTS_QUIRKS_CLIENT_QUIRKS_CLIENT_H_