Count site data size for important sites

chrome/browser/engagement/important_sites_usage_counter.cc

+// 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.
+
+#include "chrome/browser/engagement/important_sites_usage_counter.h"
+
+#include <utility>
+
+#include "base/threading/thread_task_runner_handle.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/dom_storage_context.h"
+
+using ImportantDomainInfo = ImportantSitesUtil::ImportantDomainInfo;
+
+// static
+void ImportantSitesUsageCounter::GetUsage(
+    std::vector<ImportantSitesUtil::ImportantDomainInfo> sites,
+    storage::QuotaManager* quota_manager,
+    content::DOMStorageContext* dom_storage_context,
+    UsageCallback callback) {
+  (new ImportantSitesUsageCounter(std::move(sites), quota_manager,
+                                  dom_storage_context, callback))
+      ->RunAndDestroySelf();
+}
+
+ImportantSitesUsageCounter::ImportantSitesUsageCounter(
+    std::vector<ImportantDomainInfo> sites,
+    storage::QuotaManager* quota_manager,
+    content::DOMStorageContext* dom_storage_context,
+    UsageCallback callback)
+    : callback_(callback),
+      sites_(std::move(sites)),
+      quota_manager_(quota_manager),
+      dom_storage_context_(dom_storage_context),
+      tasks_(-1) {
+  for (ImportantDomainInfo& site : sites_)
+    site.usage = 0;
+}
+
+ImportantSitesUsageCounter::~ImportantSitesUsageCounter() {}
+
+void ImportantSitesUsageCounter::RunAndDestroySelf() {

[Bernhard Bauer, 2017/05/04 14:16:44]

This method doesn't immediately destroy the object, so the name is a bit
misleading.

[dullweber, 2017/05/05 08:35:27]

I changed it to RunAndDestroySelfWhenFinished()

[Bernhard Bauer, 2017/05/05 09:36:04]

FWIW, I would be fine with just Run() too (and a comment that it will destroy
itself) :)

+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  tasks_ = 1;  // Task for this method
+  tasks_ += 1;
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&ImportantSitesUsageCounter::GetQuotaUsageOnIOThread,

[Bernhard Bauer, 2017/05/04 14:16:44]

Nit: base::BindOnce() where possible.

[dullweber, 2017/05/05 08:35:27]

I changed to BindOnce where it was accepted

+                 base::Unretained(this)));
+  tasks_ += 1;
+  dom_storage_context_->GetLocalStorageUsage(
+      base::Bind(&ImportantSitesUsageCounter::ReceiveLocalStorageUsage,
+                 base::Unretained(this)));
+  Done();

[Bernhard Bauer, 2017/05/04 14:16:44]

You can set |tasks_| to 0 and leave out this call. You *know* that there is
going to be at least one outstanding task at this point, so you don't really
need this check.

[dullweber, 2017/05/05 08:35:27]

Ok, I removed it. Just to confirm: I can only do this because the first tasks
calls the IO thread, so it is impossible to get a synchronous response?

[Bernhard Bauer, 2017/05/05 09:36:04]

Yes, otherwise it would depend on whether or not GetLocalStorageUsage() might
run its callback synchronously. If it's run asynchronously, it would be fine
again; if it's synchronously, it would probably still work, but it would be
reentrant, which tends to be tricky.

+}
+
+void ImportantSitesUsageCounter::GetQuotaUsageOnIOThread() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  quota_manager_->GetUsageInfo(
+      base::Bind(&ImportantSitesUsageCounter::ReceiveQuotaUsageOnIOThread,
+                 base::Unretained(this)));
+}
+
+void ImportantSitesUsageCounter::ReceiveQuotaUsageOnIOThread(
+    const std::vector<storage::UsageInfo>& usage_infos) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&ImportantSitesUsageCounter::ReceiveQuotaUsage,
+                 base::Unretained(this), usage_infos));
+}
+
+void ImportantSitesUsageCounter::ReceiveQuotaUsage(
+    const std::vector<storage::UsageInfo>& usage_infos) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  for (const storage::UsageInfo& info : usage_infos) {
+    IncrementUsage(
+        ImportantSitesUtil::GetRegisterableDomainOrIPFromHost(info.host),
+        info.usage);
+  }
+  Done();
+}
+
+void ImportantSitesUsageCounter::ReceiveLocalStorageUsage(
+    const std::vector<content::LocalStorageUsageInfo>& storage_infos) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  for (const content::LocalStorageUsageInfo& info : storage_infos) {
+    IncrementUsage(ImportantSitesUtil::GetRegisterableDomainOrIP(info.origin),
+                   info.data_size);
+  }
+  Done();
+}
+
+// Look up the corresponding ImportantDomainInfo for |url| and increase its
+// usage by |size|.
+void ImportantSitesUsageCounter::IncrementUsage(const std::string& domain,
+                                                int64_t size) {
+  // Use a linear search over sites_ because it only has up to 10 entries.
+  auto it = std::find_if(sites_.begin(), sites_.end(),
+                         [domain](ImportantDomainInfo& info) {
+                           return info.registerable_domain == domain;
+                         });
+  if (it != sites_.end())
+    it->usage += size;
+}
+
+void ImportantSitesUsageCounter::Done() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK_GT(tasks_, 0);
+  if (--tasks_ == 0) {
+    callback_.Run(std::move(sites_));
+    base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this);

[Bernhard Bauer, 2017/05/04 14:16:44]

I would just directly delete |this| here. Given that the object is not used
anywhere else, you don't need to wait to destroy it, and it makes analyzing the
control flow e.g. from a stack trace much easier if you have a direct call.

[dullweber, 2017/05/05 08:35:27]

Done.

+  }
+}