Allow local file to run though content handler.

mojo/shell/dynamic_application_loader.cc

Index: mojo/shell/dynamic_application_loader.cc
diff --git a/mojo/shell/dynamic_application_loader.cc b/mojo/shell/dynamic_application_loader.cc
index 3f5eaba0ae60ca2f2e9c84aa56ee7526adbe11eb..4c7aa1eee9489121cacfe71bb28010a251517a4d 100644
--- a/mojo/shell/dynamic_application_loader.cc
+++ b/mojo/shell/dynamic_application_loader.cc
@@ -8,13 +8,17 @@
 #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/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/public/interfaces/network/url_loader.mojom.h"
 #include "mojo/shell/context.h"
 #include "mojo/shell/data_pipe_peek.h"
@@ -25,77 +29,182 @@
 namespace mojo {
 namespace shell {
 
-// Encapsulates loading and running one individual application.
-//
-// Loaders are owned by DynamicApplicationLoader. DynamicApplicationLoader must
-// ensure that all the parameters passed to Loader subclasses stay valid through
-// Loader's lifetime.
-//
-// Async operations are done with WeakPtr to protect against
-// DynamicApplicationLoader going away (and taking all the Loaders with it)
-// while the async operation is outstanding.
-class DynamicApplicationLoader::Loader {
+namespace {
+
+static const char kMojoMagic[] = "#!mojo:";
+static const size_t kMaxShebangLength = 2048;
+
+void IgnoreResult(bool result) {
+}
+
+// A request to load a given URL.
+class LoaderRequest {
  public:
-  Loader(Context* context,
-         DynamicServiceRunnerFactory* runner_factory,
-         scoped_refptr<ApplicationLoader::LoadCallbacks> load_callbacks,
-         const LoaderCompleteCallback& loader_complete_callback)
-      : load_callbacks_(load_callbacks),
-        loader_complete_callback_(loader_complete_callback),
-        context_(context),
-        runner_factory_(runner_factory),
-        weak_ptr_factory_(this) {}
+  virtual ~LoaderRequest() {}
 
-  virtual ~Loader() {}
+  virtual URLResponsePtr AsURLResponse(base::TaskRunner* task_runner,
+                                       uint32_t skip) = 0;
+
+  virtual void AsPath(
+      base::TaskRunner* task_runner,
+      base::Callback<void(const base::FilePath&, bool)> callback) = 0;
+
+  bool PeekContentHandler(std::string* mojo_shebang,

[Aaron Boodman, 2014/11/07 08:56:27]

I would prefer this function in loader, and make LoaderRequest a pure interface.

+                          GURL* mojo_content_handler_url) {
+    std::string shebang;
+    if (HasMojoMagic() && PeekFirstLine(&shebang)) {
+      GURL url(shebang.substr(2, std::string::npos));
+      if (url.is_valid()) {
+        *mojo_shebang = shebang;
+        *mojo_content_handler_url = url;
+        return true;
+      }
+    }
+    return false;
+  }
+
+  virtual std::string MimeType() = 0;
 
  protected:
-  void RunLibrary(const base::FilePath& path, bool path_exists) {
-    ScopedMessagePipeHandle shell_handle =
-        load_callbacks_->RegisterApplication();
-    if (!shell_handle.is_valid()) {
-      LoaderComplete();
-      return;
+  virtual bool HasMojoMagic() = 0;
+  virtual bool PeekFirstLine(std::string* line) = 0;
+};
+
+// A request to load a remote URL.
+class NetworkLoaderRequest : public LoaderRequest {
+ public:
+  NetworkLoaderRequest(const GURL& url,
+                       NetworkService* network_service,
+                       const base::Callback<void(bool)>& completion_callback)
+      : completion_callback_(completion_callback), weak_ptr_factory_(this) {
+    Load(url, network_service);
+  }
+
+  ~NetworkLoaderRequest() override {
+    if (!path_.empty())
+      base::DeleteFile(path_, false);
+  }
+
+  URLResponsePtr AsURLResponse(base::TaskRunner* task_runner,
+                               uint32_t skip) override {
+    if (skip != 0) {
+      MojoResult result = ReadDataRaw(
+          response_->body.get(), nullptr, &skip,
+          MOJO_READ_DATA_FLAG_ALL_OR_NONE | MOJO_READ_DATA_FLAG_DISCARD);
+      DCHECK_EQ(result, MOJO_RESULT_OK);
     }
+    return response_.Pass();
+  }
 
-    if (!path_exists) {
-      LOG(ERROR) << "Library not started because library path '" << path.value()
-                 << "' does not exist.";
-      LoaderComplete();
+  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;
     }
+    base::CreateTemporaryFile(&path_);
+    common::CopyToFile(response_->body.Pass(), path_, task_runner,
+                       base::Bind(callback, path_));
+  }
 
-    runner_ = runner_factory_->Create(context_);
-    runner_->Start(
-        path,
-        shell_handle.Pass(),
-        base::Bind(&Loader::LoaderComplete, weak_ptr_factory_.GetWeakPtr()));
+  std::string MimeType() override {
+    DCHECK(response_);
+    return response_->mime_type;
   }
 
-  void LoaderComplete() { loader_complete_callback_.Run(this); }
+ private:
+  // TODO(hansmuller): Revisit this when a real peek operation is available.
+  static const MojoDeadline kPeekTimeout = MOJO_DEADLINE_INDEFINITE;
 
-  scoped_refptr<ApplicationLoader::LoadCallbacks> load_callbacks_;
-  LoaderCompleteCallback loader_complete_callback_;
-  Context* context_;
+  bool HasMojoMagic() override {
+    std::string magic;
+    return BlockingPeekNBytes(response_->body.get(), &magic, strlen(kMojoMagic),
+                              kPeekTimeout) &&
+           magic == kMojoMagic;
+  }
 
- private:
-  DynamicServiceRunnerFactory* runner_factory_;
-  scoped_ptr<DynamicServiceRunner> runner_;
-  base::WeakPtrFactory<Loader> weak_ptr_factory_;
+  bool PeekFirstLine(std::string* line) override {
+    return BlockingPeekLine(response_->body.get(), line, kMaxShebangLength,
+                            kPeekTimeout);
+  }
+
+  void Load(const GURL& url, NetworkService* network_service) {
+    URLRequestPtr request(URLRequest::New());
+    request->url = String::From(url);
+    request->auto_follow_redirects = true;
+
+    if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kDisableCache)) {
+      request->bypass_cache = true;
+    }
+
+    network_service->CreateURLLoader(GetProxy(&url_loader_));
+    url_loader_->Start(request.Pass(),
+                       base::Bind(&NetworkLoaderRequest::OnLoadComplete,
+                                  weak_ptr_factory_.GetWeakPtr()));
+  }
+
+  void OnLoadComplete(URLResponsePtr response) {
+    if (response->error) {
+      LOG(ERROR) << "Error (" << response->error->code << ": "
+                 << response->error->description << ") while fetching "
+                 << response->url;
+      completion_callback_.Run(false);
+      return;
+    }
+    response_ = response.Pass();
+    completion_callback_.Run(true);
+  }
+
+  base::Callback<void(bool)> completion_callback_;
+  URLLoaderPtr url_loader_;
+  URLResponsePtr response_;
+  base::FilePath path_;
+  base::WeakPtrFactory<NetworkLoaderRequest> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkLoaderRequest);
 };
 
-// A loader for local files.
-class DynamicApplicationLoader::LocalLoader : public Loader {
+// A request to load a file URL.
+class LocalLoaderRequest : public LoaderRequest {
  public:
-  LocalLoader(const GURL& url,
-              Context* context,
-              DynamicServiceRunnerFactory* runner_factory,
-              scoped_refptr<ApplicationLoader::LoadCallbacks> load_callbacks,
-              const LoaderCompleteCallback& loader_complete_callback)
-      : Loader(context,
-               runner_factory,
-               load_callbacks,
-               loader_complete_callback),
-        weak_ptr_factory_(this) {
+  LocalLoaderRequest(const GURL& url,
+                     const base::Callback<void(bool)>& completion_callback)
+      : url_(url), path_(UrlToFile(url)) {
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE, base::Bind(completion_callback, true));
+  }
+
+  URLResponsePtr AsURLResponse(base::TaskRunner* task_runner,
+                               uint32_t skip) override {
+    URLResponsePtr response(URLResponse::New());
+    response->url = String::From(url_);
+    DataPipe data_pipe;
+    response->body = data_pipe.consumer_handle.Pass();
+    int64 file_size;
+    if (base::GetFileSize(path_, &file_size)) {
+      response->headers = Array<String>(1);
+      response->headers[0] =
+          base::StringPrintf("Content-Length: %" PRId64, file_size);
+    }
+    common::CopyFromFile(path_, data_pipe.producer_handle.Pass(), skip,
+                         task_runner, base::Bind(&IgnoreResult));
+    return response.Pass();
+  }
+
+  void AsPath(
+      base::TaskRunner* task_runner,
+      base::Callback<void(const base::FilePath&, bool)> callback) override {
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE, base::Bind(callback, path_, base::PathExists(path_)));
+  }
+
+  std::string MimeType() override { return ""; }
+
+ private:
+  static base::FilePath UrlToFile(const GURL& url) {
     DCHECK(url.SchemeIsFile());
     url::RawCanonOutputW<1024> output;
     url::DecodeURLEscapeSequences(
@@ -108,88 +217,89 @@ class DynamicApplicationLoader::LocalLoader : public Loader {
 #else
     base::FilePath path(base::UTF16ToUTF8(decoded_path));
 #endif
+    return path;
+  }
 
-    // Async for consistency with network case.
-    base::MessageLoop::current()->PostTask(
-        FROM_HERE,
-        base::Bind(&LocalLoader::RunLibrary,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   path,
-                   base::PathExists(path)));
+  bool HasMojoMagic() override {
+    std::string magic;
+    ReadFileToString(path_, &magic, strlen(kMojoMagic));
+    return magic == kMojoMagic;
   }
 
-  ~LocalLoader() override {}
+  bool PeekFirstLine(std::string* line) override {
+    std::string start_of_file;
+    ReadFileToString(path_, &start_of_file, kMaxShebangLength);
+    size_t return_position = start_of_file.find('\n');
+    if (return_position == std::string::npos)
+      return false;
+    *line = start_of_file.substr(0, return_position + 1);
+    return true;
+  }
 
- private:
-  base::WeakPtrFactory<LocalLoader> weak_ptr_factory_;
+  GURL url_;
+  base::FilePath path_;
+
+  DISALLOW_COPY_AND_ASSIGN(LocalLoaderRequest);
 };
 
-// A loader for network files.
-class DynamicApplicationLoader::NetworkLoader : public Loader {
+}  // namespace
+
+// Encapsulates loading and running one individual application.
+//
+// Loaders are owned by DynamicApplicationLoader. DynamicApplicationLoader must
+// ensure that all the parameters passed to Loader subclasses stay valid through
+// Loader's lifetime.
+//
+// Async operations are done with WeakPtr to protect against
+// DynamicApplicationLoader going away (and taking all the Loaders with it)
+// while the async operation is outstanding.
+class DynamicApplicationLoader::Loader {
  public:
-  NetworkLoader(const GURL& url,
-                MimeTypeToURLMap* mime_type_to_url,
-                Context* context,
-                DynamicServiceRunnerFactory* runner_factory,
-                NetworkService* network_service,
-                scoped_refptr<ApplicationLoader::LoadCallbacks> load_callbacks,
-                const LoaderCompleteCallback& loader_complete_callback)
-      : Loader(context,
-               runner_factory,
-               load_callbacks,
-               loader_complete_callback),
+  Loader(const GURL& url,
+         MimeTypeToURLMap* mime_type_to_url,
+         Context* context,
+         DynamicServiceRunnerFactory* runner_factory,
+         NetworkServicePtr* network_service,
+         scoped_refptr<ApplicationLoader::LoadCallbacks> load_callbacks,
+         const LoaderCompleteCallback& loader_complete_callback)
+      : load_callbacks_(load_callbacks),
+        loader_complete_callback_(loader_complete_callback),
+        context_(context),
         mime_type_to_url_(mime_type_to_url),
+        runner_factory_(runner_factory),
+        network_service_(network_service),
         weak_ptr_factory_(this) {
-    URLRequestPtr request(URLRequest::New());
-    request->url = String::From(url);
-    request->auto_follow_redirects = true;
-
-    if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-            switches::kDisableCache)) {
-      request->bypass_cache = true;
-    }
-
-    network_service->CreateURLLoader(GetProxy(&url_loader_));
-    url_loader_->Start(request.Pass(),
-                       base::Bind(&NetworkLoader::OnLoadComplete,
-                                  weak_ptr_factory_.GetWeakPtr()));
+    Load(url, network_service);
   }
 
-  ~NetworkLoader() override {
-    if (!file_.empty())
-      base::DeleteFile(file_, false);
-  }
+  virtual ~Loader() {}
 
  private:
-  bool PeekContentHandler(DataPipeConsumerHandle source,
-                          std::string* mojo_shebang,
-                          GURL* mojo_content_handler_url)
-  {
-    const char* kMojoMagic = "#!mojo:";
-    // TODO(hansmuller): Revisit this when a real peek operation is available.
-    const MojoDeadline kPeekTimeout = MOJO_DEADLINE_INDEFINITE;
-    const size_t kMaxShebangLength = 2048;
+  void RunLibrary(const base::FilePath& path, bool path_exists) {
+    ScopedMessagePipeHandle shell_handle =
+        load_callbacks_->RegisterApplication();
+    if (!shell_handle.is_valid()) {
+      LoaderComplete();
+      return;
+    }
 
-    std::string magic;
-    std::string shebang;
-    if (BlockingPeekNBytes(source, &magic, strlen(kMojoMagic), kPeekTimeout) &&
-        magic == kMojoMagic &&
-        BlockingPeekLine(source, &shebang, kMaxShebangLength, kPeekTimeout)) {
-      GURL url(shebang.substr(2, std::string::npos));
-      if (url.is_valid()) {
-        *mojo_shebang = shebang;
-        *mojo_content_handler_url = url;
-        return true;
-      }
+    if (!path_exists) {
+      LOG(ERROR) << "Library not started because library path '" << path.value()
+                 << "' does not exist.";
+      LoaderComplete();
+      return;
     }
-    return false;
+
+    runner_ = runner_factory_->Create(context_);
+    runner_->Start(
+        path, shell_handle.Pass(),
+        base::Bind(&Loader::LoaderComplete, weak_ptr_factory_.GetWeakPtr()));
   }
 
-  void OnLoadComplete(URLResponsePtr response) {
-    if (response->error) {
-      LOG(ERROR) << "Error (" << response->error->code << ": "
-                 << response->error->description << ") while fetching "
-                 << response->url;
+  void LoaderComplete() { loader_complete_callback_.Run(this); }
+
+  void OnResponse(bool success) {
+    if (!success) {
       LoaderComplete();
       return;
     }
@@ -198,24 +308,21 @@ class DynamicApplicationLoader::NetworkLoader : public Loader {
     {
       GURL url;
       std::string shebang;
-      if (PeekContentHandler(response->body.get(), &shebang, &url)) {
-        uint32_t num_skip_bytes = shebang.size();
-        if (ReadDataRaw(response->body.get(),
-                        nullptr,
-                        &num_skip_bytes,
-                        MOJO_READ_DATA_FLAG_ALL_OR_NONE |
-                        MOJO_READ_DATA_FLAG_DISCARD) ==
-            MOJO_RESULT_OK) {
-          load_callbacks_->LoadWithContentHandler(url, response.Pass());
-          return;
-        }
+      if (request_->PeekContentHandler(&shebang, &url)) {
+        load_callbacks_->LoadWithContentHandler(
+            url,
+            request_->AsURLResponse(context_->task_runners()->blocking_pool(),
+                                    shebang.size()));
+        return;
       }
     }
 
     MimeTypeToURLMap::iterator iter =
-        mime_type_to_url_->find(response->mime_type);
+        mime_type_to_url_->find(request_->MimeType());
     if (iter != mime_type_to_url_->end()) {
-      load_callbacks_->LoadWithContentHandler(iter->second, response.Pass());
+      load_callbacks_->LoadWithContentHandler(
+          iter->second, request_->AsURLResponse(
+                            context_->task_runners()->blocking_pool(), 0));
       return;
     }
 
@@ -224,20 +331,49 @@ class DynamicApplicationLoader::NetworkLoader : public Loader {
     // header, or looking for some specific mojo signature prepended to the
     // library.
 
-    base::CreateTemporaryFile(&file_);
-    common::CopyToFile(
-        response->body.Pass(),
-        file_,
+    request_->AsPath(
         context_->task_runners()->blocking_pool(),
-        base::Bind(
-            &NetworkLoader::RunLibrary, weak_ptr_factory_.GetWeakPtr(), file_));
+        base::Bind(&Loader::RunLibrary, weak_ptr_factory_.GetWeakPtr()));
   }
 
+  void Load(const GURL& url, NetworkServicePtr* network_service) {

[Aaron Boodman, 2014/11/07 08:56:27]

second param never used.

+    GURL resolved_url;
+    if (url.SchemeIs("mojo")) {
+      resolved_url = context_->mojo_url_resolver()->Resolve(url);
+    } else {
+      resolved_url = url;
+    }
+
+    if (resolved_url.SchemeIsFile()) {
+      request_.reset(new LocalLoaderRequest(
+          resolved_url,
+          base::Bind(&DynamicApplicationLoader::Loader::OnResponse,
+                     weak_ptr_factory_.GetWeakPtr())));
+      return;
+    }
+
+    if (!network_service_->get()) {

[Aaron Boodman, 2014/11/07 08:56:27]

Put this back in DAL::Load() so that we share one instance across all loaders.

+      context_->application_manager()->ConnectToService(
+          GURL("mojo:network_service"), network_service_);

[Aaron Boodman, 2014/11/07 08:56:27]

network_service_ is a pointer, so I guess this pipe will never be closed.

[qsr, 2014/11/07 09:33:22]

I mixed 2 things here, sorry. network_service_ should not exist, and we should
use the network_service provided by the  DynamicApplicationLoader::Load

+    }
+
+    request_.reset(new NetworkLoaderRequest(
+        resolved_url, network_service_->get(),
+        base::Bind(&DynamicApplicationLoader::Loader::OnResponse,
+                   weak_ptr_factory_.GetWeakPtr())));
+  }
+
+  scoped_refptr<ApplicationLoader::LoadCallbacks> load_callbacks_;
+  LoaderCompleteCallback loader_complete_callback_;
+  Context* context_;
   MimeTypeToURLMap* mime_type_to_url_;
-  URLLoaderPtr url_loader_;
-  base::FilePath file_;
-  base::WeakPtrFactory<NetworkLoader> weak_ptr_factory_;
+  DynamicServiceRunnerFactory* runner_factory_;
+  NetworkServicePtr* network_service_;
+  scoped_ptr<DynamicServiceRunner> runner_;
+  scoped_ptr<LoaderRequest> request_;
+  base::WeakPtrFactory<Loader> weak_ptr_factory_;
 };
+
 DynamicApplicationLoader::DynamicApplicationLoader(
     Context* context,
     scoped_ptr<DynamicServiceRunnerFactory> runner_factory)
@@ -264,34 +400,9 @@ void DynamicApplicationLoader::Load(
     ApplicationManager* manager,
     const GURL& url,
     scoped_refptr<LoadCallbacks> load_callbacks) {
-  GURL resolved_url;
-  if (url.SchemeIs("mojo")) {
-    resolved_url = context_->mojo_url_resolver()->Resolve(url);
-  } else {
-    resolved_url = url;
-  }
-
-  if (resolved_url.SchemeIsFile()) {
-    loaders_.push_back(new LocalLoader(resolved_url,
-                                       context_,
-                                       runner_factory_.get(),
-                                       load_callbacks,
-                                       loader_complete_callback_));
-    return;
-  }
-
-  if (!network_service_) {
-    context_->application_manager()->ConnectToService(
-        GURL("mojo:network_service"), &network_service_);
-  }
-
-  loaders_.push_back(new NetworkLoader(resolved_url,
-                                       &mime_type_to_url_,
-                                       context_,
-                                       runner_factory_.get(),
-                                       network_service_.get(),
-                                       load_callbacks,
-                                       loader_complete_callback_));
+  loaders_.push_back(new Loader(url, &mime_type_to_url_, context_,
+                                runner_factory_.get(), &network_service_,

[Aaron Boodman, 2014/11/07 08:56:27]

this is always null.

+                                load_callbacks, loader_complete_callback_));
 }
 
 void DynamicApplicationLoader::OnApplicationError(ApplicationManager* manager,