Quirks Client for downloading and providing display profiles

components/quirks_client/quirks_client.cc

+// 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/path_service.h"
+#include "base/strings/stringprintf.h"
+#include "base/task_runner_util.h"
+#include "base/thread_task_runner_handle.h"
+#include "chromeos/chromeos_paths.h"
+#include "components/prefs/scoped_user_pref_update.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 unsigned kMaxRetrySleepSeconds = 6 * 3600;  // 6 hours
+
+// Access to singleton QuirksClientManager for static and nonmember funcitons.
+QuirksClientManager* GetManager() {
+  return QuirksClientManager::Get();
+}
+
+// Check if file exists, VLOG results.
+bool CheckAndLogFile(const base::FilePath& path) {
+  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
+
+std::string IdToHexString(int64_t product_id) {
+  return base::StringPrintf("%08" PRIx64, product_id);
+}
+
+// Must be called on file thread.
+base::FilePath RequestIccProfilePath(
+    int64_t product_id,
+    const 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 (!QuirksClient::IsEnabled()) {
+    VLOG(1) << "Quirks Client disabled, no built-in icc file available.";
+    return base::FilePath();
+  }
+
+  if (!GetManager()) {

[Bernhard Bauer, 2016/02/12 14:51:21]

Wait, you call this method on a background thread? That seems very dangerous,
concurrency-wise. How do you know that the object won't be destroyed under you
on its origin thread?

I think you will have to go through the QuirksClientManager first (on the UI
thread), grab everything you need there, and then post a task to the background
thread to do the file access and if necessary post a task back to the origin
thread. In general, making objects operate on one single thread (or sequence)
and doing background stuff exclusively using static methods is a very useful
pattern if you want to avoid these kinds of issues.

An alternative might be to use locking, but that's a) not quite as encouraged in
Chrome as single-threadedness, and b) would also require a bit of refactoring to
be able to do everything under a single lock.

[Greg Levin, 2016/02/16 03:14:46]

Keeping the manager on one thread is tricky, as it needs info from both ui and
file threads that is only available once it has a product id, which it doesn't
have at initialization time.  If the file already exists, the path must be
returned immediately, and can only be provided by the delegate object.  I
*could* cache that path as a file-global at startup, so that code from deletable
objects would only run in posted tasks.

I know this only mitigates the problem and doesn't solve it, but I've refactored
to limit the concurrency danger by moving most of this function into a method,
and only using 

  if (g_manager)
    return g_manager->RequestIccFilePath()

to access it.

+    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 =
+      GetManager()->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.
+
+  GetManager()->RunClient(product_id, on_download_finished);
+
+  return base::FilePath();
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// QuirksClient
+
+QuirksClient::QuirksClient(int64_t product_id,
+                           const DownloadFinishedCallback& on_download_finished,
+                           QuirksClientManager* manager)
+    : product_id_(product_id),
+      on_download_finished_(on_download_finished),
+      manager_(manager),
+      icc_path_(manager->delegate()->GetDisplayProfileDirectory().Append(
+          IdToHexString(product_id) + ".icc")),
+      request_reason_(FIRST),
+      retries_(0),
+      weak_ptr_factory_(this) {}
+
+QuirksClient::~QuirksClient() {}
+
+void QuirksClient::StartDownload() {
+  DCHECK(base::ThreadTaskRunnerHandle::IsSet());
+
+  // 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_ = manager_->CreateURLFetcher(url, 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();
+}
+
+// static
+bool QuirksClient::IsEnabled() {
+  return base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableDisplayQuirksClient);
+}
+
+void QuirksClient::OnURLFetchComplete(const net::URLFetcher* source) {
+  DCHECK(base::ThreadTaskRunnerHandle::IsSet());
+  DCHECK_EQ(url_fetcher_.get(), source);
+
+  const int HTTP_INTERNAL_SERVER_ERROR_LAST =
+      net::HTTP_INTERNAL_SERVER_ERROR + 99;
+  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 <= HTTP_INTERNAL_SERVER_ERROR_LAST);
+
+  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(false);
+    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 file thread.
+  std::string data;
+  if (!ParseResult(response, &data))

[Bernhard Bauer, 2016/02/12 14:51:21]

So if we can't parse the result, we'll just never finish?

[Greg Levin, 2016/02/15 21:53:52]

Done (added Shutdown()).

+    return;
+
+  base::PostTaskAndReplyWithResult(
+      manager_->blocking_pool(), FROM_HERE,
+      base::Bind(&WriteIccFile, icc_path_, data),
+      base::Bind(&QuirksClient::Shutdown, weak_ptr_factory_.GetWeakPtr()));
+}
+
+void QuirksClient::Shutdown(bool success) {
+  DCHECK(base::ThreadTaskRunnerHandle::IsSet());
+  if (!on_download_finished_.is_null())
+    on_download_finished_.Run(success ? icc_path_ : base::FilePath());
+
+  manager_->SetLastServerCheck(product_id_, base::Time::Now());
+  manager_->DeleteClient(this);
+}
+
+void QuirksClient::Retry() {
+  DCHECK(base::ThreadTaskRunnerHandle::IsSet());
+  ++retries_;
+
+  const unsigned delay_seconds =
+      std::min(retries_ * retries_ * kRetrySleepSeconds, kMaxRetrySleepSeconds);
+  const base::TimeDelta delay = base::TimeDelta::FromSeconds(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(GetManager() &&
+         GetManager()->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);
+}
+
+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