Always use mojo_shell in over-http mode

shell/dynamic_application_loader.cc

Index: shell/dynamic_application_loader.cc
diff --git a/shell/dynamic_application_loader.cc b/shell/dynamic_application_loader.cc
index e350b409dea92aff9168d654b726b6be3be3f4da..1cf7e17f4e82d6b7f0f762791ec25e9809699d51 100644
--- a/shell/dynamic_application_loader.cc
+++ b/shell/dynamic_application_loader.cc
@@ -13,6 +13,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
+#include "base/process/process.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -109,7 +110,7 @@ class DynamicApplicationLoader::Loader {
            base::Bind(&Loader::RunLibrary, weak_ptr_factory_.GetWeakPtr()));
   }
 
-  void ReportComplete() { loader_complete_callback_.Run(this); }
+  virtual void ReportComplete() { loader_complete_callback_.Run(this); }
 
  private:
   bool PeekContentHandler(std::string* mojo_shebang,
@@ -289,11 +290,31 @@ class DynamicApplicationLoader::NetworkLoader : public Loader {
           FROM_HERE, base::Bind(callback, path_, base::PathExists(path_)));
       return;
     }
+    // We don't use the created file, just want the directory and random name.
     base::CreateTemporaryFile(&path_);
-    // This is used to extract symbols on android.
-    LOG(INFO) << "Caching mojo app " << url_ << " at " << path_.value();
+    base::DeleteFile(path_, false);

[vtl, 2015/01/21 23:16:21]

This makes me sad, but oh well.

[eseidel, 2015/01/21 23:23:30]

Acknowledged.

+    path_ = path_.AddExtension(".mojo"); // Make libraries easy to spot.

[vtl, 2015/01/21 23:16:21]

nit: Usually we put two spaces before the "//" for same-line comments.

[eseidel, 2015/01/21 23:23:30]

Done.

     common::CopyToFile(response_->body.Pass(), path_, task_runner,
                        base::Bind(callback, path_));
+
+    // This is used to extract symbols on android.
+    // TODO(eseidel): All users of this log should move to using the map file.
+    LOG(INFO) << "Caching mojo app " << url_ << " at " << path_.value();
+
+    // Record the URL -> temp-file mapping for debugging tools.

[vtl, 2015/01/21 23:16:21]

Could you factor this out into a separate function?

[eseidel, 2015/01/21 23:23:30]

Done.

+    base::FilePath temp_dir;
+    GetTempDir(&temp_dir);

[vtl, 2015/01/21 23:16:21]

You should probably say base::GetTempDir, rather than relying on ADL.

[eseidel, 2015/01/21 23:23:30]

Done.

+    base::ProcessId pid = base::Process::Current().pid();
+    std::string map_name = base::StringPrintf("mojo_shell.%d.maps", pid);
+    base::FilePath map_path = temp_dir.Append(map_name);
+
+    std::string map_entry = base::StringPrintf("%s %s\n",

[vtl, 2015/01/21 23:16:21]

Probably we should consider quoting/escaping the path/URL at some point, but we
can worry about that later.

[eseidel, 2015/01/21 23:23:30]

Acknowledged.

+        path_.value().data(), url_.spec().data());
+    // TODO(eseidel): AppendToFile is missing O_CREAT, crbug.com/450696
+    if (!PathExists(map_path))
+      base::WriteFile(map_path, map_entry.data(), map_entry.length());
+    else
+      base::AppendToFile(map_path, map_entry.data(), map_entry.length());
   }
 
   std::string MimeType() override {
@@ -341,6 +362,15 @@ class DynamicApplicationLoader::NetworkLoader : public Loader {
     Load();
   }
 
+  void ReportComplete() override {
+    Loader::ReportComplete();
+    // As soon as we've loaded the library we can delete the tmp file.

[vtl, 2015/01/21 23:16:21]

"tmp" -> "temp"?

[eseidel, 2015/01/21 23:23:30]

Done.

+    // Tools can read the mojo_shell.PID.maps file to find the original library.
+    if (!path_.empty())
+      DeleteFile(path_, false);
+  }
+

[vtl, 2015/01/21 23:16:21]

nit: One fewer blank line.

[eseidel, 2015/01/21 23:23:30]

Done.

+
   const GURL url_;
   URLLoaderPtr url_loader_;
   URLResponsePtr response_;