Refactor blacklist client into own .h/.cc.

chrome/browser/permissions/permission_blacklist_client.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.

[raymes, 2017/01/11 00:00:36]

nit: 2017

[meredithl, 2017/01/11 02:24:52]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/permissions/permission_blacklist_client.h"
+
+#include <set>
+#include <string>
+
+#include "base/memory/ptr_util.h"
+#include "components/safe_browsing_db/database_manager.h"

[raymes, 2017/01/11 00:00:36]

this is in the header, so not needed here

[meredithl, 2017/01/11 02:24:52]

Done.

+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/web_contents.h"
+#include "url/gurl.h"
+
+// static
+void PermissionBlacklistClient::CheckSafeBrowsingBlacklist(
+    scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager,
+    content::PermissionType permission_type,
+    const GURL& request_origin,
+    content::WebContents* web_contents,
+    int timeout,
+    base::Callback<void(bool)> callback) {
+  new PermissionBlacklistClient(db_manager, permission_type, request_origin,
+                                web_contents, timeout, callback);
+}
+
+PermissionBlacklistClient::PermissionBlacklistClient(
+    scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager,
+    content::PermissionType permission_type,
+    const GURL& request_origin,
+    content::WebContents* web_contents,
+    int timeout,
+    base::Callback<void(bool)> callback)
+    : content::WebContentsObserver(web_contents),
+      db_manager_(db_manager),
+      permission_type_(permission_type),
+      callback_(callback),
+      timeout_(timeout),
+      is_active_(true) /* Web Contents is active upon construction */ {

[raymes, 2017/01/11 00:00:36]

nit: don't use an inline comment here. I think it's ok to omit the
comment in this case. 

This was fixed in the committed code - is the code in this file up to date with
the code in permission_context_base? Please make sure they are identical.

[meredithl, 2017/01/11 02:24:52]

Sorry raymes - yes, I had already pulled most of the client out before landing
the previous CL, so didn't have the last few changes in there. Should be fixed
now.

+  // Balanced by a call to Release() in OnCheckApiBlacklistUrlResult().
+  AddRef();
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&PermissionBlacklistClient::StartCheck, this, request_origin));
+}
+
+PermissionBlacklistClient::~PermissionBlacklistClient() {}
+
+void PermissionBlacklistClient::StartCheck(const GURL& request_origin) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);

[raymes, 2017/01/11 00:00:36]

#include base/logging.h

[dominickn, 2017/01/11 01:47:38]

Why do we need base/logging.h? DCHECK_CURRENTLY_ON is defined in
browser_thread.h

[raymes, 2017/01/11 01:56:30]

You're right, ignore me :)

[meredithl, 2017/01/11 02:24:52]

Acknowledged.

+
+  // Start the timer to interrupt into the client callback method with an
+  // empty response if Safe Browsing times out.
+  safe_browsing::ThreatMetadata empty_metadata;
+  timer_ = base::MakeUnique<base::OneShotTimer>();

[raymes, 2017/01/11 00:00:36]

#include base/timer.h

[meredithl, 2017/01/11 02:24:52]

Done.

+  timer_->Start(
+      FROM_HERE, base::TimeDelta::FromMilliseconds(timeout_),
+      base::Bind(&PermissionBlacklistClient::OnCheckApiBlacklistUrlResult, this,
+                 request_origin, empty_metadata));
+  db_manager_->CheckApiBlacklistUrl(request_origin, this);
+}
+
+void PermissionBlacklistClient::OnCheckApiBlacklistUrlResult(
+    const GURL& url,
+    const safe_browsing::ThreatMetadata& metadata) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  if (timer_->IsRunning())
+    timer_->Stop();
+  else
+    db_manager_->CancelApiCheck(this);
+  timer_.reset(nullptr);
+
+  // TODO(meredithl): Convert the strings returned from Safe Browsing to the
+  // ones used by PermissionUtil for comparison.
+  bool permission_blocked =
+      metadata.api_permissions.find(PermissionUtil::GetPermissionString(
+          permission_type_)) != metadata.api_permissions.end();
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&PermissionBlacklistClient::EvaluateBlacklistResultOnUiThread,
+                 this, permission_blocked));
+}
+
+void PermissionBlacklistClient::EvaluateBlacklistResultOnUiThread(
+    bool permission_blocked) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (is_active_)
+    callback_.Run(permission_blocked);
+  Release();
+}
+
+void PermissionBlacklistClient::WebContentsDestroyed() {
+  is_active_ = false;
+  Observe(nullptr);
+}