Emit an error log line when attempting to connect to an app at an invalid URL.

mojo/application_manager/application_manager.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 "mojo/application_manager/application_manager.h"
 
 #include <stdio.h>
 
 #include "base/bind.h"
 #include "base/lazy_instance.h"
@@ skipping 82 matching lines @@
     client()->AcceptConnection(String::From(requestor_url),
                                service_provider.Pass());
   }
 
   // ServiceProvider implementation:
   void ConnectToApplication(
       const String& app_url,
       InterfaceRequest<ServiceProvider> in_service_provider) override {
     ServiceProviderPtr out_service_provider;
     out_service_provider.Bind(in_service_provider.PassMessagePipe());
-    manager_->ConnectToApplication(
-        app_url.To<GURL>(), url_, out_service_provider.Pass());
+    GURL app_gurl(app_url);
+    if (!app_gurl.is_valid() && app_gurl.possibly_invalid_spec().size() == 0) {

[Aaron Boodman, 2014/11/20 21:53:59]

This is the first code that gets called when some other application calls
ApplicationManager::ConnectToApplication().

Since anyone could pass anything to that method, we don't want to consider this
a DCHECK-level error. We should validate it, print a nice error, but otherwise
continue gracefully.

When I brought up the DCHECK, I meant it should go in
ApplicationManager::ConnectToApplication(), not here. Because at that point,
higher layers should have already validated the GURL.

[azani, 2014/11/20 22:10:03]

Done.

+      LOG(ERROR) << "Error: invalid URL: " << app_url;

[Aaron Boodman, 2014/11/20 21:53:59]

return here. The InterfaceRequest will get destroyed and the caller will get an
error. This is the same thing that ApplicationManager is going to do later, but
if we get that far, AM is going to assert, which we don't want.

[azani, 2014/11/20 22:10:03]

Done.

+    }
+    DCHECK(app_gurl.possibly_invalid_spec().size() > 0);

[Aaron Boodman, 2014/11/20 21:53:59]

This moves to ApplicationManager::ConnectToApplication().

[azani, 2014/11/20 22:10:03]

Done.

+    manager_->ConnectToApplication(app_gurl, url_, out_service_provider.Pass());
   }
 
   const GURL& url() const { return url_; }
 
  private:
   void OnConnectionError() override { manager_->OnShellImplError(this); }
 
   ApplicationManager* const manager_;
   const GURL url_;
 
@@ skipping 201 matching lines @@
   StubServiceProvider* stub_sp = new StubServiceProvider;
   ServiceProviderPtr spp;
   BindToProxy(stub_sp, &spp);
   ConnectToApplication(application_url, GURL(), spp.Pass());
   MessagePipe pipe;
   stub_sp->GetRemoteServiceProvider()->ConnectToService(interface_name,
                                                         pipe.handle1.Pass());
   return pipe.handle0.Pass();
 }
 }  // namespace mojo