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) {

[Devlin, 2015/04/22 21:15:33]

Why do we need this?

[Xi Han, 2015/04/22 22:34:00]

We store the info objects in a map, so it requires to overload the "<" operator.

[Devlin, 2015/04/22 23:20:50]

Only the key type of a std::map requires the operator<, not the value type.

[Xi Han, 2015/04/23 15:01:55]

You are right, removed.

+    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),
+      complete_fetchers_(0),
+      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) {

[Devlin, 2015/04/22 21:15:33]

Let's make this LoadScriptsAndSerialize, and I still think I'd prefer it to take
a callback (so that OnScriptsLoaded() can be private again).

[Xi Han, 2015/04/22 22:34:00]

A compromise is we have to declare the callback type as a public type of
UserScriptLoader, since there are some functions in the anonymous namespace
refers to it.

[Devlin, 2015/04/22 23:20:50]

I don't understand.  Why?

LoadUserScriptsAndSerialize(Callback on_complete) {
  callback_ = on_complete;
  FetchStuff();
}

OnFetchComplete() {
  SerializeOnFileThread(callback_);
  callback_.Reset();
}

SerializeOnFileThread(Callback c) {
  Serialize();
  c.Run();
}

[Xi Han, 2015/04/23 15:01:55]

I removed the declaration of LoadScriptsCallback from this file, and declare it
in the UserScriptLoader, because the private LoadScripts() needs to know this
type as well:
virtual void LoadScripts(scoped_ptr<UserScriptList> user_scripts,
                         const std::set<HostID>& changed_hosts,
                         const std::set<int>& added_script_ids,
                         LoadScriptsCallback callback) = 0;

However, SerializeOnFileThread(..., UserScriptLoader::LoadScriptsCallback) (in
this file and LoadScriptsOnFileThread(...) in the ExtensionUserScriptLoader)
also needs UserScriptLoader::LoadScriptsCallback in the anonymous namespace. So
I have to declare LoadScriptsCallback as public.

Does it sound good to you?

[Devlin, 2015/04/23 16:34:38]

Yeah, there's nothing wrong with having the typedef be public.

+  user_scripts_cache_.swap(user_scripts);
+
+  // 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;
+
+    auto iter = script_render_info_map_.find(script.id());
+    DCHECK(iter != script_render_info_map_.end());
+    int render_process_id = iter->second.render_process_id;
+    int render_view_id = iter->second.render_view_id;
+
+    CreateWebUIURLFetchers(&script.js_scripts(), render_process_id,

[Devlin, 2015/04/22 21:15:33]

Why not pass by const &?

[Xi Han, 2015/04/22 22:34:00]

Once the fetch is done, we will modify the file by copying the data to set its
content. We can't pass in a const &.

+                           render_view_id);
+    CreateWebUIURLFetchers(&script.css_scripts(), render_process_id,
+                           render_view_id);
+
+    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::CreateWebUIURLFetchers(
+    extensions::UserScript::FileList* script_files,
+    int render_process_id,
+    int render_view_id) {
+  for (extensions::UserScript::File& file : *script_files) {
+    if (file.GetContent().empty()) {
+      // The WebUIUserScriptLoader owns these WebUIURLFetchers. Once the
+      // loader is destroyed, all the fetchers will be destroyed. Therefore,
+      // we are sure it is safe to use base::Unretained(this) here.
+      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());
+    }
+  }
+}
+
+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())));
+}