Mojo content handler shebang support

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 71dcada0d129f2125307e42199d59ae6da248cc3..231eb56fe18d654002e1cc77c2f4294b8759877f 100644
--- a/mojo/shell/dynamic_application_loader.cc
+++ b/mojo/shell/dynamic_application_loader.cc
@@ -16,6 +16,7 @@
 #include "mojo/common/data_pipe_utils.h"
 #include "mojo/services/public/interfaces/network/url_loader.mojom.h"
 #include "mojo/shell/context.h"
+#include "mojo/shell/data_pipe_peek.h"
 #include "mojo/shell/filename_util.h"
 #include "mojo/shell/switches.h"
 #include "url/url_util.h"
@@ -158,6 +159,29 @@ class DynamicApplicationLoader::NetworkLoader : public Loader {
   }
 
  private:
+  bool PeekContentHandler(DataPipeConsumerHandle source,
+                          std::string* mojo_shebang,
+                          GURL* mojo_content_handler_url)
+  {
+    const std::string kMojoMagic("#!mojo:");

[Aaron Boodman, 2014/11/03 23:50:36]

No real reason to make this a string, const char* works.

[hansmuller, 2014/11/04 00:26:03]

Done.

+    const MojoDeadline kPeekTimeout = 1000 * 5; // 5ms

[Aaron Boodman, 2014/11/03 23:50:36]

urgh. Can you put a TODO on this to fix once peeking is supported properly?

[hansmuller, 2014/11/04 00:26:03]

Done. I've defeated the timeout for now, it's MOJO_DEADLINE_INDEFINITE.

+    const size_t kMaxShebangLength = 2048;
+
+    std::string magic;
+    std::string shebang;
+    if (BlockingPeekNBytes(source, &magic, kMojoMagic.size(), 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;
+      }
+    }
+    return false;
+  }
+
   void OnLoadComplete(URLResponsePtr response) {

[Aaron Boodman, 2014/11/03 23:50:36]

Note that this is about to move around some:

https://codereview.chromium.org/694303002/

[hansmuller, 2014/11/04 00:26:03]

It looks like the shebang code will merge with qsr's CL without too much
trouble.

     if (response->error) {
       LOG(ERROR) << "Error (" << response->error->code << ": "
@@ -167,6 +191,25 @@ class DynamicApplicationLoader::NetworkLoader : public Loader {
       return;
     }
 
+    // If the response begins with a #!mojo:<content-handler-url>, use it.
+    {
+      GURL url;
+      std::string shebang;
+      if (PeekContentHandler(response->body.get(), &shebang, &url)) {
+        // TBD(hansmuller): content-handlers should skip the shebang themselves.

[Aaron Boodman, 2014/11/03 23:50:36]

I'm not sure I agree with this statement.

How about we just assume it is the shell's responsibility for now, and change it
if/when necessary. Unless you have some specific reason you think apps should do
it?

[hansmuller, 2014/11/04 00:26:03]

I thought that content-handlers would check the shebang as sanity check. The
shebang is optional so that probably doesn't make sense. I will remove the
comment.

+        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;
+        }
+      }
+    }
+
     MimeTypeToURLMap::iterator iter =
         mime_type_to_url_->find(response->mime_type);
     if (iter != mime_type_to_url_->end()) {
@@ -174,11 +217,11 @@ class DynamicApplicationLoader::NetworkLoader : public Loader {
       return;
     }
 
-    LOG(INFO) << "Failed to find content handler for " << response->url
-              << " (mimetype: " << response->url << ")" << std::endl
-              << "Attempting to load as native library instead...";
+    LOG(WARNING) << "Failed to find content handler for " << response->url

[Aaron Boodman, 2014/11/03 23:50:36]

Let's just remove this. I don't think it warrants a warning, and the presubmit
script will keep whining if we leave it as INFO.

[hansmuller, 2014/11/04 00:26:03]

Done.

+                 << " (mimetype: " << response->url << ")" << std::endl
+                 << "Attempting to load as native library instead...";
 
-    // TODO(aa): Santify check that the thing we got looks vaguely like a mojo
+    // 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.
@@ -197,7 +240,6 @@ class DynamicApplicationLoader::NetworkLoader : public Loader {
   base::FilePath file_;
   base::WeakPtrFactory<NetworkLoader> weak_ptr_factory_;
 };
-
 DynamicApplicationLoader::DynamicApplicationLoader(
     Context* context,
     scoped_ptr<DynamicServiceRunnerFactory> runner_factory)