Quirks Client for downloading and providing display profiles

components/quirks/quirks_manager.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/quirks_manager.h"
+
+#include <utility>
+
+#include "base/command_line.h"
+#include "base/files/file_util.h"
+#include "base/format_macros.h"
+#include "base/path_service.h"
+#include "base/rand_util.h"
+#include "base/strings/stringprintf.h"
+#include "base/task_runner_util.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/scoped_user_pref_update.h"
+#include "components/quirks/pref_names.h"
+#include "components/quirks/quirks_client.h"
+#include "components/quirks/switches.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "url/gurl.h"
+
+namespace quirks {
+
+namespace {
+
+QuirksManager* g_manager = nullptr;

[oshima, 2016/03/22 20:27:21]

g_ is used typically for glocal, and I usually avoid  for non global (this is
file scoped).

[Greg Levin, 2016/03/23 00:16:18]

I'll fix this if you have a strong preference, but to me this seems clearer.  It
being in an anonymous namespace makes it clear that it's only file-global, and
the style guide says to use g_ to distinguish from local variables (which it's
not (?)).  Further, there are numerous examples of this usage in the Chrome
code.

[oshima, 2016/03/23 02:13:27]

This one (https://google.github.io/styleguide/cppguide.html#Variable_Names) is
for global variable (and this is not), and there is no suggestion for file
scoped variable.

One issue is that it gives wrong impress that that's global, even though it's
not in global namespace. For example:

file a:
namespace {
g_something
}

file b:
namespace {
g_something
}

are totally different variable, and it's less obvious when you're navigating /
searching for the code.
The benefit of having g_ only for global namespace is that it'll be unique. (or
link will fail)
I know, and I'm suggesting to avoid it for the above reason when I saw it.
It's not big deal, so if you want to keep it, then fine.

[Greg Levin, 2016/03/23 16:09:19]

Well, I wanted *something* to distinguish it from a local var, so I went with
manager_, as per the example of |detail_| here:
https://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Move-s...

+
+const char kIccExtension[] = ".icc";
+
+// How often we query Quirks Server.
+const int kDaysBetweenServerChecks = 30;
+
+// 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;
+}
+
+base::FilePath CheckForIccFile(base::FilePath built_in_path,
+                               base::FilePath download_path) {
+  // 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.
+  if (CheckAndLogFile(built_in_path))
+    return built_in_path;
+
+  // If experimental Quirks flag isn't set, no other icc file is available.
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableQuirksClient)) {
+    VLOG(1) << "Quirks Client disabled, no built-in icc file available.";
+    return base::FilePath();
+  }
+
+  // Check if QuirksClient has already downloaded icc file from server.
+  if (CheckAndLogFile(download_path))
+    return download_path;
+
+  return base::FilePath();
+}
+
+}  // namespace
+
+std::string IdToHexString(int64_t product_id) {
+  return base::StringPrintf("%08" PRIx64, product_id);
+}
+
+std::string IdToFileName(int64_t product_id) {
+  return IdToHexString(product_id).append(kIccExtension);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// QuirksManager
+
+QuirksManager::QuirksManager(
+    scoped_ptr<Delegate> delegate,
+    scoped_refptr<base::SequencedWorkerPool> blocking_pool,
+    PrefService* local_state,
+    scoped_refptr<net::URLRequestContextGetter> url_context_getter)
+    : waiting_for_login_(true),
+      delegate_(std::move(delegate)),
+      blocking_pool_(blocking_pool),
+      local_state_(local_state),
+      url_context_getter_(url_context_getter),
+      weak_ptr_factory_(this) {}
+
+QuirksManager::~QuirksManager() {
+  clients_.clear();
+  g_manager = nullptr;
+}
+
+// static
+void QuirksManager::Initialize(
+    scoped_ptr<Delegate> delegate,
+    scoped_refptr<base::SequencedWorkerPool> blocking_pool,
+    PrefService* local_state,
+    scoped_refptr<net::URLRequestContextGetter> url_context_getter) {
+  g_manager = new QuirksManager(std::move(delegate), blocking_pool, local_state,
+                                url_context_getter);
+}
+
+// static
+void QuirksManager::Shutdown() {
+  delete g_manager;
+}
+
+// static
+QuirksManager* QuirksManager::Get() {
+  DCHECK(g_manager);
+  return g_manager;
+}
+
+// static
+void QuirksManager::RegisterPrefs(PrefRegistrySimple* registry) {
+  registry->RegisterDictionaryPref(prefs::kQuirksClientLastServerCheck);
+}
+
+// Delay downloads until after login, to ensure that device policy has been set.
+void QuirksManager::OnLoginCompleted() {
+  if (!waiting_for_login_)
+    return;
+
+  waiting_for_login_ = false;
+  for (const scoped_ptr<QuirksClient>& client : clients_)
+    client->StartDownload();
+}
+
+void QuirksManager::RequestIccProfilePath(
+    int64_t product_id,
+    const RequestFinishedCallback& on_request_finished) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  std::string name = IdToFileName(product_id);
+  base::PostTaskAndReplyWithResult(
+      blocking_pool_.get(), FROM_HERE,
+      base::Bind(&CheckForIccFile,
+                 delegate_->GetBuiltInDisplayProfileDirectory().Append(name),
+                 delegate_->GetDownloadDisplayProfileDirectory().Append(name)),
+      base::Bind(&QuirksManager::OnIccFilePathRequestCompleted,
+                 weak_ptr_factory_.GetWeakPtr(), product_id,
+                 on_request_finished));
+}
+
+void QuirksManager::ClientFinished(QuirksClient* client) {

[oshima, 2016/03/22 20:27:21]

DCHECK(thread_checker_.CalledOnValidThread());

?

[Greg Levin, 2016/03/23 00:16:18]

Done.

+  SetLastServerCheck(client->product_id(), base::Time::Now());
+  auto it = std::find_if(clients_.begin(), clients_.end(),
+                         [client](const scoped_ptr<QuirksClient>& c) {
+                           return c.get() == client;
+                         });
+  CHECK(it != clients_.end());
+  clients_.erase(it);
+}
+
+scoped_ptr<net::URLFetcher> QuirksManager::CreateURLFetcher(
+    const GURL& url,
+    net::URLFetcherDelegate* delegate) {
+  if (!fake_quirks_fetcher_creator_.is_null())
+    return fake_quirks_fetcher_creator_.Run(url, delegate);
+
+  return net::URLFetcher::Create(url, net::URLFetcher::GET, delegate);
+}
+
+void QuirksManager::OnIccFilePathRequestCompleted(
+    int64_t product_id,
+    const RequestFinishedCallback& on_request_finished,
+    base::FilePath path) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  // If we found a file or client is disabled, inform requester.
+  if (!path.empty() ||
+      !base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableQuirksClient)) {
+    on_request_finished.Run(path, false);
+    // TODO(glevin): If Quirks files are ever modified on the server, we'll need
+    // to modify this logic to check for updates. See crbug.com/595024.
+    return;
+  }
+
+  double last_check = 0.0;
+  local_state_->GetDictionary(prefs::kQuirksClientLastServerCheck)
+      ->GetDouble(IdToHexString(product_id), &last_check);
+
+  // If never checked server before, need to check for new device.
+  if (last_check == 0.0) {
+    delegate_->GetDaysSinceOobe(base::Bind(
+        &QuirksManager::OnDaysSinceOobeReceived, weak_ptr_factory_.GetWeakPtr(),
+        product_id, on_request_finished));
+    return;
+  }
+
+  const base::TimeDelta time_since =
+      base::Time::Now() - base::Time::FromDoubleT(last_check);
+
+  // Don't need server check if we've checked within last 30 days.
+  if (time_since < base::TimeDelta::FromDays(kDaysBetweenServerChecks)) {
+    VLOG(2) << time_since.InDays()
+            << " days since last Quirks Server check for display "
+            << IdToHexString(product_id);
+    on_request_finished.Run(base::FilePath(), false);
+    return;
+  }
+
+  CreateClient(product_id, on_request_finished);
+}
+
+void QuirksManager::OnDaysSinceOobeReceived(
+    int64_t product_id,
+    const RequestFinishedCallback& on_request_finished,
+    int days_since_oobe) {
+  // On newer devices, we want to check server immediately (after OOBE/login).
+  if (days_since_oobe <= kDaysBetweenServerChecks) {
+    CreateClient(product_id, on_request_finished);
+    return;
+  }
+
+  // Otherwise, for the first check on an older device, we want to stagger
+  // it over 30 days, so artificially set last check accordingly.
+  // TODO(glevin): I believe that it makes sense to remove this random delay
+  // in the next Chrome release.
+  const int rand_days = base::RandInt(0, kDaysBetweenServerChecks);
+  const base::Time fake_last_check =
+      base::Time::Now() - base::TimeDelta::FromDays(rand_days);
+  SetLastServerCheck(product_id, fake_last_check);
+  VLOG(2) << "Delaying first Quirks Server check by "
+          << kDaysBetweenServerChecks - rand_days << " days.";
+
+  on_request_finished.Run(base::FilePath(), false);
+}
+
+void QuirksManager::CreateClient(
+    int64_t product_id,
+    const RequestFinishedCallback& on_request_finished) {

[oshima, 2016/03/22 20:27:21]

DCHECK(thread_checker_.CalledOnValidThread());

?

[Greg Levin, 2016/03/23 00:16:18]

Done.

+  QuirksClient* client =
+      new QuirksClient(product_id, on_request_finished, this);
+  clients_.insert(make_scoped_ptr(client));
+  if (!waiting_for_login_)
+    client->StartDownload();
+  else
+    VLOG(2) << "Quirks Client created; waiting for login to begin download.";
+}
+
+void QuirksManager::SetLastServerCheck(int64_t product_id,
+                                       const base::Time& last_check) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DictionaryPrefUpdate dict(local_state_, prefs::kQuirksClientLastServerCheck);
+  dict->SetDouble(IdToHexString(product_id), last_check.ToDoubleT());
+}
+
+}  // namespace quirks