Getting rid of raw pointers to SharedMemory in UserScriptMaster

chrome/browser/extensions/user_script_master.cc

Index: chrome/browser/extensions/user_script_master.cc
diff --git a/chrome/browser/extensions/user_script_master.cc b/chrome/browser/extensions/user_script_master.cc
index ccf7f1918a2920eedfd0dccc6ebc8bae91419f72..9785136550b87acde97fdd93081b869475e81969 100644
--- a/chrome/browser/extensions/user_script_master.cc
+++ b/chrome/browser/extensions/user_script_master.cc
@@ -7,6 +7,7 @@
 #include <string>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/version.h"
@@ -166,12 +167,10 @@ void UserScriptMaster::ScriptReloader::StartLoad(
 UserScriptMaster::ScriptReloader::~ScriptReloader() {}
 
 void UserScriptMaster::ScriptReloader::NotifyMaster(
-    base::SharedMemory* memory) {
-  // The master went away, so these new scripts aren't useful anymore.
-  if (!master_)
-    delete memory;
-  else
-    master_->NewScriptsAvailable(memory);
+    scoped_ptr<base::SharedMemory> memory) {
+  // The master could go away
+  if (master_)
+    master_->NewScriptsAvailable(memory.Pass());
 
   // Drop our self-reference.
   // Balances StartLoad().
@@ -284,7 +283,7 @@ SubstitutionMap* UserScriptMaster::ScriptReloader::GetLocalizationMessages(
 }
 
 // Pickle user scripts and return pointer to the shared memory.
-static base::SharedMemory* Serialize(const UserScriptList& scripts) {
+static scoped_ptr<base::SharedMemory> Serialize(const UserScriptList& scripts) {
   Pickle pickle;
   pickle.WriteUInt64(scripts.size());
   for (size_t i = 0; i < scripts.size(); i++) {
@@ -312,10 +311,10 @@ static base::SharedMemory* Serialize(const UserScriptList& scripts) {
   options.size = pickle.size();
   options.share_read_only = true;
   if (!shared_memory.Create(options))
-    return NULL;
+    return scoped_ptr<base::SharedMemory>();
 
   if (!shared_memory.Map(pickle.size()))
-    return NULL;
+    return scoped_ptr<base::SharedMemory>();
 
   // Copy the pickle to shared memory.
   memcpy(shared_memory.memory(), pickle.data(), pickle.size());
@@ -323,9 +322,10 @@ static base::SharedMemory* Serialize(const UserScriptList& scripts) {
   base::SharedMemoryHandle readonly_handle;
   if (!shared_memory.ShareReadOnlyToProcess(base::GetCurrentProcessHandle(),
                                             &readonly_handle))
-    return NULL;
+    return scoped_ptr<base::SharedMemory>();
 
-  return new base::SharedMemory(readonly_handle, /*read_only=*/true);
+  return make_scoped_ptr(new base::SharedMemory(readonly_handle,
+                                                /*read_only=*/true));
 }
 
 // This method will be called on the file thread.
@@ -338,8 +338,8 @@ void UserScriptMaster::ScriptReloader::RunLoad(
   // back even if no scripts ware found to balance the AddRef/Release calls.
   BrowserThread::PostTask(
       master_thread_id_, FROM_HERE,
-      base::Bind(
-          &ScriptReloader::NotifyMaster, this, Serialize(user_scripts)));
+      base::Bind(&ScriptReloader::NotifyMaster, this,

[Jeffrey Yasskin, 2014/05/17 00:50:33]

Generally use `git cl format` to format your changes before uploading them.

+                 base::Passed(Serialize(user_scripts))));
 }
 
 UserScriptMaster::UserScriptMaster(Profile* profile)
@@ -359,9 +359,8 @@ UserScriptMaster::~UserScriptMaster() {
     script_reloader_->DisownMaster();
 }
 
-void UserScriptMaster::NewScriptsAvailable(base::SharedMemory* handle) {
-  // Ensure handle is deleted or released.
-  scoped_ptr<base::SharedMemory> handle_deleter(handle);
+void UserScriptMaster::NewScriptsAvailable(
+    scoped_ptr<base::SharedMemory> handle) {
 
   if (pending_load_) {
     // While we were loading, there were further changes.  Don't bother
@@ -385,18 +384,18 @@ void UserScriptMaster::NewScriptsAvailable(base::SharedMemory* handle) {
     }
 
     // We've got scripts ready to go.
-    shared_memory_.swap(handle_deleter);
+    shared_memory_.swap(handle);

[Jeffrey Yasskin, 2014/05/17 00:50:33]

Since we're not using |handle| anymore, it's a bit clearer to use
"shared_memory_ = handle.Pass();" ... unless we actually can't destroy the old
memory that soon? It looks like destroying it eagerly is safe.

 
     for (content::RenderProcessHost::iterator i(
             content::RenderProcessHost::AllHostsIterator());
          !i.IsAtEnd(); i.Advance()) {
-      SendUpdate(i.GetCurrentValue(), handle);
+      SendUpdate(i.GetCurrentValue(), shared_memory_.get());
     }
 
     content::NotificationService::current()->Notify(
         chrome::NOTIFICATION_USER_SCRIPTS_UPDATED,
         content::Source<Profile>(profile_),
-        content::Details<base::SharedMemory>(handle));
+        content::Details<base::SharedMemory>(shared_memory_.get()));
   }
 }