[Extensions Cleanup] Rename some UserScriptLoader members

Rename some UserScriptLoader members to be a bit more clear.
- Rename "pending_load_" to "queued_load_". This better explains that a
  load is queued up, but not yet started.
- Rename "user_scripts_" to "loaded_scripts_". This more accurately
  indicates that the scripts contained within |loaded_scripts_| are the
  ones that have been fully loaded, rather than those that are queued
  up to load.

Also expose a UserScriptLoader getter on SharedUserScriptMaster, and
remove the forwarding method to UserScriptLoader::initial_load_complete.

Bug: None
Change-Id: I9276387a888572cf3e9575b5e2a2a1ee4dd47e07
Reviewed-on: https://chromium-review.googlesource.com/c/1387394Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618364}

chrome/browser/extensions/extension_startup_browsertest.cc

@@ -37,6 +37,7 @@
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/shared_user_script_master.h"
+#include "extensions/browser/user_script_loader.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/feature_switch.h"
@@ -155,9 +156,9 @@ class ExtensionStartupTestBase : public InProcessBrowserTest {
     extensions::SharedUserScriptMaster* master =
         extensions::ExtensionSystem::Get(browser()->profile())
             ->shared_user_script_master();
-    if (!master->initial_load_complete())
+    if (!master->script_loader()->initial_load_complete())
       user_scripts_observer.Wait();
-    ASSERT_TRUE(master->initial_load_complete());
+    ASSERT_TRUE(master->script_loader()->initial_load_complete());
   }
 
   void TestInjection(bool expect_css, bool expect_script) {

extensions/browser/shared_user_script_master.h

@@ -31,9 +31,7 @@ class SharedUserScriptMaster : public ExtensionRegistryObserver {
   explicit SharedUserScriptMaster(content::BrowserContext* browser_context);
   ~SharedUserScriptMaster() override;
 
-  // Returns true if the initial load of static content scripts for extensions
-  // is complete.
-  bool initial_load_complete() const { return loader_.initial_load_complete(); }
+  UserScriptLoader* script_loader() { return &loader_; }
 
  private:
   // ExtensionRegistryObserver implementation.

extensions/browser/user_script_loader.cc

@@ -157,10 +157,10 @@ bool UserScriptLoader::ParseMetadataHeader(const base::StringPiece& script_text,
 
 UserScriptLoader::UserScriptLoader(BrowserContext* browser_context,
                                    const HostID& host_id)
-    : user_scripts_(new UserScriptList()),
+    : loaded_scripts_(new UserScriptList()),
       clear_scripts_(false),
       ready_(false),
-      pending_load_(false),
+      queued_load_(false),
       browser_context_(browser_context),
       host_id_(host_id),
       weak_factory_(this) {
@@ -202,7 +202,7 @@ void UserScriptLoader::RemoveScripts(
     removed_script_hosts_.insert(UserScriptIDPair(id_pair.id, id_pair.host_id));
     // TODO(lazyboy): We shouldn't be trying to remove scripts that were never
     // a) added to |added_scripts_map_| or b) being loaded or has done loading
-    // through |user_scripts_|. This would reduce sending redundant IPC.
+    // through |loaded_scripts_|. This would reduce sending redundant IPC.
     added_scripts_map_.erase(id_pair.id);
   }
   AttemptLoad();
@@ -237,13 +237,13 @@ bool UserScriptLoader::ScriptsMayHaveChanged() const {
   //     scripts that need to be cleared), or
   // (2) The current set of scripts is non-empty (so they need to be cleared).
   return (added_scripts_map_.size() || removed_script_hosts_.size() ||
-          (clear_scripts_ && (is_loading() || user_scripts_->size())));
+          (clear_scripts_ && (is_loading() || loaded_scripts_->size())));
 }
 
 void UserScriptLoader::AttemptLoad() {
   if (ready_ && ScriptsMayHaveChanged()) {
     if (is_loading())
-      pending_load_ = true;
+      queued_load_ = true;
     else
       StartLoad();
   }
@@ -253,22 +253,27 @@ void UserScriptLoader::StartLoad() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(!is_loading());
 
-  // If scripts were marked for clearing before adding and removing, then clear
-  // them.
+  // Reload any loaded scripts, and clear out |loaded_scripts_| to indicate that
+  // the scripts aren't currently ready.
+  std::unique_ptr<UserScriptList> scripts_to_load = std::move(loaded_scripts_);
+
   if (clear_scripts_) {
-    user_scripts_->clear();
+    // If scripts were marked for clearing before adding and removing, then
+    // clear them...
+    scripts_to_load->clear();
   } else {
-    for (auto it = user_scripts_->begin(); it != user_scripts_->end();) {
+    // ... otherwise, filter out any scripts that are queued for removal.
+    for (auto it = scripts_to_load->begin(); it != scripts_to_load->end();) {
       UserScriptIDPair id_pair(it->get()->id());
       if (removed_script_hosts_.count(id_pair) > 0u)
-        it = user_scripts_->erase(it);
+        it = scripts_to_load->erase(it);
       else
         ++it;
     }
   }
 
   std::set<int> added_script_ids;
-  user_scripts_->reserve(user_scripts_->size() + added_scripts_map_.size());
+  scripts_to_load->reserve(scripts_to_load->size() + added_scripts_map_.size());
   for (auto& id_and_script : added_scripts_map_) {
     std::unique_ptr<UserScript>& script = id_and_script.second;
     added_script_ids.insert(script->id());
@@ -276,20 +281,19 @@ void UserScriptLoader::StartLoad() {
     // its IPC message. This must be done before we clear |added_scripts_map_|
     // and |removed_script_hosts_| below.
     changed_hosts_.insert(script->host_id());
-    // Move script from |added_scripts_map_| into |user_scripts_|.
-    user_scripts_->push_back(std::move(script));
+    // Move script from |added_scripts_map_| into |scripts_to_load|.
+    scripts_to_load->push_back(std::move(script));
   }
   for (const UserScriptIDPair& id_pair : removed_script_hosts_)
     changed_hosts_.insert(id_pair.host_id);
 
-  LoadScripts(std::move(user_scripts_), changed_hosts_, added_script_ids,
+  LoadScripts(std::move(scripts_to_load), changed_hosts_, added_script_ids,
               base::Bind(&UserScriptLoader::OnScriptsLoaded,
                          weak_factory_.GetWeakPtr()));
 
   clear_scripts_ = false;
   added_scripts_map_.clear();
   removed_script_hosts_.clear();
-  user_scripts_.reset();
 }
 
 // static
@@ -362,11 +366,11 @@ void UserScriptLoader::SetReady(bool ready) {
 void UserScriptLoader::OnScriptsLoaded(
     std::unique_ptr<UserScriptList> user_scripts,
     std::unique_ptr<base::SharedMemory> shared_memory) {
-  user_scripts_ = std::move(user_scripts);
-  if (pending_load_) {
+  loaded_scripts_ = std::move(user_scripts);
+  if (queued_load_) {
     // While we were loading, there were further changes. Don't bother
     // notifying about these scripts and instead just immediately reload.
-    pending_load_ = false;
+    queued_load_ = false;
     StartLoad();
     return;
   }

extensions/browser/user_script_loader.h

@@ -136,8 +136,8 @@ class UserScriptLoader : public content::NotificationObserver {
                   const std::set<HostID>& changed_hosts);
 
   bool is_loading() const {
-    // Ownership of |user_scripts_| is passed to the file thread when loading.
-    return user_scripts_.get() == nullptr;
+    // |loaded_scripts_| is reset when loading.
+    return loaded_scripts_.get() == nullptr;
   }
 
   // Manages our notification registrations.
@@ -146,8 +146,9 @@ class UserScriptLoader : public content::NotificationObserver {
   // Contains the scripts that were found the last time scripts were updated.
   std::unique_ptr<base::SharedMemory> shared_memory_;
 
-  // List of scripts from currently-installed extensions we should load.
-  std::unique_ptr<UserScriptList> user_scripts_;
+  // List of scripts that are currently loaded. This is null when a load is in
+  // progress.
+  std::unique_ptr<UserScriptList> loaded_scripts_;
 
   // The mutually-exclusive information about sets of scripts that were added or
   // removed since the last script load. These maps are keyed by script ids.
@@ -169,7 +170,7 @@ class UserScriptLoader : public content::NotificationObserver {
   // 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_;
+  bool queued_load_;
 
   // The browser_context for which the scripts managed here are installed.
   content::BrowserContext* browser_context_;