Getting rid of raw pointers to SharedMemory in UserScriptMaster

chrome/browser/extensions/user_script_master.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 "chrome/browser/extensions/user_script_master.h"
 
 #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"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/image_loader.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/api/i18n/default_locale_handler.h"
 #include "chrome/common/extensions/manifest_handlers/content_scripts_handler.h"
@@ skipping 139 matching lines @@
   this->extensions_info_ = extensions_info;
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(
           &UserScriptMaster::ScriptReloader::RunLoad, this, user_scripts));
 }
 
 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().
   Release();
 }
 
 static void VerifyContent(ContentVerifier* verifier,
                           const std::string& extension_id,
                           const base::FilePath& extension_root,
                           const base::FilePath& relative_path,
@@ skipping 92 matching lines @@
     return NULL;
   }
 
   return file_util::LoadMessageBundleSubstitutionMap(
       extensions_info_[extension_id].first,
       extension_id,
       extensions_info_[extension_id].second);
 }
 
 // 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++) {
     const UserScript& script = scripts[i];
     // TODO(aa): This can be replaced by sending content script metadata to
     // renderers along with other extension data in ExtensionMsg_Loaded.
     // See crbug.com/70516.
     script.Pickle(&pickle);
     // Write scripts as 'data' so that we can read it out in the slave without
     // allocating a new string.
     for (size_t j = 0; j < script.js_scripts().size(); j++) {
       base::StringPiece contents = script.js_scripts()[j].GetContent();
       pickle.WriteData(contents.data(), contents.length());
     }
     for (size_t j = 0; j < script.css_scripts().size(); j++) {
       base::StringPiece contents = script.css_scripts()[j].GetContent();
       pickle.WriteData(contents.data(), contents.length());
     }
   }
 
   // Create the shared memory object.
   base::SharedMemory shared_memory;
 
   base::SharedMemoryCreateOptions options;
   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());
 
   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.
 void UserScriptMaster::ScriptReloader::RunLoad(
     const UserScriptList& user_scripts) {
   LoadUserScripts(const_cast<UserScriptList*>(&user_scripts));
 
   // Scripts now contains list of up-to-date scripts. Load the content in the
   // shared memory and let the master know it's ready. We need to post the task
   // 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)
     : extensions_service_ready_(false),
       pending_load_(false),
       profile_(profile),
       extension_registry_observer_(this) {
   extension_registry_observer_.Add(ExtensionRegistry::Get(profile_));
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSIONS_READY,
                  content::Source<Profile>(profile_));
   registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CREATED,
                  content::NotificationService::AllBrowserContextsAndSources());
 }
 
 UserScriptMaster::~UserScriptMaster() {
   if (script_reloader_.get())
     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
     // notifying about these scripts and instead just immediately reload.
     pending_load_ = false;
     StartLoad();
   } else {
     // We're no longer loading.
     script_reloader_ = NULL;
 
     if (handle == NULL) {
       // This can happen if we run out of file descriptors.  In that case, we
       // have a choice between silently omitting all user scripts for new tabs,
       // by nulling out shared_memory_, or only silently omitting new ones by
       // leaving the existing object in place. The second seems less bad, even
       // though it removes the possibility that freeing the shared memory block
       // would open up enough FDs for long enough for a retry to succeed.
 
       // Pretend the extension change didn't happen.
       return;
     }
 
     // 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()));
   }
 }
 
 ContentVerifier* UserScriptMaster::content_verifier() {
   ExtensionSystem* system = ExtensionSystem::Get(profile_);
   return system->content_verifier();
 }
 
 void UserScriptMaster::OnExtensionLoaded(
     content::BrowserContext* browser_context,
@@ skipping 100 matching lines @@
 
   base::SharedMemoryHandle handle_for_process;
   if (!shared_memory->ShareToProcess(handle, &handle_for_process))
     return;  // This can legitimately fail if the renderer asserts at startup.
 
   if (base::SharedMemory::IsHandleValid(handle_for_process))
     process->Send(new ExtensionMsg_UpdateUserScripts(handle_for_process));
 }
 
 }  // namespace extensions