Simplify the ApplicationLoader interface in preparation for changes.
The ApplicationLoader interface has become very complicated because it
provides a way for clients of ApplicationManager to do two different
things in response to a connection request.
1. Bind to an Application implementation.
2. Delegate back to ApplicationManager with details about a
ContentHandler to use instead.
ApplicationLoader was originally designed to do only #1. I extended it
to do #2 in the misguided thought that it would be nice for
ApplicationLoader implementations to be able to provide content for a
ContentHandler as well actual applications.
However, this second use case was never needed and adds lots of
complexity.
I think it will simplify things dramatically to move URL fetching and
ContentHandler handling into application manager directly. If we ever
need to generalize that behavior, we can create a separate interface
from ApplicationLoader for it at that time.
This CL is a first step that is pretty mechanical in nature, but I
think is still an improvement. Suggest reviewing in the following
order:
- application_loader.*
- application_manager.*
- everything else
The next change will break apart DynamicApplicationLoader into pieces
which will be used from within ApplicationManager.
R=davemoore@chromium.org
Review URL: https://codereview.chromium.org/930243006
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 08fd544..00995e8 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -886,7 +886,7 @@
input_api.DEFAULT_BLACK_LIST +
(r"^base[\\\/]logging\.h$",
r"^base[\\\/]logging\.cc$",
- r"^shell[\\\/]dynamic_application_loader\.cc$",
+ r"^shell[\\\/]native_application_loader\.cc$",
r"^sandbox[\\\/]linux[\\\/].*",
r"^tools[\\\/]",
r"^ui[\\\/]aura[\\\/]bench[\\\/]bench_main\.cc$",))
diff --git a/services/window_manager/window_manager_api_unittest.cc b/services/window_manager/window_manager_api_unittest.cc
index a8f8d6a..dbd3da6 100644
--- a/services/window_manager/window_manager_api_unittest.cc
+++ b/services/window_manager/window_manager_api_unittest.cc
@@ -79,10 +79,9 @@
private:
// Overridden from mojo::ApplicationLoader:
- void Load(mojo::ApplicationManager* application_manager,
- const GURL& url,
- mojo::InterfaceRequest<mojo::Application> application_request,
- LoadCallback callback) override {
+ void Load(
+ const GURL& url,
+ mojo::InterfaceRequest<mojo::Application> application_request) override {
ASSERT_TRUE(application_request.is_pending());
scoped_ptr<ApplicationImpl> app(
new ApplicationImpl(this, application_request.Pass()));
diff --git a/shell/BUILD.gn b/shell/BUILD.gn
index 800ad70..7c7ae60 100644
--- a/shell/BUILD.gn
+++ b/shell/BUILD.gn
@@ -150,14 +150,14 @@
"context.h",
"data_pipe_peek.cc",
"data_pipe_peek.h",
- "dynamic_application_loader.cc",
- "dynamic_application_loader.h",
"external_application_listener.h",
"external_application_listener.cc",
"filename_util.cc",
"filename_util.h",
"incoming_connection_listener.cc",
"incoming_connection_listener.h",
+ "native_application_loader.cc",
+ "native_application_loader.h",
"out_of_process_dynamic_service_runner.cc",
"out_of_process_dynamic_service_runner.h",
"url_resolver.cc",
@@ -404,8 +404,8 @@
"child_process_host_unittest.cc",
"command_line_util_unittest.cc",
"data_pipe_peek_unittest.cc",
- "dynamic_application_loader_unittest.cc",
"in_process_dynamic_service_runner_unittest.cc",
+ "native_application_loader_unittest.cc",
"shell_test_base.cc",
"shell_test_base_android.cc",
"shell_test_base.h",
diff --git a/shell/android/android_handler_loader.cc b/shell/android/android_handler_loader.cc
index 1b334f3..abbf8d7 100644
--- a/shell/android/android_handler_loader.cc
+++ b/shell/android/android_handler_loader.cc
@@ -14,10 +14,8 @@
}
void AndroidHandlerLoader::Load(
- ApplicationManager* manager,
const GURL& url,
- InterfaceRequest<Application> application_request,
- LoadCallback callback) {
+ InterfaceRequest<Application> application_request) {
DCHECK(application_request.is_pending());
application_.reset(
new ApplicationImpl(&android_handler_, application_request.Pass()));
diff --git a/shell/android/android_handler_loader.h b/shell/android/android_handler_loader.h
index 6696138..1b209a4 100644
--- a/shell/android/android_handler_loader.h
+++ b/shell/android/android_handler_loader.h
@@ -22,10 +22,8 @@
private:
// ApplicationLoader overrides:
- void Load(ApplicationManager* manager,
- const GURL& url,
- InterfaceRequest<Application> application_request,
- LoadCallback callback) override;
+ void Load(const GURL& url,
+ InterfaceRequest<Application> application_request) override;
void OnApplicationError(ApplicationManager* manager,
const GURL& url) override;
diff --git a/shell/android/background_application_loader.cc b/shell/android/background_application_loader.cc
index 8f3c21b..2389515 100644
--- a/shell/android/background_application_loader.cc
+++ b/shell/android/background_application_loader.cc
@@ -26,10 +26,8 @@
}
void BackgroundApplicationLoader::Load(
- ApplicationManager* manager,
const GURL& url,
- InterfaceRequest<Application> application_request,
- LoadCallback callback) {
+ InterfaceRequest<Application> application_request) {
DCHECK(application_request.is_pending());
if (!thread_) {
// TODO(tim): It'd be nice if we could just have each Load call
@@ -47,7 +45,7 @@
task_runner_->PostTask(
FROM_HERE,
base::Bind(&BackgroundApplicationLoader::LoadOnBackgroundThread,
- base::Unretained(this), manager, url,
+ base::Unretained(this), url,
base::Passed(&application_request)));
}
@@ -74,11 +72,10 @@
}
void BackgroundApplicationLoader::LoadOnBackgroundThread(
- ApplicationManager* manager,
const GURL& url,
InterfaceRequest<Application> application_request) {
DCHECK(task_runner_->RunsTasksOnCurrentThread());
- loader_->Load(manager, url, application_request.Pass(), SimpleLoadCallback());
+ loader_->Load(url, application_request.Pass());
}
void BackgroundApplicationLoader::OnApplicationErrorOnBackgroundThread(
diff --git a/shell/android/background_application_loader.h b/shell/android/background_application_loader.h
index a4c07b8..850b900 100644
--- a/shell/android/background_application_loader.h
+++ b/shell/android/background_application_loader.h
@@ -24,10 +24,8 @@
~BackgroundApplicationLoader() override;
// ApplicationLoader overrides:
- void Load(ApplicationManager* manager,
- const GURL& url,
- InterfaceRequest<Application> application_request,
- LoadCallback callback) override;
+ void Load(const GURL& url,
+ InterfaceRequest<Application> application_request) override;
void OnApplicationError(ApplicationManager* manager,
const GURL& url) override;
@@ -37,10 +35,7 @@
// These functions are exected on the background thread. They call through
// to |background_loader_| to do the actual loading.
- // TODO: having this code take a |manager| is fragile (as ApplicationManager
- // isn't thread safe).
void LoadOnBackgroundThread(
- ApplicationManager* manager,
const GURL& url,
InterfaceRequest<Application> application_request);
void OnApplicationErrorOnBackgroundThread(ApplicationManager* manager,
diff --git a/shell/android/background_application_loader_unittest.cc b/shell/android/background_application_loader_unittest.cc
index 62e9234..5a6360d 100644
--- a/shell/android/background_application_loader_unittest.cc
+++ b/shell/android/background_application_loader_unittest.cc
@@ -17,10 +17,8 @@
~DummyLoader() override {}
// ApplicationLoader overrides:
- void Load(ApplicationManager* manager,
- const GURL& url,
- InterfaceRequest<Application> application_request,
- LoadCallback callback) override {
+ void Load(const GURL& url,
+ InterfaceRequest<Application> application_request) override {
if (simulate_app_quit_)
base::MessageLoop::current()->Quit();
}
@@ -50,8 +48,7 @@
BackgroundApplicationLoader loader(real_loader.Pass(), "test",
base::MessageLoop::TYPE_DEFAULT);
ApplicationPtr application;
- loader.Load(NULL, GURL(), GetProxy(&application),
- ApplicationLoader::SimpleLoadCallback());
+ loader.Load(GURL(), GetProxy(&application));
}
} // namespace mojo
diff --git a/shell/android/native_viewport_application_loader.cc b/shell/android/native_viewport_application_loader.cc
index cd27ba4..3e8c76e 100644
--- a/shell/android/native_viewport_application_loader.cc
+++ b/shell/android/native_viewport_application_loader.cc
@@ -18,10 +18,8 @@
}
void NativeViewportApplicationLoader::Load(
- ApplicationManager* manager,
const GURL& url,
- InterfaceRequest<Application> application_request,
- LoadCallback callback) {
+ InterfaceRequest<Application> application_request) {
DCHECK(application_request.is_pending());
app_.reset(new ApplicationImpl(this, application_request.Pass()));
}
diff --git a/shell/android/native_viewport_application_loader.h b/shell/android/native_viewport_application_loader.h
index 773c4e4..13e3e8c 100644
--- a/shell/android/native_viewport_application_loader.h
+++ b/shell/android/native_viewport_application_loader.h
@@ -30,10 +30,8 @@
private:
// ApplicationLoader implementation.
- void Load(ApplicationManager* manager,
- const GURL& url,
- InterfaceRequest<Application> application_request,
- LoadCallback callback) override;
+ void Load(const GURL& url,
+ InterfaceRequest<Application> application_request) override;
void OnApplicationError(ApplicationManager* manager,
const GURL& url) override;
diff --git a/shell/android/ui_application_loader_android.cc b/shell/android/ui_application_loader_android.cc
index a9eacff..fff6251 100644
--- a/shell/android/ui_application_loader_android.cc
+++ b/shell/android/ui_application_loader_android.cc
@@ -23,15 +23,13 @@
}
void UIApplicationLoader::Load(
- ApplicationManager* manager,
const GURL& url,
- InterfaceRequest<Application> application_request,
- LoadCallback callback) {
+ InterfaceRequest<Application> application_request) {
DCHECK(application_request.is_pending());
ui_message_loop_->PostTask(
FROM_HERE,
base::Bind(&UIApplicationLoader::LoadOnUIThread, base::Unretained(this),
- manager, url, base::Passed(&application_request)));
+ url, base::Passed(&application_request)));
}
void UIApplicationLoader::OnApplicationError(ApplicationManager* manager,
@@ -42,10 +40,9 @@
}
void UIApplicationLoader::LoadOnUIThread(
- ApplicationManager* manager,
const GURL& url,
InterfaceRequest<Application> application_request) {
- loader_->Load(manager, url, application_request.Pass(), SimpleLoadCallback());
+ loader_->Load(url, application_request.Pass());
}
void UIApplicationLoader::OnApplicationErrorOnUIThread(
diff --git a/shell/android/ui_application_loader_android.h b/shell/android/ui_application_loader_android.h
index e7933c6..06ccc09 100644
--- a/shell/android/ui_application_loader_android.h
+++ b/shell/android/ui_application_loader_android.h
@@ -27,10 +27,8 @@
~UIApplicationLoader() override;
// ApplicationLoader overrides:
- void Load(ApplicationManager* manager,
- const GURL& url,
- InterfaceRequest<Application> application_request,
- LoadCallback callback) override;
+ void Load(const GURL& url,
+ InterfaceRequest<Application> application_request) override;
void OnApplicationError(ApplicationManager* manager,
const GURL& url) override;
@@ -41,8 +39,7 @@
// to |background_loader_| to do the actual loading.
// TODO: having this code take a |manager| is fragile (as ApplicationManager
// isn't thread safe).
- void LoadOnUIThread(ApplicationManager* manager,
- const GURL& url,
+ void LoadOnUIThread(const GURL& url,
InterfaceRequest<Application> application_request);
void OnApplicationErrorOnUIThread(ApplicationManager* manager,
const GURL& url);
diff --git a/shell/application_manager/application_loader.cc b/shell/application_manager/application_loader.cc
index 1e32a5a..6cbf032 100644
--- a/shell/application_manager/application_loader.cc
+++ b/shell/application_manager/application_loader.cc
@@ -19,7 +19,7 @@
} // namespace
-ApplicationLoader::LoadCallback ApplicationLoader::SimpleLoadCallback() {
+LoadCallback NativeApplicationLoader::SimpleLoadCallback() {
return base::Bind(&NotReached);
}
diff --git a/shell/application_manager/application_loader.h b/shell/application_manager/application_loader.h
index b081819..9e39de1 100644
--- a/shell/application_manager/application_loader.h
+++ b/shell/application_manager/application_loader.h
@@ -17,36 +17,14 @@
class Application;
class ApplicationManager;
-// Interface to allowing loading behavior to be established for schemes,
-// specific urls or as the default.
-// A ApplicationLoader is responsible to using whatever mechanism is appropriate
-// to load the application at url.
-// The handle to the shell is passed to that application so it can bind it to
-// a Shell instance. This will give the Application a way to connect to other
-// apps and services.
+// Interface to implement special application loading behavior for a particular
+// URL or scheme.
class MOJO_APPLICATION_MANAGER_EXPORT ApplicationLoader {
public:
- typedef base::Callback<void(const GURL& content_handler_url,
- InterfaceRequest<Application> application_request,
- URLResponsePtr url_request)> LoadCallback;
virtual ~ApplicationLoader() {}
- // Returns a callback that will should never be called.
- static LoadCallback SimpleLoadCallback();
-
- // Load the application named |url|. Applications can be loaded two ways:
- //
- // 1. |url| can refer directly to a Mojo application. In this case,
- // shell_handle should be used to implement the mojo.Application interface.
- //
- // 2. |url| can refer to some content that can be handled by some other Mojo
- // application. In this case, call callbacks and specify the URL of the
- // application that should handle the content. The specified application
- // must implement the mojo.ContentHandler interface.
- virtual void Load(ApplicationManager* application_manager,
- const GURL& url,
- InterfaceRequest<Application> application_request,
- LoadCallback callback) = 0;
+ virtual void Load(const GURL& url,
+ InterfaceRequest<Application> application_request) = 0;
// Called when the Application exits.
virtual void OnApplicationError(ApplicationManager* manager,
@@ -56,6 +34,26 @@
ApplicationLoader() {}
};
+// TODO(aa): Remove this temporary interface. This is just here as a stepping
+// stone, until I can tear apart NativeApplicationLoader into smaller pieces.
+// See comments at top of shell/native_application_loader.h.
+
+typedef base::Callback<void(const GURL& content_handler_url,
+ InterfaceRequest<Application> application_request,
+ URLResponsePtr url_request)> LoadCallback;
+
+class NativeApplicationLoader {
+ public:
+ // Returns a callback that should never be called.
+ static LoadCallback SimpleLoadCallback();
+
+ virtual ~NativeApplicationLoader() {}
+
+ virtual void Load(const GURL& url,
+ InterfaceRequest<Application> application_request,
+ LoadCallback callback) {}
+};
+
} // namespace mojo
#endif // SHELL_APPLICATION_MANAGER_APPLICATION_LOADER_H_
diff --git a/shell/application_manager/application_manager.cc b/shell/application_manager/application_manager.cc
index f0c4bac..255c093 100644
--- a/shell/application_manager/application_manager.cc
+++ b/shell/application_manager/application_manager.cc
@@ -104,37 +104,48 @@
InterfaceRequest<ServiceProvider> services,
ServiceProviderPtr exposed_services) {
DCHECK(requested_url.is_valid());
- GURL mapped_url = delegate_->ResolveMappings(requested_url);
// We check both the mapped and resolved urls for existing shell_impls because
// external applications can be registered for the unresolved mojo:foo urls.
- ShellImpl* shell_impl = GetShellImpl(mapped_url);
- if (shell_impl) {
- ConnectToClient(shell_impl, mapped_url, requestor_url, services.Pass(),
- exposed_services.Pass());
+
+ GURL mapped_url = delegate_->ResolveMappings(requested_url);
+ if (ConnectToRunningApplication(mapped_url, requestor_url, &services,
+ &exposed_services)) {
return;
}
GURL resolved_url = delegate_->ResolveURL(mapped_url);
- shell_impl = GetShellImpl(resolved_url);
- if (shell_impl) {
- ConnectToClient(shell_impl, resolved_url, requestor_url, services.Pass(),
- exposed_services.Pass());
+ if (ConnectToRunningApplication(resolved_url, requestor_url, &services,
+ &exposed_services)) {
return;
}
- ApplicationLoader* loader =
- GetLoaderForURL(mapped_url, DONT_INCLUDE_DEFAULT_LOADER);
- if (loader) {
- ConnectToApplicationImpl(requested_url, mapped_url, requestor_url,
- services.Pass(), exposed_services.Pass(), loader);
+ if (ConnectToApplicationWithLoader(requested_url, mapped_url, requestor_url,
+ &services, &exposed_services,
+ GetLoaderForURL(mapped_url))) {
return;
}
- loader = GetLoaderForURL(resolved_url, INCLUDE_DEFAULT_LOADER);
- if (loader) {
- ConnectToApplicationImpl(requested_url, resolved_url, requestor_url,
- services.Pass(), exposed_services.Pass(), loader);
+ if (ConnectToApplicationWithLoader(requested_url, resolved_url, requestor_url,
+ &services, &exposed_services,
+ GetLoaderForURL(resolved_url))) {
+ return;
+ }
+
+ if (ConnectToApplicationWithLoader(requested_url, resolved_url, requestor_url,
+ &services, &exposed_services,
+ default_loader_.get())) {
+ return;
+ }
+
+ // TODO(aa): This case should go away, see comments in
+ // NativeApplicationLoader.
+ if (native_application_loader_.get()) {
+ native_application_loader_->Load(
+ resolved_url, RegisterShell(requested_url, resolved_url, requestor_url,
+ services.Pass(), exposed_services.Pass()),
+ base::Bind(&ApplicationManager::LoadWithContentHandler,
+ weak_ptr_factory_.GetWeakPtr()));
return;
}
@@ -142,25 +153,51 @@
<< requested_url.spec();
}
-void ApplicationManager::ConnectToApplicationImpl(
+bool ApplicationManager::ConnectToRunningApplication(
+ const GURL& application_url,
+ const GURL& requestor_url,
+ InterfaceRequest<ServiceProvider>* services,
+ ServiceProviderPtr* exposed_services) {
+ ShellImpl* shell_impl = GetShellImpl(application_url);
+ if (!shell_impl)
+ return false;
+
+ ConnectToClient(shell_impl, application_url, requestor_url, services->Pass(),
+ exposed_services->Pass());
+ return true;
+}
+
+bool ApplicationManager::ConnectToApplicationWithLoader(
+ const GURL& requested_url,
+ const GURL& resolved_url,
+ const GURL& requestor_url,
+ InterfaceRequest<ServiceProvider>* services,
+ ServiceProviderPtr* exposed_services,
+ ApplicationLoader* loader) {
+ if (!loader)
+ return false;
+
+ loader->Load(resolved_url,
+ RegisterShell(requested_url, resolved_url, requestor_url,
+ services->Pass(), exposed_services->Pass()));
+ return true;
+}
+
+InterfaceRequest<Application> ApplicationManager::RegisterShell(
const GURL& requested_url,
const GURL& resolved_url,
const GURL& requestor_url,
InterfaceRequest<ServiceProvider> services,
- ServiceProviderPtr exposed_services,
- ApplicationLoader* loader) {
+ ServiceProviderPtr exposed_services) {
ApplicationPtr application;
InterfaceRequest<Application> application_request = GetProxy(&application);
ShellImpl* shell =
new ShellImpl(application.Pass(), this, requested_url, resolved_url);
url_to_shell_impl_[resolved_url] = shell;
shell->InitializeApplication(GetArgsForURL(requested_url));
-
- loader->Load(this, resolved_url, application_request.Pass(),
- base::Bind(&ApplicationManager::LoadWithContentHandler,
- weak_ptr_factory_.GetWeakPtr()));
ConnectToClient(shell, resolved_url, requestor_url, services.Pass(),
exposed_services.Pass());
+ return application_request.Pass();
}
ShellImpl* ApplicationManager::GetShellImpl(const GURL& url) {
@@ -234,17 +271,13 @@
url_to_args_[url] = args;
}
-ApplicationLoader* ApplicationManager::GetLoaderForURL(
- const GURL& url,
- IncludeDefaultLoader include_default_loader) {
+ApplicationLoader* ApplicationManager::GetLoaderForURL(const GURL& url) {
auto url_it = url_to_loader_.find(url);
if (url_it != url_to_loader_.end())
return url_it->second;
auto scheme_it = scheme_to_loader_.find(url.scheme());
if (scheme_it != scheme_to_loader_.end())
return scheme_it->second;
- if (include_default_loader == INCLUDE_DEFAULT_LOADER)
- return default_loader_.get();
return NULL;
}
@@ -257,10 +290,6 @@
DCHECK(it != url_to_shell_impl_.end());
delete it->second;
url_to_shell_impl_.erase(it);
- ApplicationLoader* loader =
- GetLoaderForURL(requested_url, INCLUDE_DEFAULT_LOADER);
- if (loader)
- loader->OnApplicationError(this, url);
delegate_->OnApplicationError(requested_url);
}
diff --git a/shell/application_manager/application_manager.h b/shell/application_manager/application_manager.h
index ab6b2f0..e8858e1 100644
--- a/shell/application_manager/application_manager.h
+++ b/shell/application_manager/application_manager.h
@@ -79,6 +79,10 @@
void set_default_loader(scoped_ptr<ApplicationLoader> loader) {
default_loader_ = loader.Pass();
}
+ void set_native_application_loader(
+ scoped_ptr<NativeApplicationLoader> loader) {
+ native_application_loader_ = loader.Pass();
+ }
// Sets a Loader to be used for a specific url.
void SetLoaderForURL(scoped_ptr<ApplicationLoader> loader, const GURL& url);
// Sets a Loader to be used for a specific url scheme.
@@ -97,11 +101,6 @@
void OnShellImplError(ShellImpl* shell_impl);
private:
- enum IncludeDefaultLoader {
- INCLUDE_DEFAULT_LOADER,
- DONT_INCLUDE_DEFAULT_LOADER,
- };
-
class ContentHandlerConnection;
typedef std::map<std::string, ApplicationLoader*> SchemeToLoaderMap;
@@ -110,12 +109,25 @@
typedef std::map<GURL, ContentHandlerConnection*> URLToContentHandlerMap;
typedef std::map<GURL, std::vector<std::string>> URLToArgsMap;
- void ConnectToApplicationImpl(const GURL& requested_url,
- const GURL& resolved_url,
- const GURL& requestor_url,
- InterfaceRequest<ServiceProvider> services,
- ServiceProviderPtr exposed_services,
- ApplicationLoader* loader);
+ bool ConnectToRunningApplication(const GURL& application_url,
+ const GURL& requestor_url,
+ InterfaceRequest<ServiceProvider>* services,
+ ServiceProviderPtr* exposed_services);
+
+ bool ConnectToApplicationWithLoader(
+ const GURL& requested_url,
+ const GURL& resolved_url,
+ const GURL& requestor_url,
+ InterfaceRequest<ServiceProvider>* services,
+ ServiceProviderPtr* exposed_services,
+ ApplicationLoader* loader);
+
+ InterfaceRequest<Application> RegisterShell(
+ const GURL& requested_url,
+ const GURL& resolved_url,
+ const GURL& requestor_url,
+ InterfaceRequest<ServiceProvider> services,
+ ServiceProviderPtr exposed_services);
ShellImpl* GetShellImpl(const GURL& url);
@@ -130,9 +142,8 @@
URLResponsePtr url_response);
// Return the appropriate loader for |url|. This can return NULL if there is
- // no default loader configured.
- ApplicationLoader* GetLoaderForURL(const GURL& url,
- IncludeDefaultLoader fallback);
+ // no loader configured for the URL.
+ ApplicationLoader* GetLoaderForURL(const GURL& url);
// Removes a ContentHandler when it encounters an error.
void OnContentHandlerError(ContentHandlerConnection* content_handler);
@@ -142,9 +153,11 @@
Delegate* delegate_;
// Loader management.
+ // Loaders are chosen in the order they are listed here.
URLToLoaderMap url_to_loader_;
SchemeToLoaderMap scheme_to_loader_;
scoped_ptr<ApplicationLoader> default_loader_;
+ scoped_ptr<NativeApplicationLoader> native_application_loader_;
URLToShellImplMap url_to_shell_impl_;
URLToContentHandlerMap url_to_content_handler_;
diff --git a/shell/application_manager/application_manager_unittest.cc b/shell/application_manager/application_manager_unittest.cc
index 0481e74..4c8368b 100644
--- a/shell/application_manager/application_manager_unittest.cc
+++ b/shell/application_manager/application_manager_unittest.cc
@@ -112,10 +112,8 @@
private:
// ApplicationLoader implementation.
- void Load(ApplicationManager* manager,
- const GURL& url,
- InterfaceRequest<Application> application_request,
- LoadCallback callback) override {
+ void Load(const GURL& url,
+ InterfaceRequest<Application> application_request) override {
++num_loads_;
test_app_.reset(new ApplicationImpl(this, application_request.Pass()));
}
@@ -334,10 +332,8 @@
~Tester() override {}
private:
- void Load(ApplicationManager* manager,
- const GURL& url,
- InterfaceRequest<Application> application_request,
- LoadCallback callback) override {
+ void Load(const GURL& url,
+ InterfaceRequest<Application> application_request) override {
app_.reset(new ApplicationImpl(this, application_request.Pass()));
}
diff --git a/shell/context.cc b/shell/context.cc
index 0c43944..0707cef 100644
--- a/shell/context.cc
+++ b/shell/context.cc
@@ -29,10 +29,10 @@
#include "shell/application_manager/application_loader.h"
#include "shell/application_manager/application_manager.h"
#include "shell/command_line_util.h"
-#include "shell/dynamic_application_loader.h"
#include "shell/external_application_listener.h"
#include "shell/filename_util.h"
#include "shell/in_process_dynamic_service_runner.h"
+#include "shell/native_application_loader.h"
#include "shell/out_of_process_dynamic_service_runner.h"
#include "shell/switches.h"
#include "url/gurl.h"
@@ -57,7 +57,7 @@
static base::LazyInstance<Setup>::Leaky setup = LAZY_INSTANCE_INITIALIZER;
-void InitContentHandlers(DynamicApplicationLoader* loader,
+void InitContentHandlers(NativeApplicationLoader* loader,
base::CommandLine* command_line) {
// Default content handlers.
loader->RegisterContentHandler("application/pdf", GURL("mojo:pdf_viewer"));
@@ -257,11 +257,11 @@
else
runner_factory.reset(new InProcessDynamicServiceRunnerFactory());
- DynamicApplicationLoader* dynamic_application_loader =
- new DynamicApplicationLoader(this, runner_factory.Pass());
- InitContentHandlers(dynamic_application_loader, command_line);
- application_manager_.set_default_loader(
- scoped_ptr<ApplicationLoader>(dynamic_application_loader));
+ scoped_ptr<NativeApplicationLoader> native_application_loader(
+ new NativeApplicationLoader(this, runner_factory.Pass()));
+ InitContentHandlers(native_application_loader.get(), command_line);
+ application_manager_.set_native_application_loader(
+ scoped_ptr<NativeApplicationLoader>(native_application_loader.Pass()));
ServiceProviderPtr tracing_service_provider_ptr;
new TracingServiceProvider(GetProxy(&tracing_service_provider_ptr));
diff --git a/shell/context.h b/shell/context.h
index 5bd8ecd..4d2cf34 100644
--- a/shell/context.h
+++ b/shell/context.h
@@ -17,8 +17,8 @@
namespace shell {
-class DynamicApplicationLoader;
class ExternalApplicationListener;
+class NativeApplicationLoader;
// The "global" context for the shell's main process.
class Context : ApplicationManager::Delegate, embedder::ProcessDelegate {
diff --git a/shell/external_application_listener_unittest.cc b/shell/external_application_listener_unittest.cc
index e841486..0678ab8 100644
--- a/shell/external_application_listener_unittest.cc
+++ b/shell/external_application_listener_unittest.cc
@@ -35,10 +35,8 @@
NotAnApplicationLoader() {}
~NotAnApplicationLoader() override {}
- void Load(ApplicationManager* application_manager,
- const GURL& url,
- InterfaceRequest<Application> application_request,
- LoadCallback callback) override {
+ void Load(const GURL& url,
+ InterfaceRequest<Application> application_request) override {
NOTREACHED();
}
diff --git a/shell/dynamic_application_loader.cc b/shell/native_application_loader.cc
similarity index 92%
rename from shell/dynamic_application_loader.cc
rename to shell/native_application_loader.cc
index afc9a53..f7960ca 100644
--- a/shell/dynamic_application_loader.cc
+++ b/shell/native_application_loader.cc
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "shell/dynamic_application_loader.h"
+#include "shell/native_application_loader.h"
#include "base/bind.h"
#include "base/command_line.h"
@@ -46,21 +46,21 @@
// Encapsulates loading and running one individual application.
//
-// Loaders are owned by DynamicApplicationLoader. DynamicApplicationLoader must
+// Loaders are owned by NativeApplicationLoader. NativeApplicationLoader 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)
+// NativeApplicationLoader going away (and taking all the Loaders with it)
// while the async operation is outstanding.
-class DynamicApplicationLoader::Loader {
+class NativeApplicationLoader::Loader {
public:
Loader(DynamicServiceRunner::CleanupBehavior cleanup_behavior,
MimeTypeToURLMap* mime_type_to_url,
Context* context,
DynamicServiceRunnerFactory* runner_factory,
InterfaceRequest<Application> application_request,
- ApplicationLoader::LoadCallback load_callback,
+ LoadCallback load_callback,
const LoaderCompleteCallback& loader_complete_callback)
: cleanup_behavior_(cleanup_behavior),
application_request_(application_request.Pass()),
@@ -151,7 +151,7 @@
DynamicServiceRunner::CleanupBehavior cleanup_behavior_;
InterfaceRequest<Application> application_request_;
- ApplicationLoader::LoadCallback load_callback_;
+ LoadCallback load_callback_;
LoaderCompleteCallback loader_complete_callback_;
Context* context_;
MimeTypeToURLMap* mime_type_to_url_;
@@ -161,14 +161,14 @@
};
// A loader for local files.
-class DynamicApplicationLoader::LocalLoader : public Loader {
+class NativeApplicationLoader::LocalLoader : public Loader {
public:
LocalLoader(const GURL& url,
MimeTypeToURLMap* mime_type_to_url,
Context* context,
DynamicServiceRunnerFactory* runner_factory,
InterfaceRequest<Application> application_request,
- ApplicationLoader::LoadCallback load_callback,
+ LoadCallback load_callback,
const LoaderCompleteCallback& loader_complete_callback)
: Loader(DynamicServiceRunner::DontDeleteAppPath,
mime_type_to_url,
@@ -249,7 +249,7 @@
};
// A loader for network files.
-class DynamicApplicationLoader::NetworkLoader : public Loader {
+class NativeApplicationLoader::NetworkLoader : public Loader {
public:
NetworkLoader(const GURL& url,
NetworkService* network_service,
@@ -257,7 +257,7 @@
Context* context,
DynamicServiceRunnerFactory* runner_factory,
InterfaceRequest<Application> application_request,
- ApplicationLoader::LoadCallback load_callback,
+ LoadCallback load_callback,
const LoaderCompleteCallback& loader_complete_callback)
: Loader(DynamicServiceRunner::DeleteAppPath,
mime_type_to_url,
@@ -445,23 +445,23 @@
DISALLOW_COPY_AND_ASSIGN(NetworkLoader);
};
-DynamicApplicationLoader::DynamicApplicationLoader(
+NativeApplicationLoader::NativeApplicationLoader(
Context* context,
scoped_ptr<DynamicServiceRunnerFactory> runner_factory)
: context_(context),
runner_factory_(runner_factory.Pass()),
- // Unretained() is correct here because DynamicApplicationLoader owns the
+ // Unretained() is correct here because NativeApplicationLoader owns the
// loaders that we pass this callback to.
loader_complete_callback_(
- base::Bind(&DynamicApplicationLoader::LoaderComplete,
+ base::Bind(&NativeApplicationLoader::LoaderComplete,
base::Unretained(this))) {
}
-DynamicApplicationLoader::~DynamicApplicationLoader() {
+NativeApplicationLoader::~NativeApplicationLoader() {
}
-void DynamicApplicationLoader::RegisterContentHandler(
+void NativeApplicationLoader::RegisterContentHandler(
const std::string& mime_type,
const GURL& content_handler_url) {
DCHECK(content_handler_url.is_valid())
@@ -469,8 +469,7 @@
mime_type_to_url_[mime_type] = content_handler_url;
}
-void DynamicApplicationLoader::Load(
- ApplicationManager* manager,
+void NativeApplicationLoader::Load(
const GURL& url,
InterfaceRequest<Application> application_request,
LoadCallback load_callback) {
@@ -492,13 +491,7 @@
loader_complete_callback_));
}
-void DynamicApplicationLoader::OnApplicationError(ApplicationManager* manager,
- const GURL& url) {
- // TODO(darin): What should we do about service errors? This implies that
- // the app closed its handle to the service manager. Maybe we don't care?
-}
-
-void DynamicApplicationLoader::LoaderComplete(Loader* loader) {
+void NativeApplicationLoader::LoaderComplete(Loader* loader) {
loaders_.erase(std::find(loaders_.begin(), loaders_.end(), loader));
}
diff --git a/shell/dynamic_application_loader.h b/shell/native_application_loader.h
similarity index 68%
rename from shell/dynamic_application_loader.h
rename to shell/native_application_loader.h
index e91bcea..db66599 100644
--- a/shell/dynamic_application_loader.h
+++ b/shell/native_application_loader.h
@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#ifndef SHELL_DYNAMIC_APPLICATION_LOADER_H_
-#define SHELL_DYNAMIC_APPLICATION_LOADER_H_
+#ifndef SHELL_NATIVE_APPLICATION_LOADER_H_
+#define SHELL_NATIVE_APPLICATION_LOADER_H_
#include <map>
@@ -24,26 +24,24 @@
class DynamicServiceRunnerFactory;
class DynamicServiceRunner;
-// An implementation of ApplicationLoader that retrieves a dynamic library
-// containing the implementation of the service and loads/runs it (via a
-// DynamicServiceRunner).
-class DynamicApplicationLoader : public ApplicationLoader {
+// Loads applications that might be either DSOs or content applications.
+// TODO(aa): Tear this apart into:
+// - fetching urls (probably two sublcasses for http/file)
+// - detecting type of response (dso, content, nacl (future))
+// - executing response (either via DynamicServiceRunner or a content handler)
+class NativeApplicationLoader : public mojo::NativeApplicationLoader {
public:
- DynamicApplicationLoader(
+ NativeApplicationLoader(
Context* context,
scoped_ptr<DynamicServiceRunnerFactory> runner_factory);
- ~DynamicApplicationLoader() override;
+ ~NativeApplicationLoader() override;
void RegisterContentHandler(const std::string& mime_type,
const GURL& content_handler_url);
- // ApplicationLoader methods:
- void Load(ApplicationManager* manager,
- const GURL& url,
+ void Load(const GURL& url,
InterfaceRequest<Application> application_request,
LoadCallback callback) override;
- void OnApplicationError(ApplicationManager* manager,
- const GURL& url) override;
private:
class Loader;
@@ -62,10 +60,10 @@
ScopedVector<Loader> loaders_;
LoaderCompleteCallback loader_complete_callback_;
- DISALLOW_COPY_AND_ASSIGN(DynamicApplicationLoader);
+ DISALLOW_COPY_AND_ASSIGN(NativeApplicationLoader);
};
} // namespace shell
} // namespace mojo
-#endif // SHELL_DYNAMIC_APPLICATION_LOADER_H_
+#endif // SHELL_NATIVE_APPLICATION_LOADER_H_
diff --git a/shell/dynamic_application_loader_unittest.cc b/shell/native_application_loader_unittest.cc
similarity index 82%
rename from shell/dynamic_application_loader_unittest.cc
rename to shell/native_application_loader_unittest.cc
index 833e475..473412a 100644
--- a/shell/dynamic_application_loader_unittest.cc
+++ b/shell/native_application_loader_unittest.cc
@@ -4,9 +4,9 @@
#include "base/files/scoped_temp_dir.h"
#include "shell/context.h"
-#include "shell/dynamic_application_loader.h"
#include "shell/dynamic_service_runner.h"
#include "shell/filename_util.h"
+#include "shell/native_application_loader.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace mojo {
@@ -60,32 +60,32 @@
} // namespace
-class DynamicApplicationLoaderTest : public testing::Test {
+class NativeApplicationLoaderTest : public testing::Test {
public:
- DynamicApplicationLoaderTest() {}
- ~DynamicApplicationLoaderTest() override {}
+ NativeApplicationLoaderTest() {}
+ ~NativeApplicationLoaderTest() override {}
void SetUp() override {
context_.Init();
scoped_ptr<DynamicServiceRunnerFactory> factory(
new TestDynamicServiceRunnerFactory(&state_));
- loader_.reset(new DynamicApplicationLoader(&context_, factory.Pass()));
+ loader_.reset(new NativeApplicationLoader(&context_, factory.Pass()));
}
protected:
Context context_;
base::MessageLoop loop_;
- scoped_ptr<DynamicApplicationLoader> loader_;
+ scoped_ptr<NativeApplicationLoader> loader_;
TestState state_;
};
-TEST_F(DynamicApplicationLoaderTest, DoesNotExist) {
+TEST_F(NativeApplicationLoaderTest, DoesNotExist) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
base::FilePath nonexistent_file(FILE_PATH_LITERAL("nonexistent.txt"));
GURL url(FilePathToFileURL(temp_dir.path().Append(nonexistent_file)));
ApplicationPtr application;
- loader_->Load(context_.application_manager(), url, GetProxy(&application),
- ApplicationLoader::SimpleLoadCallback());
+ loader_->Load(url, GetProxy(&application),
+ NativeApplicationLoader::SimpleLoadCallback());
EXPECT_FALSE(state_.runner_was_created);
EXPECT_FALSE(state_.runner_was_started);
EXPECT_FALSE(state_.runner_was_destroyed);
