Add threadsafe version of PermissionManager::GetPermissionStatus

Add a version of GetPermissionStatus that takes a host_content_settings_map.
This method is guaranteed to be threadsafe.

BUG=658020

R=jochen, mlamouri

Review-Url: https://codereview.chromium.org/2439673004
Cr-Commit-Position: refs/heads/master@{#427078}

chrome/browser/permissions/permission_context_base.cc

@@ -122,9 +122,9 @@ void PermissionContextBase::RequestPermission(
 }
 
 ContentSetting PermissionContextBase::GetPermissionStatus(
+    HostContentSettingsMap* host,
     const GURL& requesting_origin,
     const GURL& embedding_origin) const {
-
   // If the permission has been disabled through Finch, block all requests.
   if (IsPermissionKillSwitchOn())
     return CONTENT_SETTING_BLOCK;
@@ -134,9 +134,16 @@ ContentSetting PermissionContextBase::GetPermissionStatus(
     return CONTENT_SETTING_BLOCK;
   }
 
-  return HostContentSettingsMapFactory::GetForProfile(profile_)
-      ->GetContentSetting(requesting_origin, embedding_origin,
-                          content_settings_type_, std::string());
+  return host->GetContentSetting(requesting_origin, embedding_origin,
+                                 content_settings_type_, std::string());
+}
+
+ContentSetting PermissionContextBase::GetPermissionStatus(
+    const GURL& requesting_origin,
+    const GURL& embedding_origin) const {
+  HostContentSettingsMap* host =
+      HostContentSettingsMapFactory::GetForProfile(profile_);
+  return GetPermissionStatus(host, requesting_origin, embedding_origin);
 }
 
 void PermissionContextBase::ResetPermission(

chrome/browser/permissions/permission_context_base.h

@@ -21,6 +21,7 @@
 class PermissionQueueController;
 #endif
 class GURL;
+class HostContentSettingsMap;
 class PermissionRequestID;
 class Profile;
 
@@ -85,6 +86,13 @@ class PermissionContextBase : public KeyedService {
       const GURL& requesting_origin,
       const GURL& embedding_origin) const;
 
+  // Thread safe version of GetPermissionStatus for consumers that already have
+  // an associated HostContentSettingsMap.
+  virtual ContentSetting GetPermissionStatus(
+      HostContentSettingsMap* host,
+      const GURL& requesting_origin,
+      const GURL& embedding_origin) const;
+
   // Resets the permission to its default value.
   virtual void ResetPermission(const GURL& requesting_origin,
                                const GURL& embedding_origin);
@@ -148,6 +156,7 @@ class PermissionContextBase : public KeyedService {
                                     ContentSetting content_setting);
 
   // Whether the permission should be restricted to secure origins.
+  // Note: Maybe be called from multiple threads.
   virtual bool IsRestrictedToSecureOrigins() const = 0;
 
   content::PermissionType permission_type() const { return permission_type_; }

chrome/browser/permissions/permission_manager.cc

@@ -388,6 +388,22 @@ void PermissionManager::ResetPermission(PermissionType permission,
                            embedding_origin.GetOrigin());
 }
 
+PermissionStatus PermissionManager::GetPermissionStatus(
+    HostContentSettingsMap* host,
+    content::PermissionType permission,
+    const GURL& requesting_origin,
+    const GURL& embedding_origin) {
+  if (IsConstantPermission(permission))
+    return GetPermissionStatusForConstantPermission(permission);
+
+  PermissionContextBase* context = GetPermissionContext(permission);
+  if (!context)
+    return PermissionStatus::DENIED;
+
+  return ContentSettingToPermissionStatus(context->GetPermissionStatus(
+      host, requesting_origin.GetOrigin(), embedding_origin.GetOrigin()));
+}
+
 PermissionStatus PermissionManager::GetPermissionStatus(
     PermissionType permission,
     const GURL& requesting_origin,

chrome/browser/permissions/permission_manager.h

@@ -17,6 +17,7 @@
 #include "components/keyed_service/core/keyed_service.h"
 #include "content/public/browser/permission_manager.h"
 
+class HostContentSettingsMap;
 class PermissionContextBase;
 class Profile;
 
@@ -58,6 +59,13 @@ class PermissionManager : public KeyedService,
       content::PermissionType permission,
       const GURL& requesting_origin,
       const GURL& embedding_origin) override;
+
+  blink::mojom::PermissionStatus GetPermissionStatus(
+      HostContentSettingsMap* host,
+      content::PermissionType permission,
+      const GURL& requesting_origin,
+      const GURL& embedding_origin);
+
   void RegisterPermissionUsage(content::PermissionType permission,
                                const GURL& requesting_origin,
                                const GURL& embedding_origin) override;

chrome/browser/permissions/permission_manager_unittest.cc

@@ -5,11 +5,14 @@
 #include "chrome/browser/permissions/permission_manager.h"
 
 #include "base/macros.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "build/build_config.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/permissions/permission_manager_factory.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/permission_type.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -40,30 +43,42 @@ class PermissionManagerTest : public testing::Test {
     callback_result_ = permission;
   }
 
