Always use mojo_shell in over-http mode

shell/dynamic_application_loader.cc

 // Copyright 2014 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 "shell/dynamic_application_loader.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
 #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"
 #include "mojo/common/common_type_converters.h"
 #include "mojo/common/data_pipe_utils.h"
 #include "mojo/public/cpp/system/data_pipe.h"
 #include "mojo/services/network/public/interfaces/url_loader.mojom.h"
 #include "shell/context.h"
 #include "shell/data_pipe_peek.h"
 #include "shell/filename_util.h"
@@ skipping 76 matching lines @@
 
     // TODO(aa): Sanity check that the thing we got looks vaguely like a mojo
     // application. That could either mean looking for the platform-specific dll
     // header, or looking for some specific mojo signature prepended to the
     // library.
 
     AsPath(context_->task_runners()->blocking_pool(),
            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,
                           GURL* mojo_content_handler_url) {
     std::string shebang;
     if (HasMojoMagic() && PeekFirstLine(&shebang)) {
       GURL url(shebang.substr(arraysize(kMojoMagic) - 1, std::string::npos));
       if (url.is_valid()) {
         *mojo_shebang = shebang;
         *mojo_content_handler_url = url;
@@ skipping 159 matching lines @@
   }
 
   void AsPath(
       base::TaskRunner* task_runner,
       base::Callback<void(const base::FilePath&, bool)> callback) override {
     if (!path_.empty() || !response_) {
       base::MessageLoop::current()->PostTask(
           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 {
     DCHECK(response_);
     return response_->mime_type;
   }
 
   bool HasMojoMagic() override {
     std::string magic;
     return BlockingPeekNBytes(response_->body.get(), &magic, strlen(kMojoMagic),
@@ skipping 27 matching lines @@
       LOG(ERROR) << "Error (" << response->error->code << ": "
                  << response->error->description << ") while fetching "
                  << response->url;
       ReportComplete();
       return;
     }
     response_ = response.Pass();
     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_;
   base::FilePath path_;
   base::WeakPtrFactory<NetworkLoader> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkLoader);
 };
 
 DynamicApplicationLoader::DynamicApplicationLoader(
@@ skipping 47 matching lines @@
   // TODO(darin): What should we do about service errors? This implies that
   // the app closed its handle to the service manager. Maybe we don't care?
 }
 
 void DynamicApplicationLoader::LoaderComplete(Loader* loader) {
   loaders_.erase(std::find(loaders_.begin(), loaders_.end(), loader));
 }
 
 }  // namespace shell
 }  // namespace mojo