Implement <webview>.addContentScript/removeContentScript API [2]

extensions/browser/web_ui_user_script_loader.cc

+// Copyright 2015 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 "extensions/browser/web_ui_user_script_loader.h"
+
+#include "base/bind.h"
+#include "base/pickle.h"
+#include "base/strings/string_util.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/browser_thread.h"
+#include "extensions/browser/content_verifier.h"
+
+namespace {
+using LoadScriptsCallback =
+    base::Callback<void(scoped_ptr<extensions::UserScriptList>,
+                        scoped_ptr<base::SharedMemory>)>;
+
+void SerializeOnFileThread(scoped_ptr<extensions::UserScriptList> user_scripts,
+                           LoadScriptsCallback callback) {
+  scoped_ptr<base::SharedMemory> memory =
+      extensions::UserScriptLoader::Serialize(*user_scripts);
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(callback, base::Passed(&user_scripts), base::Passed(&memory)));
+}
+
+}  // namespace
+
+struct WebUIUserScriptLoader::UserScriptRenderInfo {
+  int render_process_id;
+  int render_view_id;
+
+  UserScriptRenderInfo() : render_process_id(-1), render_view_id(-1) {}
+
+  UserScriptRenderInfo(int render_process_id, int render_view_id)
+      : render_process_id(render_process_id), render_view_id(render_view_id) {}
+
+  bool operator<(const UserScriptRenderInfo& other) {
+    if (render_process_id != other.render_process_id)
+      return render_process_id < other.render_process_id;
+
+    if (render_view_id != other.render_view_id)
+      return render_view_id < other.render_view_id;
+
+    return false;
+  }
+};
+
+WebUIUserScriptLoader::WebUIUserScriptLoader(
+    content::BrowserContext* browser_context,
+    const HostID& host_id)
+    : UserScriptLoader(browser_context, host_id),

[Devlin, 2015/04/21 22:32:20]

you're missing an initialization here

[Xi Han, 2015/04/22 15:32:11]

Got it:)

+      user_scripts_cache_(nullptr),
+      weak_factory_(this) {
+  SetReady(true);
+}
+
+WebUIUserScriptLoader::~WebUIUserScriptLoader() {
+}
+
+void WebUIUserScriptLoader::AddScripts(
+    const std::set<extensions::UserScript>& scripts,
+    int render_process_id,
+    int render_view_id) {
+  UserScriptRenderInfo info(render_process_id, render_view_id);
+  for (const extensions::UserScript& script : scripts) {
+    script_render_info_map_.insert(
+        std::pair<int, UserScriptRenderInfo>(script.id(), info));
+  }
+
+  extensions::UserScriptLoader::AddScripts(scripts);
+}
+
+void WebUIUserScriptLoader::LoadScripts(
+    scoped_ptr<extensions::UserScriptList> user_scripts,
+    const std::set<HostID>& changed_hosts,
+    const std::set<int>& added_script_ids) {
+  user_scripts_cache_.swap(user_scripts);
+  ClearScripts(false);
+
+  // The total number of the tasks is used to trace whether all the fetches
+  // are complete. Therefore, we store all the fetcher pointers in |fetchers_|
+  // before we get theis number. Once we get the total number, start each
+  // fetch tasks.
+  complete_fetchers_ = 0;
+
+  for (extensions::UserScript& script : *user_scripts_cache_) {
+    if (added_script_ids.count(script.id()) == 0)
+      continue;
+
+    int render_process_id = -1;
+    int render_view_id = -1;
+    auto iter = script_render_info_map_.find(script.id());
+    DCHECK(iter != script_render_info_map_.end());
+    render_process_id = iter->second.render_process_id;
+    render_view_id = iter->second.render_view_id;
+
+    for (size_t k = 0; k < script.js_scripts().size(); ++k) {
+      extensions::UserScript::File& file = script.js_scripts()[k];

[Devlin, 2015/04/21 22:32:20]

The fact that you make this temporary implies the for loop would be easier as
for (extensions::UserScript::File& file : scripts.js_scripts()) {
}

[Xi Han, 2015/04/22 15:32:11]

Copy and paste from old code, updated:)

+      if (file.GetContent().empty()) {

[Devlin, 2015/04/21 22:32:20]

Let's extract these lines into a lambda, since they're identical.

[Xi Han, 2015/04/22 15:32:11]

The whole loops are identical (only the difference is js_files or css_files), we
can remove all these duplications. Since it contains several line, I put them
into a private helper function.

+        scoped_ptr<WebUIURLFetcher> fetcher(new WebUIURLFetcher(
+            browser_context(), render_process_id, render_view_id, file.url(),
+            base::Bind(&WebUIUserScriptLoader::OnSingleWebUIURLFetchComplete,
+                       base::Unretained(this), &file)));

[Devlin, 2015/04/21 22:32:20]

You should comment on why the Unretained is safe.

[Xi Han, 2015/04/22 15:32:11]

Done.

+        fetchers_.push_back(fetcher.release());
+      }
+    }
+    for (size_t k = 0; k < script.css_scripts().size(); ++k) {
+      extensions::UserScript::File& file = script.css_scripts()[k];
+      if (file.GetContent().empty()) {
+        scoped_ptr<WebUIURLFetcher> fetcher(new WebUIURLFetcher(
+            browser_context(), render_process_id, render_view_id, file.url(),
+            base::Bind(&WebUIUserScriptLoader::OnSingleWebUIURLFetchComplete,
+                       base::Unretained(this), &file)));
+        fetchers_.push_back(fetcher.release());
+      }
+    }
+    script_render_info_map_.erase(script.id());
+  }
+
+  // If no fetch is needed, call OnWebUIURLFetchComplete directly.
+  if (fetchers_.empty()) {
+    OnWebUIURLFetchComplete();
+    return;
+  }
+  for (auto fetcher : fetchers_)
+    fetcher->Start();
+}
+
+void WebUIUserScriptLoader::OnSingleWebUIURLFetchComplete(
+    extensions::UserScript::File* script_file,
+    bool success,
+    const std::string& data) {
+  if (success) {
+    // Remove BOM from the content.
+    std::string::size_type index = data.find(base::kUtf8ByteOrderMark);
+    if (index == 0)
+      script_file->set_content(data.substr(strlen(base::kUtf8ByteOrderMark)));
+    else
+      script_file->set_content(data);
+  }
+
+  ++complete_fetchers_;
+  if (complete_fetchers_ == fetchers_.size()) {
+    complete_fetchers_ = 0;
+    OnWebUIURLFetchComplete();
+    fetchers_.clear();
+  }
+}
+
+void WebUIUserScriptLoader::OnWebUIURLFetchComplete() {
+  content::BrowserThread::PostTask(
+      content::BrowserThread::FILE, FROM_HERE,
+      base::Bind(&SerializeOnFileThread, base::Passed(&user_scripts_cache_),
+                 base::Bind(&WebUIUserScriptLoader::OnScriptsLoaded,
+                            weak_factory_.GetWeakPtr())));
+  user_scripts_cache_.reset(nullptr);

[Devlin, 2015/04/21 22:32:20]

base::Passed should reset the pointer already.

Also, FYI, you can just do ptr.reset() (you don't need to specify nullptr).

[Xi Han, 2015/04/22 15:32:11]

I see, I will update the place wheRE calling user_scripts.reset(NULL).

+}