Makes the identity of apps with the mojo scheme remain mojo

Before this change the identity becomes the final location, eg
file:///build/out/Debug/foo.mojo . This change makes it so that the
identity is the url requested, eg mojo:foo. The identity does not take
into consideration mappings. For example, if mojo:foo maps to
mojo:foo2 the identity is mojo:foo.

R=ben@chromium.org

Review URL: https://codereview.chromium.org/1112253002

Cr-Commit-Position: refs/heads/master@{#327608}

3 files changed, 41 insertions, 25 deletions

diff --git a/mojo/shell/application_manager.cc b/mojo/shell/application_manager.cc
index 4648c1f..3f51008 100644
--- a/mojo/shell/application_manager.cc
+++ b/mojo/shell/application_manager.cc
@@ -132,29 +132,30 @@ void ApplicationManager::ConnectToApplicationWithParameters(
   }
 
   // The application is not running, let's compute the parameters.
-  if (ConnectToApplicationWithLoader(mapped_url, requestor_url, &services,
-                                     &exposed_services, on_application_end,
-                                     pre_redirect_parameters,
-                                     GetLoaderForURL(mapped_url))) {
+  if (ConnectToApplicationWithLoader(
+          requested_url, mapped_url, requestor_url, &services,
+          &exposed_services, on_application_end, pre_redirect_parameters,
+          GetLoaderForURL(mapped_url))) {
     return;
   }
 
-  if (ConnectToApplicationWithLoader(resolved_url, requestor_url, &services,
-                                     &exposed_services, on_application_end,
-                                     pre_redirect_parameters,
-                                     GetLoaderForURL(resolved_url))) {
+  if (ConnectToApplicationWithLoader(
+          requested_url, resolved_url, requestor_url, &services,
+          &exposed_services, on_application_end, pre_redirect_parameters,
+          GetLoaderForURL(resolved_url))) {
     return;
   }
 
   if (ConnectToApplicationWithLoader(
-          resolved_url, requestor_url, &services, &exposed_services,
-          on_application_end, pre_redirect_parameters, default_loader_.get())) {
+          requested_url, resolved_url, requestor_url, &services,
+          &exposed_services, on_application_end, pre_redirect_parameters,
+          default_loader_.get())) {
     return;
   }
 
   auto callback = base::Bind(&ApplicationManager::HandleFetchCallback,
-                             weak_ptr_factory_.GetWeakPtr(), requestor_url,
-                             base::Passed(services.Pass()),
+                             weak_ptr_factory_.GetWeakPtr(), requested_url,
+                             requestor_url, base::Passed(services.Pass()),
                              base::Passed(exposed_services.Pass()),
                              on_application_end, pre_redirect_parameters);
 
@@ -194,6 +195,7 @@ bool ApplicationManager::ConnectToRunningApplication(
 }
 
 bool ApplicationManager::ConnectToApplicationWithLoader(
+    const GURL& requested_url,
     const GURL& resolved_url,
     const GURL& requestor_url,
     InterfaceRequest<ServiceProvider>* services,
@@ -204,21 +206,24 @@ bool ApplicationManager::ConnectToApplicationWithLoader(
   if (!loader)
     return false;
 
+  const GURL app_url =
+      requested_url.scheme() == "mojo" ? requested_url : resolved_url;
+
   loader->Load(
       resolved_url,
-      RegisterShell(resolved_url, requestor_url, services->Pass(),
+      RegisterShell(app_url, requestor_url, services->Pass(),
                     exposed_services->Pass(), on_application_end, parameters));
   return true;
 }
 
 InterfaceRequest<Application> ApplicationManager::RegisterShell(
-    const GURL& resolved_url,
+    const GURL& app_url,
     const GURL& requestor_url,
     InterfaceRequest<ServiceProvider> services,
     ServiceProviderPtr exposed_services,
     const base::Closure& on_application_end,
     const std::vector<std::string>& parameters) {
-  Identity app_identity(resolved_url);
+  Identity app_identity(app_url);
 
   ApplicationPtr application;
   InterfaceRequest<Application> application_request = GetProxy(&application);
@@ -226,7 +231,7 @@ InterfaceRequest<Application> ApplicationManager::RegisterShell(
       new ShellImpl(application.Pass(), this, app_identity, on_application_end);
   identity_to_shell_impl_[app_identity] = shell;
   shell->InitializeApplication(Array<String>::From(parameters));
-  ConnectToClient(shell, resolved_url, requestor_url, services.Pass(),
+  ConnectToClient(shell, app_url, requestor_url, services.Pass(),
                   exposed_services.Pass());
   return application_request.Pass();
 }
@@ -249,6 +254,7 @@ void ApplicationManager::ConnectToClient(
 }
 
 void ApplicationManager::HandleFetchCallback(
+    const GURL& requested_url,
     const GURL& requestor_url,
     InterfaceRequest<ServiceProvider> services,
     ServiceProviderPtr exposed_services,
@@ -264,6 +270,7 @@ void ApplicationManager::HandleFetchCallback(
   GURL redirect_url = fetcher->GetRedirectURL();
   if (!redirect_url.is_empty()) {
     // And around we go again... Whee!
+    // TODO(sky): this loses |requested_url|.
     ConnectToApplicationWithParameters(redirect_url, requestor_url,
                                        services.Pass(), exposed_services.Pass(),
                                        on_application_end, parameters);
@@ -276,13 +283,16 @@ void ApplicationManager::HandleFetchCallback(
   //
   // Also, it's possible the original URL was redirected to an app that is
   // already running.
-  if (ConnectToRunningApplication(fetcher->GetURL(), requestor_url, &services,
+  if (ConnectToRunningApplication(requested_url, requestor_url, &services,
                                   &exposed_services)) {
     return;
   }
 
+  const GURL app_url =
+      requested_url.scheme() == "mojo" ? requested_url : fetcher->GetURL();
+
   InterfaceRequest<Application> request(
-      RegisterShell(fetcher->GetURL(), requestor_url, services.Pass(),
+      RegisterShell(app_url, requestor_url, services.Pass(),
                     exposed_services.Pass(), on_application_end, parameters));
 
   // If the response begins with a #!mojo <content-handler-url>, use it.
diff --git a/mojo/shell/application_manager.h b/mojo/shell/application_manager.h
index f33d85b..ba1454c 100644
--- a/mojo/shell/application_manager.h
+++ b/mojo/shell/application_manager.h
@@ -149,6 +149,7 @@ class ApplicationManager {
                                    ServiceProviderPtr* exposed_services);
 
   bool ConnectToApplicationWithLoader(
+      const GURL& requested_url,
       const GURL& resolved_url,
       const GURL& requestor_url,
       InterfaceRequest<ServiceProvider>* services,
@@ -158,8 +159,7 @@ class ApplicationManager {
       ApplicationLoader* loader);
 
   InterfaceRequest<Application> RegisterShell(
-      // The URL after resolution and redirects, including the querystring.
-      const GURL& resolved_url,
+      const GURL& app_url,
       const GURL& requestor_url,
       InterfaceRequest<ServiceProvider> services,
       ServiceProviderPtr exposed_services,
@@ -174,7 +174,10 @@ class ApplicationManager {
                        InterfaceRequest<ServiceProvider> services,
                        ServiceProviderPtr exposed_services);
 
-  void HandleFetchCallback(const GURL& requestor_url,
+  // Called once |fetcher| has found app. |requested_url| is the url of the
+  // requested application before any mappings/resolution have been applied.
+  void HandleFetchCallback(const GURL& requested_url,
+                           const GURL& requestor_url,
                            InterfaceRequest<ServiceProvider> services,
                            ServiceProviderPtr exposed_services,
                            const base::Closure& on_application_end,
diff --git a/mojo/shell/application_manager_unittest.cc b/mojo/shell/application_manager_unittest.cc
index 7819e37..ef77f2f 100644
--- a/mojo/shell/application_manager_unittest.cc
+++ b/mojo/shell/application_manager_unittest.cc
@@ -446,9 +446,9 @@ class ApplicationManagerTest : public testing::Test {
         make_scoped_ptr(new Tester(&tester_context_, requestor_url)), url);
   }
 
-  bool HasFactoryForTestURL() {
+  bool HasFactoryForURL(const GURL& url) {
     ApplicationManager::TestAPI manager_test_api(application_manager_.get());
-    return manager_test_api.HasFactoryForURL(GURL(kTestURLString));
+    return manager_test_api.HasFactoryForURL(url);
   }
 
  protected:
@@ -514,13 +514,13 @@ TEST_F(ApplicationManagerTest, URLMapping) {
 
 TEST_F(ApplicationManagerTest, ClientError) {
   test_client_->Test("test");
-  EXPECT_TRUE(HasFactoryForTestURL());
+  EXPECT_TRUE(HasFactoryForURL(GURL(kTestURLString)));
   loop_.Run();
   EXPECT_EQ(1, context_.num_impls);
   test_client_.reset();
   loop_.Run();
   EXPECT_EQ(0, context_.num_impls);
-  EXPECT_TRUE(HasFactoryForTestURL());
+  EXPECT_TRUE(HasFactoryForURL(GURL(kTestURLString)));
 }
 
 TEST_F(ApplicationManagerTest, Deletes) {
@@ -699,6 +699,9 @@ TEST_F(ApplicationManagerTest, MappedURLsShouldWorkWithLoaders) {
   application_manager_->ConnectToService(GURL("mojo:foo2"), &test_service);
   EXPECT_EQ(1, custom_loader->num_loads());
   custom_loader->set_context(nullptr);
+
+  EXPECT_TRUE(HasFactoryForURL(GURL("mojo:foo2")));
+  EXPECT_FALSE(HasFactoryForURL(GURL("mojo:foo")));
 }
 
 TEST_F(ApplicationManagerTest, TestQueryWithLoaders) {