Simple multi-url support for mojo apps

shell/shell_apptest.cc

+// Copyright 2015 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 "base/base_paths.h"
+#include "base/bind.h"
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/path_service.h"
+#include "base/run_loop.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
+#include "mojo/common/data_pipe_utils.h"
+#include "mojo/public/cpp/application/application_impl.h"
+#include "mojo/public/cpp/application/application_test_base.h"
+#include "mojo/public/cpp/system/macros.h"
+#include "mojo/services/network/public/interfaces/net_address.mojom.h"
+#include "services/http_server/public/http_server.mojom.h"
+#include "services/http_server/public/http_server_factory.mojom.h"
+#include "services/http_server/public/http_server_util.h"
+#include "shell/test/pingable.mojom.h"
+
+namespace mojo {
+
+namespace {
+
+std::string GetURL(uint16_t port, const std::string& path) {
+  return base::StringPrintf("http://127.0.0.1:%u/%s", port, path.c_str());
+}
+}

[qsr, 2015/02/27 17:15:58]

// namespace

+
+class GetHandler : public http_server::HttpHandler {
+ public:
+  GetHandler(InterfaceRequest<http_server::HttpHandler> request, uint16_t port)
+      : binding_(this, request.Pass()), port_(port) {
+    CHECK(PathService::Get(base::FILE_MODULE, &app_path_));
+    app_path_ = app_path_.DirName().Append("pingable_app.mojo");

[qsr, 2015/02/27 17:15:58]

I think you will have issue on android. You might be better connecting to this
using relative URL from where you are yourself, or mojo: url.

Which goes to the next thing, this will probably not work for file URL (or maybe
even for our test server), so as a suggestion should our fetch strategy retry
fetching without the query string if the request failed with. It would allow to
use query string whithout needing any special configuration on the server.

[Aaron Boodman, 2015/02/28 19:08:23]

I'm not sure what that would mean. I'm trying to test that http-served apps
work. That means I need to read the file from somewhere to actually serve it.
It can definitely work with file URLs. The LocalFetcher just needs to strip the
querystring before fetching the file. I added this and a test to exercise it.

Typically server developers have to go out of their way to disallow
querystrings, so I'm not really convinced by the other part of your argument. I
definitely don't think we should magically retry connections behind the scenes
with different URLs though -- the URLs are the contract between an app and its
clients. The app developer should know whether querystrings are supported on his
server :). If a client requests a URL and it doesn't load, we should faithfully
report that back to the client.

[qsr, 2015/03/02 08:51:31]

 Ok.

 For android, the issue is that you cannot use
PathService::Get(base::FILE_MODULE, )) and hope to find something that you build
there, because you are running on the device.

 You should do what we do with icu and bundle the app data in the apptest
executable.

[Aaron Boodman, 2015/03/02 15:36:36]

I will look at this today. However, as it is my last day, and I would like to
land what I have, I have uploaded a change that disables the tests that rely on
the data on Android. Can I get an LGTM on this?

+    CHECK(base::PathExists(app_path_));
+  }
+  ~GetHandler() override {}
+
+ private:
+  // http_server::HttpHandler:
+  void HandleRequest(
+      http_server::HttpRequestPtr request,
+      const Callback<void(http_server::HttpResponsePtr)>& callback) override {
+    http_server::HttpResponsePtr response;
+    if (StartsWithASCII(request->relative_url, "/app", true)) {
+      // Super inefficient, but meh.
+      std::string data;
+      base::ReadFileToString(app_path_, &data);
+      response = http_server::CreateHttpResponse(200, data);
+      response->content_type = "application/octet-stream";
+    } else if (request->relative_url == "/redirect") {
+      response = http_server::HttpResponse::New();
+      response->status_code = 302;
+      response->custom_headers.insert("Location", GetURL(port_, "app"));
+    } else {
+      NOTREACHED();
+    }
+
+    callback.Run(response.Pass());
+  }
+
+  Binding<http_server::HttpHandler> binding_;
+  base::FilePath app_path_;
+  uint16_t port_;
+
+  MOJO_DISALLOW_COPY_AND_ASSIGN(GetHandler);
+};
+
+class ShellAppTest : public test::ApplicationTestBase {
+ public:
+  ShellAppTest() : ApplicationTestBase() {}
+  ~ShellAppTest() override {}
+
+ protected:
+  // ApplicationTestBase:
+  void SetUp() override {
+    ApplicationTestBase::SetUp();
+
+    application_impl()->ConnectToService("mojo:http_server",
+                                         &http_server_factory_);
+
+    mojo::NetAddressPtr local_address(mojo::NetAddress::New());
+    local_address->family = mojo::NET_ADDRESS_FAMILY_IPV4;
+    local_address->ipv4 = mojo::NetAddressIPv4::New();
+    local_address->ipv4->addr.resize(4);
+    local_address->ipv4->addr[0] = 127;
+    local_address->ipv4->addr[1] = 0;
+    local_address->ipv4->addr[2] = 0;
+    local_address->ipv4->addr[3] = 1;
+    local_address->ipv4->port = 0;
+    http_server_factory_->CreateHttpServer(GetProxy(&http_server_),
+                                           local_address.Pass());
+
+    http_server_->GetPort([this](uint16_t p) { port_ = p; });
+    EXPECT_TRUE(http_server_.WaitForIncomingMethodCall());
+
+    InterfacePtr<http_server::HttpHandler> http_handler;
+    handler_.reset(new GetHandler(GetProxy(&http_handler).Pass(), port_));
+    http_server_->SetHandler(".*", http_handler.Pass(),
+                             [](bool result) { EXPECT_TRUE(result); });
+    EXPECT_TRUE(http_server_.WaitForIncomingMethodCall());
+  }
+
+  std::string GetURL(const std::string& path) {
+    return ::mojo::GetURL(port_, path);
+  }
+
+  http_server::HttpServerFactoryPtr http_server_factory_;
+  http_server::HttpServerPtr http_server_;
+  scoped_ptr<GetHandler> handler_;
+  uint16_t port_;
+
+ private:
+  MOJO_DISALLOW_COPY_AND_ASSIGN(ShellAppTest);
+};
+
+// Test that we can load apps over http.
+TEST_F(ShellAppTest, Http) {
+  InterfacePtr<Pingable> pingable;
+  application_impl()->ConnectToService(GetURL("app"), &pingable);
+  pingable->Ping("hello", [](const String& app_url, const String& message) {
+    EXPECT_EQ("hello", message);
+    base::MessageLoop::current()->Quit();
+  });
+  base::RunLoop().Run();
+}
+
+// Test that redirects work.
+// TODO(aa): Test that apps receive the correct URL parameters.
+TEST_F(ShellAppTest, Redirect) {
+  InterfacePtr<Pingable> pingable;
+  application_impl()->ConnectToService(GetURL("redirect"), &pingable);
+  pingable->Ping("hello", [](const String& app_url, const String& message) {
+    EXPECT_EQ("hello", message);
+    base::MessageLoop::current()->Quit();
+  });
+  base::RunLoop().Run();
+}
+
+// Test that querystring is not considered when resolving http applications.
+TEST_F(ShellAppTest, QueryHandling) {
+  InterfacePtr<Pingable> pingable1;
+  InterfacePtr<Pingable> pingable2;
+  application_impl()->ConnectToService(GetURL("app?foo"), &pingable1);
+  application_impl()->ConnectToService(GetURL("app?bar"), &pingable2);
+
+  int num_responses = 0;
+  auto callback =
+      [this, &num_responses](const String& app_url, const String& message) {
+        EXPECT_EQ(GetURL("app"), app_url);
+        EXPECT_EQ("hello", message);
+        if (++num_responses == 2) {
+          base::MessageLoop::current()->Quit();
+        }
+      };
+  pingable1->Ping("hello", callback);
+  pingable2->Ping("hello", callback);
+  base::RunLoop().Run();
+}
+
+}  // namespace mojo