Remove implicit conversions from scoped_refptr to T* in chrome/browser/extensions/

chrome/browser/extensions/extension_ui_unittest.cc

Index: chrome/browser/extensions/extension_ui_unittest.cc
diff --git a/chrome/browser/extensions/extension_ui_unittest.cc b/chrome/browser/extensions/extension_ui_unittest.cc
index b23ab502b7ba156ad033c93cb38a43c7a01eb1f6..49446b3f586fa4b90ee4b18bc19cd07899c13a88 100644
--- a/chrome/browser/extensions/extension_ui_unittest.cc
+++ b/chrome/browser/extensions/extension_ui_unittest.cc
@@ -76,8 +76,9 @@ class ExtensionUITest : public testing::Test {
     return static_cast<base::DictionaryValue*>(value);
   }
 
-  const Extension* CreateExtension(const std::string& name,
-                                   ListBuilder& permissions) {
+  const scoped_refptr<const Extension> CreateExtension(

[dcheng, 2014/08/26 01:15:28]

Note: this return type (and other instances in this test) was rewritten instead
of simply adding .get(), because generally speaking, there is no way of
determining if this is safe or not (obviously the existing code works, but it's
potentially dangerous--the only obvious reference is on the stack, which is lost
once the function returns).

Please let me know if you'd like me to take another action here, and I'm happy
to update the code accordingly.

+      const std::string& name,
+      ListBuilder& permissions) {
     const std::string kId = crx_file::id_util::GenerateId(name);
     scoped_refptr<const Extension> extension =
         ExtensionBuilder().SetManifest(
@@ -92,7 +93,7 @@ class ExtensionUITest : public testing::Test {
                           .Build();
 
     ExtensionRegistry::Get(profile())->AddEnabled(extension);
-    PermissionsUpdater(profile()).InitializePermissions(extension);
+    PermissionsUpdater(profile()).InitializePermissions(extension.get());
     return extension;
   }
 
@@ -320,15 +321,13 @@ TEST_F(ExtensionUITest, ExtensionUIAllUrlsCheckbox) {
       new FeatureSwitch::ScopedOverride(
           FeatureSwitch::scripts_require_action(), true));
   // Two extensions - one with all urls, one without.
-  const Extension* all_urls_extension =
-      CreateExtension("all_urls",
-                      ListBuilder().Append(kAllHostsPermission).Pass());
-  const Extension* no_urls_extension =
+  scoped_refptr<const Extension> all_urls_extension = CreateExtension(

[dcheng, 2014/08/26 01:15:27]

Note: this variable type (and other instances in this test) was rewritten
because CreateExtension()'s return type changed to scoped_refptr<T>. Assigning
this directly to a raw pointer means that there might no longer be a reference
once this line finishes executing, since the temporary scoped_refptr<T> will be
destroyed.

Please let me know if you'd like me to do something else here.

+      "all_urls", ListBuilder().Append(kAllHostsPermission).Pass());
+  scoped_refptr<const Extension> no_urls_extension =
       CreateExtension("no urls", ListBuilder().Pass());
 
-  scoped_ptr<base::DictionaryValue> value(
-      handler()->CreateExtensionDetailValue(
-          all_urls_extension, std::vector<ExtensionPage>(), NULL));
+  scoped_ptr<base::DictionaryValue> value(handler()->CreateExtensionDetailValue(
+      all_urls_extension.get(), std::vector<ExtensionPage>(), NULL));
   bool result = false;
   const std::string kWantsAllUrls = "wantsAllUrls";
   const std::string kAllowAllUrls = "allowAllUrls";
@@ -345,7 +344,7 @@ TEST_F(ExtensionUITest, ExtensionUIAllUrlsCheckbox) {
 
   // Now the extension should both want and have all urls.
   value.reset(handler()->CreateExtensionDetailValue(
-      all_urls_extension, std::vector<ExtensionPage>(), NULL));
+      all_urls_extension.get(), std::vector<ExtensionPage>(), NULL));
   EXPECT_TRUE(value->GetBoolean(kWantsAllUrls, &result));
   EXPECT_TRUE(result);
   EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result));
@@ -353,7 +352,7 @@ TEST_F(ExtensionUITest, ExtensionUIAllUrlsCheckbox) {
 
   // The other extension should neither want nor have all urls.
   value.reset(handler()->CreateExtensionDetailValue(
-      no_urls_extension, std::vector<ExtensionPage>(), NULL));
+      no_urls_extension.get(), std::vector<ExtensionPage>(), NULL));
   EXPECT_TRUE(value->GetBoolean(kWantsAllUrls, &result));
   EXPECT_FALSE(result);
   EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result));
@@ -366,7 +365,7 @@ TEST_F(ExtensionUITest, ExtensionUIAllUrlsCheckbox) {
   // Even though the extension has the all urls preference, the checkbox
   // shouldn't show up with the switch off.
   value.reset(handler()->CreateExtensionDetailValue(
-      all_urls_extension, std::vector<ExtensionPage>(), NULL));
+      all_urls_extension.get(), std::vector<ExtensionPage>(), NULL));
   EXPECT_TRUE(value->GetBoolean(kWantsAllUrls, &result));
   EXPECT_FALSE(result);
   EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result));
@@ -379,7 +378,7 @@ TEST_F(ExtensionUITest, ExtensionUIAllUrlsCheckbox) {
   // Even though the extension has all_urls permission, the checkbox shouldn't
   // show up without the switch.
   value.reset(handler()->CreateExtensionDetailValue(
-      all_urls_extension, std::vector<ExtensionPage>(), NULL));
+      all_urls_extension.get(), std::vector<ExtensionPage>(), NULL));
   EXPECT_TRUE(value->GetBoolean(kWantsAllUrls, &result));
   EXPECT_FALSE(result);
   EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result));