Add threadsafe version of PermissionManager::GetPermissionStatus

chrome/browser/permissions/permission_manager_unittest.cc

 // Copyright 2015 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/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"
 
 using blink::mojom::PermissionStatus;
 using content::PermissionType;
 
 namespace {
 
 class PermissionManagerTestingProfile final : public TestingProfile {
@@ skipping 10 matching lines @@
 
 }  // anonymous namespace
 
 class PermissionManagerTest : public testing::Test {
  public:
   void OnPermissionChange(PermissionStatus permission) {
     callback_called_ = true;
     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 {
     return url_;
   }
 
   const GURL& other_url() const {
     return other_url_;
   }
 
   bool callback_called() const {
     return callback_called_;
   }
 
   PermissionStatus callback_result() const { return callback_result_; }
 
   void Reset() {
     callback_called_ = false;
     callback_result_ = PermissionStatus::ASK;
   }
 
  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);
   CheckPermissionStatus(PermissionType::GEOLOCATION, PermissionStatus::ASK);
 #if defined(OS_ANDROID)
   CheckPermissionStatus(PermissionType::PROTECTED_MEDIA_IDENTIFIER,
                         PermissionStatus::ASK);
 #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();

[mlamouri (slow - plz ping), 2016/10/24 10:40:51]

Looks fine but I would have done this a bit differently:
- one test checking that calling the regular GetPermissionStatus on DEBUG will
crash
- one test checking that calling the GetPermissionStatus will
HostContentSettingsMap returns the correct value (and doesn't crash ;))

I'm not super comfortable with the magic thread-check in the test.

However, lgtm either way :)

+}
+
 TEST_F(PermissionManagerTest, GetPermissionStatusAfterSet) {
   SetPermission(CONTENT_SETTINGS_TYPE_GEOLOCATION, CONTENT_SETTING_ALLOW);
   CheckPermissionStatus(PermissionType::GEOLOCATION, PermissionStatus::GRANTED);
 
   SetPermission(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, CONTENT_SETTING_ALLOW);
   CheckPermissionStatus(PermissionType::NOTIFICATIONS,
                         PermissionStatus::GRANTED);
   CheckPermissionStatus(PermissionType::PUSH_MESSAGING,
                         PermissionStatus::GRANTED);
 
@@ skipping 191 matching lines @@
   CheckPermissionStatus(PermissionType::GEOLOCATION, PermissionStatus::ASK);
   GetHostContentSettingsMap()->SetContentSettingDefaultScope(
       url(), url(), CONTENT_SETTINGS_TYPE_GEOLOCATION, std::string(),
       CONTENT_SETTING_ALLOW);
   CheckPermissionStatus(PermissionType::GEOLOCATION, PermissionStatus::GRANTED);
 
   EXPECT_FALSE(callback_called());
 
   GetPermissionManager()->UnsubscribePermissionStatusChange(subscription_id);
 }