Fix dangerous pointer use in UserScriptMaster

chrome/browser/extensions/user_script_master.h

 // 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.
 
 #ifndef CHROME_BROWSER_EXTENSIONS_USER_SCRIPT_MASTER_H_
 #define CHROME_BROWSER_EXTENSIONS_USER_SCRIPT_MASTER_H_
 
 #include <map>
 #include <string>
 
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/scoped_observer.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "extensions/browser/extension_registry_observer.h"
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/user_script.h"
 
+namespace base {
+class SharedMemory;
+}
+
 namespace content {
 class RenderProcessHost;
 }
 
 class Profile;
 
 namespace extensions {
 
 class ContentVerifier;
 class ExtensionRegistry;
 
 typedef std::map<std::string, ExtensionSet::ExtensionPathAndDefaultLocale>
     ExtensionsInfo;
 
 // Manages a segment of shared memory that contains the user scripts the user
 // has installed.  Lives on the UI thread.
-class UserScriptMaster : public base::RefCountedThreadSafe<UserScriptMaster>,
-                         public content::NotificationObserver,
+class UserScriptMaster : public content::NotificationObserver,
                          public ExtensionRegistryObserver {
  public:
+  // Parses the includes out of |script| and returns them in |includes|.
+  static bool ParseMetadataHeader(const base::StringPiece& script_text,

[Devlin, 2014/07/07 16:07:27]

This really doesn't belong here - UserScriptMaster doesn't use it.  But I'll
save that for a later CL.

+                                  UserScript* script);
+
+  // A wrapper around the method to load user scripts, which is normally run on
+  // the file thread. Exposed only for tests.
+  static void LoadScriptsForTest(UserScriptList* user_scripts);
+
   explicit UserScriptMaster(Profile* profile);
+  virtual ~UserScriptMaster();
 
   // Kicks off a process on the file thread to reload scripts from disk
   // into a new chunk of shared memory and notify renderers.
   virtual void StartLoad();
 
   // Gets the segment of shared memory for the scripts.
   base::SharedMemory* GetSharedMemory() const {
     return shared_memory_.get();
   }
 
-  // Called by the script reloader when new scripts have been loaded.
-  void NewScriptsAvailable(scoped_ptr<base::SharedMemory> handle);
-
   // Return true if we have any scripts ready.
   bool ScriptsReady() const { return shared_memory_.get() != NULL; }
 
-  // Returns the content verifier for our browser context.
-  ContentVerifier* content_verifier();
-
- protected:
-  friend class base::RefCountedThreadSafe<UserScriptMaster>;
-
-  virtual ~UserScriptMaster();
-
- public:
-  // We reload user scripts on the file thread to prevent blocking the UI.
-  // ScriptReloader lives on the file thread and does the reload
-  // work, and then sends a message back to its master with a new SharedMemory*.
-  // ScriptReloader is the worker that manages running the script load
-  // on the file thread. It must be created on, and its public API must only be
-  // called from, the master's thread.
-  class ScriptReloader
-      : public base::RefCountedThreadSafe<UserScriptMaster::ScriptReloader> {
-   public:
-    // Parses the includes out of |script| and returns them in |includes|.
-    static bool ParseMetadataHeader(const base::StringPiece& script_text,
-                                    UserScript* script);
-
-    explicit ScriptReloader(UserScriptMaster* master);
-
-    // Start loading of scripts.
-    // Will always send a message to the master upon completion.
-    void StartLoad(const UserScriptList& external_scripts,
-                   const ExtensionsInfo& extensions_info);
-
-    // The master is going away; don't call it back.
-    void DisownMaster() {
-      master_ = NULL;
-    }
-
-   private:
-    FRIEND_TEST_ALL_PREFIXES(UserScriptMasterTest, SkipBOMAtTheBeginning);
-    FRIEND_TEST_ALL_PREFIXES(UserScriptMasterTest, LeaveBOMNotAtTheBeginning);
-    friend class base::RefCountedThreadSafe<UserScriptMaster::ScriptReloader>;
-
-    ~ScriptReloader();
-
-    // Where functions are run:
-    //    master          file
-    //   StartLoad   ->  RunLoad
-    //                     LoadUserScripts()
-    // NotifyMaster  <-  RunLoad
-
-    // Runs on the master thread.
-    // Notify the master that new scripts are available.
-    void NotifyMaster(scoped_ptr<base::SharedMemory> memory);
-
-    // Runs on the File thread.
-    // Load the specified user scripts, calling NotifyMaster when done.
-    // |user_scripts| is intentionally passed by value so its lifetime isn't
-    // tied to the caller.
-    void RunLoad(const UserScriptList& user_scripts);
-
-    void LoadUserScripts(UserScriptList* user_scripts);
-
-    // Uses extensions_info_ to build a map of localization messages.
-    // Returns NULL if |extension_id| is invalid.
-    SubstitutionMap* GetLocalizationMessages(const std::string& extension_id);
-
-    // A pointer back to our master.
-    // May be NULL if DisownMaster() is called.
-    UserScriptMaster* master_;
-
-    // Maps extension info needed for localization to an extension ID.
-    ExtensionsInfo extensions_info_;
-
-    // The message loop to call our master back on.
-    // Expected to always outlive us.
-    content::BrowserThread::ID master_thread_id_;
-
-    scoped_refptr<ContentVerifier> verifier_;
-
-    DISALLOW_COPY_AND_ASSIGN(ScriptReloader);
-  };
-
  private:
   // content::NotificationObserver implementation.
   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
   // ExtensionRegistryObserver implementation.
   virtual void OnExtensionLoaded(content::BrowserContext* browser_context,
                                  const Extension* extension) OVERRIDE;
   virtual void OnExtensionUnloaded(
       content::BrowserContext* browser_context,
       const Extension* extension,
       UnloadedExtensionInfo::Reason reason) OVERRIDE;
 
+  // Called once we have finished loading the scripts on the file thread.
+  void OnScriptsLoaded(scoped_ptr<UserScriptList> user_scripts,
+                       scoped_ptr<base::SharedMemory> shared_memory);
+
   // Sends the renderer process a new set of user scripts. If
   // |changed_extensions| is not empty, this signals that only the scripts from
   // those extensions should be updated. Otherwise, all extensions will be
   // updated.
   void SendUpdate(content::RenderProcessHost* process,
                   base::SharedMemory* shared_memory,
                   const std::set<std::string>& changed_extensions);
 
   // Manages our notification registrations.
   content::NotificationRegistrar registrar_;
 
-  // We hang on to our pointer to know if we've already got one running.
-  scoped_refptr<ScriptReloader> script_reloader_;
-
   // Contains the scripts that were found the last time scripts were updated.
   scoped_ptr<base::SharedMemory> shared_memory_;
 
   // List of scripts from currently-installed extensions we should load.
-  UserScriptList user_scripts_;
+  scoped_ptr<UserScriptList> user_scripts_;
 
   // Maps extension info needed for localization to an extension ID.
   ExtensionsInfo extensions_info_;
 
   // The IDs of the extensions which have changed since the last update sent to
   // the renderer.
   std::set<std::string> changed_extensions_;
 
   // If the extensions service has finished loading its initial set of
   // extensions.
   bool extensions_service_ready_;
 
   // If list of user scripts is modified while we're loading it, we note
   // that we're currently mid-load and then start over again once the load
   // finishes.  This boolean tracks whether another load is pending.
   bool pending_load_;
 
+  // Whether or not we are currently loading.
+  bool is_loading_;
+
   // The profile for which the scripts managed here are installed.
   Profile* profile_;
 
   // Listen to extension load, unloaded notifications.
   ScopedObserver<ExtensionRegistry, ExtensionRegistryObserver>
       extension_registry_observer_;
 
+  base::WeakPtrFactory<UserScriptMaster> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(UserScriptMaster);
 };
 
 }  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_USER_SCRIPT_MASTER_H_