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

chrome/browser/extensions/extension_ui_unittest.cc

 // Copyright (c) 2012 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 "base/command_line.h"
 #include "base/json/json_file_value_serializer.h"
 #include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
 #include "base/strings/string_util.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ skipping 58 matching lines @@
       const base::FilePath& path,
       std::string *error) {
     base::Value* value;
 
     JSONFileValueSerializer serializer(path);
     value = serializer.Deserialize(NULL, error);
 
     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(
                                DictionaryBuilder()
                                    .Set("name", name)
                                    .Set("description", "an extension")
                                    .Set("manifest_version", 2)
                                    .Set("version", "1.0.0")
                                    .Set("permissions", permissions))
                           .SetLocation(Manifest::INTERNAL)
                           .SetID(kId)
                           .Build();
 
     ExtensionRegistry::Get(profile())->AddEnabled(extension);
-    PermissionsUpdater(profile()).InitializePermissions(extension);
+    PermissionsUpdater(profile()).InitializePermissions(extension.get());
     return extension;
   }
 
   base::DictionaryValue* CreateExtensionDetailViewFromPath(
       const base::FilePath& extension_path,
       const std::vector<ExtensionPage>& pages,
       Manifest::Location location) {
     std::string error;
 
     base::FilePath manifest_path = extension_path.Append(kManifestFilename);
@@ skipping 207 matching lines @@
 }
 
 // Test that the all_urls checkbox only shows up for extensions that want all
 // urls, and only when the switch is on.
 TEST_F(ExtensionUITest, ExtensionUIAllUrlsCheckbox) {
   // Start with the switch enabled.
   scoped_ptr<FeatureSwitch::ScopedOverride> enable_scripts_switch(
       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";
 
   // The extension should want all urls, but not currently have it.
   EXPECT_TRUE(value->GetBoolean(kWantsAllUrls, &result));
   EXPECT_TRUE(result);
   EXPECT_TRUE(value->GetBoolean(kAllowAllUrls, &result));
   EXPECT_FALSE(result);
 
   // Give the extension all urls.
   util::SetAllowedScriptingOnAllUrls(
       all_urls_extension->id(), profile(), true);
 
   // 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));
   EXPECT_TRUE(result);
 
   // 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));
   EXPECT_FALSE(result);
 
   // Turn off the switch and load another extension (so permissions are
   // re-initialized).
   enable_scripts_switch.reset();
 
   // 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));
   EXPECT_TRUE(result);
 
   // Load another extension with all urls (so permissions get re-init'd).
   all_urls_extension = CreateExtension(
       "all_urls_II", ListBuilder().Append(kAllHostsPermission).Pass());
 
   // 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));
   EXPECT_FALSE(result);
 }
 
 }  // namespace extensions