Move parsing of metadata header into browser process. This is a prerequisite

to getting user scripts working in extensions because extensions won't express
their metadata using the UserScript header, so parsing can't be done in the
renderer.

Review URL: http://codereview.chromium.org/18308

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@8249 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 223 insertions, 191 deletions

diff --git a/chrome/browser/extensions/extension_protocols.cc b/chrome/browser/extensions/extension_protocols.cc
index 8f6957d..1643c4d6 100644
--- a/chrome/browser/extensions/extension_protocols.cc
+++ b/chrome/browser/extensions/extension_protocols.cc
@@ -10,8 +10,8 @@
 #include "net/base/net_util.h"
 #include "net/url_request/url_request_file_job.h"
 
-static const char kExtensionURLScheme[] = "chrome-extension";
-static const char kUserScriptURLScheme[] = "chrome-user-script";
+const char kExtensionURLScheme[] = "chrome-extension";
+const char kUserScriptURLScheme[] = "chrome-user-script";
 
 FilePath GetPathForExtensionResource(const FilePath& extension_path,
                                      const std::string& url_path) {
diff --git a/chrome/browser/extensions/extension_protocols.h b/chrome/browser/extensions/extension_protocols.h
index 80fa654..25f9fda 100644
--- a/chrome/browser/extensions/extension_protocols.h
+++ b/chrome/browser/extensions/extension_protocols.h
@@ -7,6 +7,12 @@
 
 #include "base/file_path.h"
 
+// The URL scheme Chromium extensions are served from.
+extern const char kExtensionURLScheme[];
+
+// The URL scheme Chromium user scripts are served from.
+extern const char kUserScriptURLScheme[];
+
 // Gets a FilePath for a resource inside an extension. |extension_path| is the
 // full path to the extension directory. |resource_path| is the path to the
 // resource from the extension root, including the leading '/'.
diff --git a/chrome/browser/extensions/user_script_master.cc b/chrome/browser/extensions/user_script_master.cc
index dcd912a..b95a1d3 100644
--- a/chrome/browser/extensions/user_script_master.cc
+++ b/chrome/browser/extensions/user_script_master.cc
@@ -9,69 +9,66 @@
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/logging.h"
-#include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/pickle.h"
 #include "base/string_util.h"
+#include "chrome/browser/extensions/extension_protocols.h"
 #include "chrome/common/notification_service.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/net_util.h"
 
-// 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 scan
-// on the file thread.
-// It must be created on, and its public API must only be called from,
-// the master's thread.
-class UserScriptMaster::ScriptReloader
-    : public base::RefCounted<UserScriptMaster::ScriptReloader> {
- public:
-  ScriptReloader(UserScriptMaster* master)
-       : master_(master), master_message_loop_(MessageLoop::current()) {}
-
-  // Start a scan for scripts.
-  // Will always send a message to the master upon completion.
-  void StartScan(MessageLoop* work_loop, const FilePath& script_dir);
-
-  // The master is going away; don't call it back.
-  void DisownMaster() {
-    master_ = NULL;
+// static
+void UserScriptMaster::ScriptReloader::ParseMetadataHeader(
+      const StringPiece& script_text, std::vector<std::string> *includes) {
+  // http://wiki.greasespot.net/Metadata_block
+  StringPiece line;
+  size_t line_start = 0;
+  size_t line_end = 0;
+  bool in_metadata = false;
+
+  static const StringPiece kUserScriptBegin("// ==UserScript==");
+  static const StringPiece kUserScriptEng("// ==/UserScript==");
+  static const StringPiece kIncludeDeclaration("// @include ");
+
+  while (line_start < script_text.length()) {
+    line_end = script_text.find('\n', line_start);
+
+    // Handle the case where there is no trailing newline in the file.
+    if (line_end == std::string::npos) {
+      line_end = script_text.length() - 1;
+    }
+
+    line.set(script_text.data() + line_start, line_end - line_start);
+
+    if (!in_metadata) {
+      if (line.starts_with(kUserScriptBegin)) {
+        in_metadata = true;
+      }
+    } else {
+      if (line.starts_with(kUserScriptEng)) {
+        break;
+      }
+
+      if (line.starts_with(kIncludeDeclaration)) {
+        std::string pattern(line.data() + kIncludeDeclaration.length(),
+                            line.length() - kIncludeDeclaration.length());
+        std::string pattern_trimmed;
+        TrimWhitespace(pattern, TRIM_ALL, &pattern_trimmed);
+        includes->push_back(pattern_trimmed);
+      }
+
+      // TODO(aa): Handle more types of metadata.
+    }
+
+    line_start = line_end + 1;
   }
 
- private:
-  // Where functions are run:
-  //    master          file
-  //   StartScan   ->  RunScan
-  //                     GetNewScripts()
-  // NotifyMaster  <-  RunScan
-
-  // Runs on the master thread.
-  // Notify the master that new scripts are available.
-  void NotifyMaster(base::SharedMemory* memory);
-
-  // Runs on the File thread.
-  // Scan the script directory for scripts, calling NotifyMaster when done.
-  // The path is intentionally passed by value so its lifetime isn't tied
-  // to the caller.
-  void RunScan(const FilePath script_dir);
-
-  // Runs on the File thread.
-  // Scan the script directory for scripts, returning either a new SharedMemory
-  // or NULL on error.
-  base::SharedMemory* GetNewScripts(const FilePath& script_dir);
-
-  // A pointer back to our master.
-  // May be NULL if DisownMaster() is called.
-  UserScriptMaster* master_;
-
-  // The message loop to call our master back on.
-  // Expected to always outlive us.
-  MessageLoop* master_message_loop_;
-
-  DISALLOW_COPY_AND_ASSIGN(ScriptReloader);
-};
+  // If no @include patterns were specified, default to @include *.
+  // This is what Greasemonkey does.
+  if (includes->size() == 0) {
+    includes->push_back("*");
+  }
+}
 
 void UserScriptMaster::ScriptReloader::StartScan(
     MessageLoop* work_loop,
@@ -129,15 +126,26 @@ base::SharedMemory* UserScriptMaster::ScriptReloader::GetNewScripts(
   pickle.WriteSize(scripts.size());
   for (std::vector<std::wstring>::iterator path = scripts.begin();
        path != scripts.end(); ++path) {
-    std::string file_url = net::FilePathToFileURL(*path).spec();
+    std::string url(kUserScriptURLScheme);
+    url += ":/";
+    url += net::FilePathToFileURL(*path).ExtractFileName();
+
     std::string contents;
     // TODO(aa): Support unicode script files.
     file_util::ReadFileToString(*path, &contents);
 
+    std::vector<std::string> includes;
+    ParseMetadataHeader(contents, &includes);
+
     // Write scripts as 'data' so that we can read it out in the slave without
     // allocating a new string.
-    pickle.WriteData(file_url.c_str(), file_url.length());
+    pickle.WriteData(url.c_str(), url.length());
     pickle.WriteData(contents.c_str(), contents.length());
+    pickle.WriteSize(includes.size());
+    for (std::vector<std::string>::iterator iter = includes.begin();
+         iter != includes.end(); ++iter) {
+      pickle.WriteString(*iter);
+    }
   }
 
   // Create the shared memory object.
@@ -160,7 +168,6 @@ base::SharedMemory* UserScriptMaster::ScriptReloader::GetNewScripts(
   return shared_memory.release();
 }
 
-
 UserScriptMaster::UserScriptMaster(MessageLoop* worker_loop,
                                        const FilePath& script_dir)
     : user_script_dir_(new FilePath(script_dir)),
diff --git a/chrome/browser/extensions/user_script_master.h b/chrome/browser/extensions/user_script_master.h
index 4798bb2..8df49be 100644
--- a/chrome/browser/extensions/user_script_master.h
+++ b/chrome/browser/extensions/user_script_master.h
@@ -7,11 +7,11 @@
 
 #include "base/directory_watcher.h"
 #include "base/file_path.h"
+#include "base/message_loop.h"
 #include "base/process.h"
 #include "base/scoped_ptr.h"
 #include "base/shared_memory.h"
-
-class MessageLoop;
+#include "base/string_piece.h"
 
 // Manages a segment of shared memory that contains the user scripts the user
 // has installed.  Lives on the UI thread.
@@ -39,7 +39,68 @@ class UserScriptMaster : public base::RefCounted<UserScriptMaster>,
   FilePath user_script_dir() const { return *user_script_dir_; }
 
  private:
-  class ScriptReloader;
+  FRIEND_TEST(UserScriptMasterTest, Parse1);
+  FRIEND_TEST(UserScriptMasterTest, Parse2);
+  FRIEND_TEST(UserScriptMasterTest, Parse3);
+
+  // 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 scan
+  // 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::RefCounted<UserScriptMaster::ScriptReloader> {
+   public:
+    // Parses the includes out of |script| and returns them in |includes|.
+    static void ParseMetadataHeader(const StringPiece& script,
+                                    std::vector<std::string>* includes);
+
+    ScriptReloader(UserScriptMaster* master)
+         : master_(master), master_message_loop_(MessageLoop::current()) {}
+
+    // Start a scan for scripts.
+    // Will always send a message to the master upon completion.
+    void StartScan(MessageLoop* work_loop, const FilePath& script_dir);
+
+    // The master is going away; don't call it back.
+    void DisownMaster() {
+      master_ = NULL;
+    }
+
+   private:
+    // Where functions are run:
+    //    master          file
+    //   StartScan   ->  RunScan
+    //                     GetNewScripts()
+    //                     ParseMetadataHeader()
+    // NotifyMaster  <-  RunScan
+
+    // Runs on the master thread.
+    // Notify the master that new scripts are available.
+    void NotifyMaster(base::SharedMemory* memory);
+
+    // Runs on the File thread.
+    // Scan the script directory for scripts, calling NotifyMaster when done.
+    // The path is intentionally passed by value so its lifetime isn't tied
+    // to the caller.
+    void RunScan(const FilePath script_dir);
+
+    // Runs on the File thread.
+    // Scan the script directory for scripts, returning either a
+    // new SharedMemory or NULL on error.
+    base::SharedMemory* GetNewScripts(const FilePath& script_dir);
+
+    // A pointer back to our master.
+    // May be NULL if DisownMaster() is called.
+    UserScriptMaster* master_;
+
+    // The message loop to call our master back on.
+    // Expected to always outlive us.
+    MessageLoop* master_message_loop_;
+
+    DISALLOW_COPY_AND_ASSIGN(ScriptReloader);
+  };
 
   // DirectoryWatcher::Delegate implementation.
   virtual void OnDirectoryChanged(const FilePath& path);
diff --git a/chrome/browser/extensions/user_script_master_unittest.cc b/chrome/browser/extensions/user_script_master_unittest.cc
index 8e3b95d..4c11661 100644
--- a/chrome/browser/extensions/user_script_master_unittest.cc
+++ b/chrome/browser/extensions/user_script_master_unittest.cc
@@ -116,3 +116,50 @@ TEST_F(UserScriptMasterTest, ExistingScripts) {
 
   ASSERT_TRUE(shared_memory_ != NULL);
 }
+
+TEST_F(UserScriptMasterTest, Parse1) {
+  const std::string text(
+    "// This is my awesome script\n"
+    "// It does stuff.\n"
+    "// ==UserScript==   trailing garbage\n"
+    "// @name foobar script\n"
+    "// @namespace http://www.google.com/\n"
+    "// @include *mail.google.com*\n"
+    "// \n"
+    "// @othergarbage\n"
+    "// @include *mail.yahoo.com*\r\n"
+    "// @include  \t *mail.msn.com*\n" // extra spaces after "@include" OK
+    "//@include not-recognized\n" // must have one space after "//"
+    "// ==/UserScript==  trailing garbage\n"
+    "\n"
+    "\n"
+    "alert('hoo!');\n");
+
+  std::vector<std::string> includes;
+  UserScriptMaster::ScriptReloader::ParseMetadataHeader(text, &includes);
+  EXPECT_EQ(3U, includes.size());
+  EXPECT_EQ("*mail.google.com*", includes[0]);
+  EXPECT_EQ("*mail.yahoo.com*", includes[1]);
+  EXPECT_EQ("*mail.msn.com*", includes[2]);
+}
+
+TEST_F(UserScriptMasterTest, Parse2) {
+  const std::string text("default to @include *");
+
+  std::vector<std::string> includes;
+  UserScriptMaster::ScriptReloader::ParseMetadataHeader(text, &includes);
+  EXPECT_EQ(1U, includes.size());
+  EXPECT_EQ("*", includes[0]);
+}
+
+TEST_F(UserScriptMasterTest, Parse3) {
+  const std::string text(
+    "// ==UserScript==\n"
+    "// @include *foo*\n"
+    "// ==/UserScript=="); // no trailing newline
+
+  std::vector<std::string> includes;
+  UserScriptMaster::ScriptReloader::ParseMetadataHeader(text, &includes);
+  EXPECT_EQ(1U, includes.size());
+  EXPECT_EQ("*foo*", includes[0]);
+}
diff --git a/chrome/renderer/user_script_slave.cc b/chrome/renderer/user_script_slave.cc
index 52d9529..d227d5e 100644
--- a/chrome/renderer/user_script_slave.cc
+++ b/chrome/renderer/user_script_slave.cc
@@ -18,16 +18,6 @@ static const char kUserScriptTail[] = "\n})(window);";
 
 // UserScript
 
-void UserScript::Parse(const StringPiece& script_text) {
-  ParseMetadata(script_text);
-
-  // TODO(aa): Set body to just the part after the metadata block? This would
-  // significantly cut down on the size of the injected script in some cases.
-  // Would require remembering the line number the body begins at, for correct
-  // error line number reporting.
-  body_ = script_text;
-}
-
 bool UserScript::MatchesUrl(const GURL& url) {
   for (std::vector<std::string>::iterator pattern = include_patterns_.begin();
        pattern != include_patterns_.end(); ++pattern) {
@@ -39,57 +29,6 @@ bool UserScript::MatchesUrl(const GURL& url) {
   return false;
 }
 
-void UserScript::ParseMetadata(const StringPiece& script_text) {
-  // http://wiki.greasespot.net/Metadata_block
-  StringPiece line;
-  size_t line_start = 0;
-  size_t line_end = 0;
-  bool in_metadata = false;
-
-  static const StringPiece kUserScriptBegin("// ==UserScript==");
-  static const StringPiece kUserScriptEng("// ==/UserScript==");
-  static const StringPiece kIncludeDeclaration("// @include ");
-
-  while (line_start < script_text.length()) {
-    line_end = script_text.find('\n', line_start);
-
-    // Handle the case where there is no trailing newline in the file.
-    if (line_end == std::string::npos) {
-      line_end = script_text.length() - 1;
-    }
-
-    line.set(script_text.data() + line_start, line_end - line_start);
-
-    if (!in_metadata) {
-      if (line.starts_with(kUserScriptBegin)) {
-        in_metadata = true;
-      }
-    } else {
-      if (line.starts_with(kUserScriptEng)) {
-        break;
-      }
-
-      if (line.starts_with(kIncludeDeclaration)) {
-        std::string pattern(line.data() + kIncludeDeclaration.length(),
-                            line.length() - kIncludeDeclaration.length());
-        std::string pattern_trimmed;
-        TrimWhitespace(pattern, TRIM_ALL, &pattern_trimmed);
-        AddInclude(pattern_trimmed);
-      }
-
-      // TODO(aa): Handle more types of metadata.
-    }
-
-    line_start = line_end + 1;
-  }
-
-  // If no @include patterns were specified, default to @include *.
-  // This is what Greasemonkey does.
-  if (include_patterns_.size() == 0) {
-    AddInclude("*");
-  }
-}
-
 void UserScript::AddInclude(const std::string &glob_pattern) {
   include_patterns_.push_back(EscapeGlob(glob_pattern));
 }
@@ -160,12 +99,12 @@ bool UserScriptSlave::UpdateScripts(base::SharedMemoryHandle shared_memory) {
 
   // Unpickle scripts.
   void* iter = NULL;
-  int num_scripts = 0;
+  size_t num_scripts = 0;
   Pickle pickle(reinterpret_cast<char*>(shared_memory_->memory()),
                 pickle_size);
-  pickle.ReadInt(&iter, &num_scripts);
+  pickle.ReadSize(&iter, &num_scripts);
 
-  for (int i = 0; i < num_scripts; ++i) {
+  for (size_t i = 0; i < num_scripts; ++i) {
     const char* url = NULL;
     int url_length = 0;
     const char* body = NULL;
@@ -174,9 +113,17 @@ bool UserScriptSlave::UpdateScripts(base::SharedMemoryHandle shared_memory) {
     pickle.ReadData(&iter, &url, &url_length);
     pickle.ReadData(&iter, &body, &body_length);
 
-    scripts_.push_back(UserScript(StringPiece(url, url_length)));
+    scripts_.push_back(UserScript(StringPiece(url, url_length),
+                                  StringPiece(body, body_length)));
     UserScript& script = scripts_.back();
-    script.Parse(StringPiece(body, body_length));
+
+    size_t num_includes;
+    pickle.ReadSize(&iter, &num_includes);
+    for (size_t j = 0; j < num_includes; ++j) {
+      std::string include;
+      pickle.ReadString(&iter, &include);
+      script.AddInclude(include);
+    }
   }
 
   return true;
diff --git a/chrome/renderer/user_script_slave.h b/chrome/renderer/user_script_slave.h
index 21b67dc..b4558a6 100644
--- a/chrome/renderer/user_script_slave.h
+++ b/chrome/renderer/user_script_slave.h
@@ -16,21 +16,23 @@
 // Parsed representation of a user script.
 class UserScript {
  public:
-  UserScript(const StringPiece& script_url)
-    : url_(script_url) {}
-
-  // Gets the script body that should be injected into matching content.
-  const StringPiece& GetBody() const {
-    return body_;
-  }
+  UserScript(const StringPiece& script_url,
+             const StringPiece& body)
+    : url_(script_url), body_(body) {}
 
   // Gets a URL where this script can be found.
   const StringPiece& GetURL() const {
     return url_;
   }
 
-  // Parses the text content of a user script file.
-  void Parse(const StringPiece& script_text);
+  // Gets the script body that should be injected into matching content.
+  const StringPiece& GetBody() const {
+    return body_;
+  }
+
+  // Adds an include pattern that will be checked to determine whether to
+  // include a script on a given page.
+  void AddInclude(const std::string &glob_pattern);
 
   // Returns true if the script should be applied to the specified URL, false
   // otherwise.
@@ -38,30 +40,20 @@ class UserScript {
 
  private:
   FRIEND_TEST(UserScriptSlaveTest, EscapeGlob);
-  FRIEND_TEST(UserScriptSlaveTest, Parse1);
-  FRIEND_TEST(UserScriptSlaveTest, Parse2);
-  FRIEND_TEST(UserScriptSlaveTest, Parse3);
 
   // Helper function to convert the user script glob format to the patterns
   // used internally to test URLs.
   static std::string EscapeGlob(const std::string& glob);
 
-  // Parses the metadata block from the script.
-  void ParseMetadata(const StringPiece& script_text);
-
-  // Adds an include pattern that will be checked to determine whether to
-  // include a script on a given page.
-  void AddInclude(const std::string &glob_pattern);
+  // The url of the file the script came from. This references shared_memory_,
+  // and is valid until that memory is either deleted or Unmap()'d.
+  StringPiece url_;
 
   // The body of the script, which will be injected into content pages. This
   // references shared_memory_, and is valid until that memory is either
   // deleted or Unmap()'d.
   StringPiece body_;
 
-  // The url of the file the script came from. This references shared_memory_,
-  // and is valid until that memory is either deleted or Unmap()'d.
-  StringPiece url_;
-
   // List of patterns to test URLs against for this script. These patterns have
   // been escaped for use with MatchPattern() in string_utils ('?' and '\' are
   // escaped).
diff --git a/chrome/renderer/user_script_slave_unittest.cc b/chrome/renderer/user_script_slave_unittest.cc
index 57d34a9..eeea749 100644
--- a/chrome/renderer/user_script_slave_unittest.cc
+++ b/chrome/renderer/user_script_slave_unittest.cc
@@ -16,28 +16,11 @@ TEST(UserScriptSlaveTest, EscapeGlob) {
             UserScript::EscapeGlob("foo\\bar?hot=dog"));

 }

 

-TEST(UserScriptSlaveTest, Parse1) {

-  const std::string text(

-    "// This is my awesome script\n"

-    "// It does stuff.\n"

-    "// ==UserScript==   trailing garbage\n"

-    "// @name foobar script\n"

-    "// @namespace http://www.google.com/\n"

-    "// @include *mail.google.com*\n"

-    "// \n"

-    "// @othergarbage\n"

-    "// @include *mail.yahoo.com*\r\n"

-    "// @include  \t *mail.msn.com*\n" // extra spaces after "@include" OK

-    "//@include not-recognized\n" // must have one space after "//"

-    "// ==/UserScript==  trailing garbage\n"

-    "\n"

-    "\n"

-    "alert('hoo!');\n");

-

-  UserScript script("foo");

-  script.Parse(text);

-  EXPECT_EQ(3U, script.include_patterns_.size());

-  EXPECT_EQ(text, script.GetBody());

+TEST(UserScriptSlaveTest, Match1) {

+  UserScript script("foo", "bar");

+  script.AddInclude("*mail.google.com*");

+  script.AddInclude("*mail.yahoo.com*");

+  script.AddInclude("*mail.msn.com*");

   EXPECT_TRUE(script.MatchesUrl(GURL("http://mail.google.com")));

   EXPECT_TRUE(script.MatchesUrl(GURL("http://mail.google.com/foo")));

   EXPECT_TRUE(script.MatchesUrl(GURL("http://mail.yahoo.com/bar")));

@@ -45,27 +28,16 @@ TEST(UserScriptSlaveTest, Parse1) {
   EXPECT_FALSE(script.MatchesUrl(GURL("http://www.hotmail.com")));

 }

 

-TEST(UserScriptSlaveTest, Parse2) {

-  const std::string text("default to @include *");

-

-  UserScript script("foo");

-  script.Parse(text);

-  EXPECT_EQ(1U, script.include_patterns_.size());

-  EXPECT_EQ(text, script.GetBody());

-  EXPECT_TRUE(script.MatchesUrl(GURL("foo")));

-  EXPECT_TRUE(script.MatchesUrl(GURL("bar")));

+TEST(UserScriptSlaveTest, Match2) {

+  UserScript script("foo", "bar");

+  script.AddInclude("*");

+  EXPECT_TRUE(script.MatchesUrl(GURL("http://foo.com/bar")));

+  EXPECT_TRUE(script.MatchesUrl(GURL("http://hot.com/dog")));

 }

 

-TEST(UserScriptSlaveTest, Parse3) {

-  const std::string text(

-    "// ==UserScript==\n"

-    "// @include *foo*\n"

-    "// ==/UserScript=="); // no trailing newline

-

-  UserScript script("foo");

-  script.Parse(text);

-  EXPECT_EQ(1U, script.include_patterns_.size());

-  EXPECT_EQ(text, script.GetBody());

+TEST(UserScriptSlaveTest, Match3) {

+  UserScript script("foo", "bar");

+  script.AddInclude("*foo*");

   EXPECT_TRUE(script.MatchesUrl(GURL("http://foo.com/bar")));

   EXPECT_FALSE(script.MatchesUrl(GURL("http://baz.org")));

 }