+  void TestGetPermissionStatus();
+
  protected:
   PermissionManagerTest()
-      : url_("https://example.com"),
+      : thread_bundle_(content::TestBrowserThreadBundle::REAL_IO_THREAD),
+        url_("https://example.com"),
         other_url_("https://foo.com"),
         callback_called_(false),
-        callback_result_(PermissionStatus::ASK) {}
+        callback_result_(PermissionStatus::ASK) {
+    host_content_settings_map_ =
+        HostContentSettingsMapFactory::GetForProfile(&profile_);
+  }
 
   PermissionManager* GetPermissionManager() {
     return profile_.GetPermissionManager();
   }
 
   HostContentSettingsMap* GetHostContentSettingsMap() {
-    return HostContentSettingsMapFactory::GetForProfile(&profile_);
+    return host_content_settings_map_;
   }
 
   void CheckPermissionStatus(PermissionType type,
                              PermissionStatus expected) {
-    EXPECT_EQ(expected, GetPermissionManager()->GetPermissionStatus(
-                            type, url_.GetOrigin(), url_.GetOrigin()));
+    if (content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) {
+      EXPECT_EQ(expected, GetPermissionManager()->GetPermissionStatus(
+                              type, url_.GetOrigin(), url_.GetOrigin()));
+    } else {
+      EXPECT_EQ(expected, GetPermissionManager()->GetPermissionStatus(
+                              host_content_settings_map_, type,
+                              url_.GetOrigin(), url_.GetOrigin()));
+    }
   }
 
   void SetPermission(ContentSettingsType type, ContentSetting value) {
-    HostContentSettingsMapFactory::GetForProfile(&profile_)
-        ->SetContentSettingDefaultScope(url_, url_, type, std::string(), value);
+    GetHostContentSettingsMap()->SetContentSettingDefaultScope(
+        url_, url_, type, std::string(), value);
   }
 
   const GURL& url() const {
@@ -86,15 +101,16 @@ class PermissionManagerTest : public testing::Test {
   }
 
  private:
+  content::TestBrowserThreadBundle thread_bundle_;
   const GURL url_;
   const GURL other_url_;
   bool callback_called_;
   PermissionStatus callback_result_;
-  content::TestBrowserThreadBundle thread_bundle_;
   PermissionManagerTestingProfile profile_;
+  HostContentSettingsMap* host_content_settings_map_;
 };
 
-TEST_F(PermissionManagerTest, GetPermissionStatusDefault) {
+void PermissionManagerTest::TestGetPermissionStatus() {
   CheckPermissionStatus(PermissionType::MIDI_SYSEX, PermissionStatus::ASK);
   CheckPermissionStatus(PermissionType::PUSH_MESSAGING, PermissionStatus::ASK);
   CheckPermissionStatus(PermissionType::NOTIFICATIONS, PermissionStatus::ASK);
@@ -105,6 +121,20 @@ TEST_F(PermissionManagerTest, GetPermissionStatusDefault) {
 #endif
 }
 
+TEST_F(PermissionManagerTest, GetPermissionStatusDefault) {
+  TestGetPermissionStatus();
+}
+
+TEST_F(PermissionManagerTest, GetPermissionStatusNonMainThread) {
+  base::RunLoop run_loop;
+  content::BrowserThread::PostTaskAndReply(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&PermissionManagerTest::TestGetPermissionStatus,
+                 base::Unretained(this)),
+      run_loop.QuitClosure());
+  run_loop.Run();
+}
+
 TEST_F(PermissionManagerTest, GetPermissionStatusAfterSet) {
   SetPermission(CONTENT_SETTINGS_TYPE_GEOLOCATION, CONTENT_SETTING_ALLOW);
   CheckPermissionStatus(PermissionType::GEOLOCATION, PermissionStatus::GRANTED);