Quirks Client for downloading and providing display profiles

components/quirks_client/quirks_client.cc

Index: components/quirks_client/quirks_client.cc
diff --git a/components/quirks_client/quirks_client.cc b/components/quirks_client/quirks_client.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0e7c77c23e274f674fd2ffe7f36b659a3145a4ca
--- /dev/null
+++ b/components/quirks_client/quirks_client.cc
@@ -0,0 +1,255 @@
+// Copyright 2016 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.
+
+#include "components/quirks_client/quirks_client.h"
+
+#include "base/base64.h"
+#include "base/command_line.h"
+#include "base/files/file_util.h"
+#include "base/format_macros.h"
+#include "base/json/json_reader.h"
+#include "base/message_loop/message_loop.h"
+#include "base/path_service.h"
+#include "base/prefs/scoped_user_pref_update.h"
+#include "base/strings/stringprintf.h"
+#include "base/task_runner_util.h"
+#include "chromeos/chromeos_paths.h"
+#include "components/quirks_client/quirks_client_manager.h"
+#include "components/quirks_client/switches.h"
+#include "net/base/load_flags.h"
+#include "net/http/http_status_code.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_request_context_getter.h"
+
+namespace quirks_client {
+
+namespace {
+
+const char kQuirksUrlFormat[] =
+    "https://qa-quirksserver-pa.sandbox.googleapis.com/v2/display/%08" PRIx64
+    "/clients/chromeos/M%d";
+
+// Sleep between Quirks retries (will be multiplied by squared retry number).
+const unsigned kRetrySleepSeconds = 10;
+
+// Retry is infinite with increasing intervals. When calculated delay becomes
+// longer than maximum (kMaxRetrySleepSeconds) it is set to the maximum.
+const double kMaxRetrySleepSeconds = 6 * 3600;  // 6 hours
+
+// Access to singleton QuirksClientManager.
+QuirksClientManager* manager() {

[stevenjb, 2016/02/02 23:45:51]

Use CamelCase for functions, e.g. GetManager().

[Greg Levin, 2016/02/09 18:56:38]

Done.

+  return QuirksClientManager::Get();

[stevenjb, 2016/02/02 23:45:51]

This is potentially unsafe, see comments in q_c_m.cc.

[Greg Levin, 2016/02/09 18:56:38]

Acknowledged.

+}
+
+// Check if file exists, VLOG results.
+bool CheckAndLogFile(const base::FilePath& path) {
+  DCHECK(!manager() || manager()->blocking_pool()->RunsTasksOnCurrentThread());
+  const bool exists = base::PathExists(path);
+  VLOG(1) << (exists ? "File" : "No File") << " found at " << path.value();
+  // TODO(glevin): If file exists, do we want to implement a hash to verify that
+  // the file hasn't been corrupted or tampered with?
+  return exists;
+}
+
+}  // namespace
+
+////////////////////////////////////////////////////////////////////////////////
+// QuirksClient
+
+QuirksClient::QuirksClient(int64_t product_id,
+                           DownloadFinishedCallback* on_download_finished)
+    : product_id_(product_id),
+      on_download_finished_(on_download_finished),
+      icc_path_(manager()->delegate()->GetDisplayProfileDirectory().Append(
+          IdToHexString(product_id) + ".icc")),
+      request_reason_(FIRST),
+      retries_(0),
+      weak_ptr_factory_(this) {}
+
+QuirksClient::~QuirksClient() {}
+
+// static
+base::FilePath QuirksClient::RequestIccProfilePath(
+    int64_t product_id,
+    DownloadFinishedCallback* on_download_finished) {
+  std::string file_name = IdToHexString(product_id) + ".icc";
+
+  // First, look for icc file in old read-only location.  If there, we don't use
+  // the Quirks server.
+  // TODO(glevin): Awaiting final decision on how to handle old read-only files.
+  base::FilePath path;
+  CHECK(
+      PathService::Get(chromeos::DIR_DEVICE_COLOR_CALIBRATION_PROFILES, &path));
+  path = path.Append(file_name);
+  if (CheckAndLogFile(path))
+    return path;
+
+  // If Quirks Client is disabled, no other icc file is available.
+  if (!IsEnabled()) {
+    VLOG(1) << "Quirks Client disabled, no built-in icc file available.";
+    return base::FilePath();
+  }
+
+  if (!manager()) {
+    VLOG(1) << "Quirks Client Manager not initialized; can't start Client.";
+    return base::FilePath();
+  }
+
+  // Check if QuirksClient has already downloaded icc file from server.
+  path = manager()->delegate()->GetDisplayProfileDirectory().Append(file_name);
+  if (CheckAndLogFile(path))
+    return path;
+
+  // TODO(glevin): Eventually we'll want to check the server for updates to
+  // files, so we'll still want to get down here even if we find the icc file.
+
+  manager()->RunClient(product_id, on_download_finished);

[stevenjb, 2016/02/02 23:45:51]

It seems like this entire method should be part of QCM? Having a static QC
method call into QCM which then instantiates a QC instance is very confusing.

[Greg Levin, 2016/02/09 18:56:38]

I agree that the back-and-forth is confusing.  But in the way I've separated the
responsibilities of the two classes, this makes more sense.
  QC: Retrieve and provide display profiles.
  QCM: Track and manage QCs and provide thread/browser services.
Display code should ideally only interact with QC.  The only external (non-QC)
interaction with QCM should be creating/destroying it at startup/shutdown.  At
least, that's the model I've been using.

I suppose I could refactor and rename all of this with:
  QCM -> QuirksClient: Single persistent object that handles all external
interactions.
  QC -> QuirksClientWorker: Handles the busywork of downloading new profiles.
(Or possibly QCM -> QuirksProvider, and QC keeps old name, as it's still the
class that's talking to the Quirks Server.)
Under this model, I would move RequestIccProfilePath() and a few utilities to
(new) QC/Provider (formerly QCM).

Let's discuss in person, and I'll address this in the next patch.

+
+  return base::FilePath();
+}
+
+// static
+bool QuirksClient::IsEnabled() {
+  return base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableDisplayQuirksClient);
+}
+
+// static
+std::string QuirksClient::IdToHexString(int64_t product_id) {
+  return base::StringPrintf("%08" PRIx64, product_id);
+}
+
+void QuirksClient::StartDownload() {
+  DCHECK(base::MessageLoopForUI::IsCurrent());
+
+  // URL of icc file on Quirks Server.
+  // TODO(glevin): Replace |44| with actual version.  This is fine for now, as
+  // the Quirks Server is not currently using this value.
+  std::string url = base::StringPrintf(kQuirksUrlFormat, product_id_, 44);
+
+  VLOG(2) << "Preparing to download\n  " << url << "\nto file "
+          << icc_path_.value();
+
+  url += "?key=";
+  url += manager()->delegate()->GetApiKey();
+
+  url_fetcher_ = net::URLFetcher::Create(GURL(url), net::URLFetcher::GET, this);
+  url_fetcher_->SetRequestContext(manager()->url_context_getter());
+  url_fetcher_->SetLoadFlags(net::LOAD_BYPASS_CACHE | net::LOAD_DISABLE_CACHE |
+                             net::LOAD_DO_NOT_SAVE_COOKIES |
+                             net::LOAD_DO_NOT_SEND_COOKIES |
+                             net::LOAD_DO_NOT_SEND_AUTH_DATA);
+  url_fetcher_->Start();
+}
+
+void QuirksClient::OnURLFetchComplete(const net::URLFetcher* source) {
+  DCHECK(base::MessageLoopForUI::IsCurrent());
+  DCHECK_EQ(url_fetcher_.get(), source);
+
+  const net::URLRequestStatus status = source->GetStatus();
+  const int response_code = source->GetResponseCode();
+  const bool server_error =
+      !status.is_success() ||
+      (response_code >= net::HTTP_INTERNAL_SERVER_ERROR &&
+       response_code < (net::HTTP_INTERNAL_SERVER_ERROR + 100));
+
+  VLOG(2) << "QuirksClient::OnURLFetchComplete():"
+          << "  status=" << status.status()
+          << ",  response_code=" << response_code
+          << ",  server_error=" << server_error;
+
+  manager()->RecordReasonUmaStat(request_reason_);
+
+  if (response_code == net::HTTP_NOT_FOUND) {
+    VLOG(1) << IdToHexString(product_id_) << ".icc not found on Quirks server.";
+    manager()->RecordFileFoundUmaStat(false);
+    Shutdown();
+    return;
+  }
+
+  if (server_error) {
+    url_fetcher_.reset();
+    Retry();
+    return;
+  }
+
+  manager()->RecordFileFoundUmaStat(true);
+  std::string response;
+  url_fetcher_->GetResponseAsString(&response);
+  VLOG(2) << "Quirks server response:\n" << response;
+
+  // Parse response data and write to file on io thread.
+  std::string data;
+  if (ParseResult(response, &data)) {

[stevenjb, 2016/02/02 23:45:51]

nit: Invert and early exit

[Greg Levin, 2016/02/09 18:56:38]

Done.

+    base::PostTaskAndReplyWithResult(
+        manager()->blocking_pool(), FROM_HERE,
+        base::Bind(&WriteIccFile, icc_path_, data),
+        base::Bind(&QuirksClient::OnWriteIccFileFinished,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+}
+
+void QuirksClient::Shutdown() {
+  manager()->SetLastServerCheck(product_id_, base::Time::Now());
+  base::MessageLoopForUI::current()->DeleteSoon(FROM_HERE, this);
+}
+
+void QuirksClient::Retry() {
+  DCHECK(base::MessageLoopForUI::IsCurrent());
+  ++retries_;
+
+  const double delay_seconds =
+      std::min(static_cast<double>(retries_) * retries_ * kRetrySleepSeconds,
+               kMaxRetrySleepSeconds);
+  const base::TimeDelta delay = base::TimeDelta::FromSecondsD(delay_seconds);
+
+  VLOG(1) << "Schedule next Quirks download attempt in " << delay.InSecondsF()
+          << " seconds (retry = " << retries_ << ").";
+  request_scheduled_.Start(FROM_HERE, delay, this,
+                           &QuirksClient::StartDownload);
+}
+
+// static
+bool QuirksClient::WriteIccFile(const base::FilePath file_path,
+                                const std::string& data) {
+  DCHECK(manager()->blocking_pool()->RunsTasksOnCurrentThread());
+  int bytes_written = base::WriteFile(file_path, data.data(), data.length());
+  if (bytes_written == -1)
+    VLOG(1) << "Failed to write: " << file_path.value() << ", err = " << errno;
+  else
+    VLOG(1) << bytes_written << "bytes written to: " << file_path.value();
+
+  return (bytes_written != -1);
+}
+
+void QuirksClient::OnWriteIccFileFinished(bool success) {
+  DCHECK(base::MessageLoopForUI::IsCurrent());
+  if (success) {
+    if (on_download_finished_)
+      on_download_finished_->Run(icc_path_);
+  }
+  // If we got data from server but couldn't write it, that's not likely to
+  // change this session, so shut down regardless of success.
+  Shutdown();
+}
+
+bool QuirksClient::ParseResult(const std::string& result, std::string* data) {
+  std::string data64;
+  const base::DictionaryValue* dict;
+  scoped_ptr<base::Value> json = base::JSONReader::Read(result);
+  if (!json || !json->GetAsDictionary(&dict) ||
+      !dict->GetString("icc", &data64)) {
+    VLOG(1) << "Failed to parse JSON icc data";
+    return false;
+  }
+
+  if (!base::Base64Decode(data64, data)) {
+    VLOG(1) << "Failed to decode Base64 icc data";
+    return false;
+  }
+
+  return true;
+}
+
+}  // namespace chromeos