Moves BackgroundShellApplicationLoader to shell/android As this is the only place that uses it. Also renamed to BackgroundApplicationLoader. BUG=none TEST=none R=qsr@chromium.org Review URL: https://codereview.chromium.org/788243007
diff --git a/shell/BUILD.gn b/shell/BUILD.gn index 4506cc2..d36b7bb 100644 --- a/shell/BUILD.gn +++ b/shell/BUILD.gn
@@ -196,6 +196,8 @@ "android/android_handler.h", "android/android_handler_loader.cc", "android/android_handler_loader.h", + "android/background_application_loader.cc", + "android/background_application_loader.h", "android/native_viewport_application_loader.cc", "android/native_viewport_application_loader.h", "android/ui_application_loader_android.cc", @@ -398,6 +400,8 @@ ] if (is_android) { + sources += [ "android/background_application_loader_unittest.cc" ] + deps += [ # TODO(GYP): #'../testing/android/native_test.gyp:native_test_native_code',
diff --git a/shell/android/background_application_loader.cc b/shell/android/background_application_loader.cc new file mode 100644 index 0000000..8e6d0f4 --- /dev/null +++ b/shell/android/background_application_loader.cc
@@ -0,0 +1,90 @@ +// 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 "shell/android/background_application_loader.h" + +#include "base/bind.h" +#include "base/run_loop.h" +#include "mojo/application_manager/application_manager.h" + +namespace mojo { + +BackgroundApplicationLoader::BackgroundApplicationLoader( + scoped_ptr<ApplicationLoader> real_loader, + const std::string& thread_name, + base::MessageLoop::Type message_loop_type) + : loader_(real_loader.Pass()), + message_loop_type_(message_loop_type), + thread_name_(thread_name), + message_loop_created_(true, false) { +} + +BackgroundApplicationLoader::~BackgroundApplicationLoader() { + if (thread_) + thread_->Join(); +} + +void BackgroundApplicationLoader::Load(ApplicationManager* manager, + const GURL& url, + ScopedMessagePipeHandle shell_handle, + LoadCallback callback) { + DCHECK(shell_handle.is_valid()); + if (!thread_) { + // TODO(tim): It'd be nice if we could just have each Load call + // result in a new thread like DynamicService{Loader, Runner}. But some + // loaders are creating multiple ApplicationImpls (NetworkApplicationLoader) + // sharing a delegate (etc). So we have to keep it single threaded, wait + // for the thread to initialize, and post to the TaskRunner for subsequent + // Load calls for now. + thread_.reset(new base::DelegateSimpleThread(this, thread_name_)); + thread_->Start(); + message_loop_created_.Wait(); + DCHECK(task_runner_.get()); + } + + task_runner_->PostTask( + FROM_HERE, + base::Bind(&BackgroundApplicationLoader::LoadOnBackgroundThread, + base::Unretained(this), manager, url, + base::Passed(&shell_handle))); +} + +void BackgroundApplicationLoader::OnApplicationError( + ApplicationManager* manager, + const GURL& url) { + task_runner_->PostTask( + FROM_HERE, + base::Bind( + &BackgroundApplicationLoader::OnApplicationErrorOnBackgroundThread, + base::Unretained(this), manager, url)); +} + +void BackgroundApplicationLoader::Run() { + base::MessageLoop message_loop(message_loop_type_); + base::RunLoop loop; + task_runner_ = message_loop.task_runner(); + quit_closure_ = loop.QuitClosure(); + message_loop_created_.Signal(); + loop.Run(); + + // Destroy |loader_| on the thread it's actually used on. + loader_.reset(); +} + +void BackgroundApplicationLoader::LoadOnBackgroundThread( + ApplicationManager* manager, + const GURL& url, + ScopedMessagePipeHandle shell_handle) { + DCHECK(task_runner_->RunsTasksOnCurrentThread()); + loader_->Load(manager, url, shell_handle.Pass(), SimpleLoadCallback()); +} + +void BackgroundApplicationLoader::OnApplicationErrorOnBackgroundThread( + ApplicationManager* manager, + const GURL& url) { + DCHECK(task_runner_->RunsTasksOnCurrentThread()); + loader_->OnApplicationError(manager, url); +} + +} // namespace mojo
diff --git a/shell/android/background_application_loader.h b/shell/android/background_application_loader.h new file mode 100644 index 0000000..5321d97 --- /dev/null +++ b/shell/android/background_application_loader.h
@@ -0,0 +1,70 @@ +// 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. + +#ifndef SHELL_ANDROID_BACKGROUND_APPLICATION_LOADER_H_ +#define SHELL_ANDROID_BACKGROUND_APPLICATION_LOADER_H_ + +#include "base/macros.h" +#include "base/memory/scoped_ptr.h" +#include "base/message_loop/message_loop.h" +#include "base/synchronization/waitable_event.h" +#include "base/threading/simple_thread.h" +#include "mojo/application_manager/application_loader.h" + +namespace mojo { + +class MOJO_APPLICATION_MANAGER_EXPORT BackgroundApplicationLoader + : public ApplicationLoader, + public base::DelegateSimpleThread::Delegate { + public: + BackgroundApplicationLoader(scoped_ptr<ApplicationLoader> real_loader, + const std::string& thread_name, + base::MessageLoop::Type message_loop_type); + ~BackgroundApplicationLoader() override; + + // ApplicationLoader overrides: + void Load(ApplicationManager* manager, + const GURL& url, + ScopedMessagePipeHandle shell_handle, + LoadCallback callback) override; + void OnApplicationError(ApplicationManager* manager, + const GURL& url) override; + + private: + // |base::DelegateSimpleThread::Delegate| method: + void Run() override; + + // 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, + ScopedMessagePipeHandle shell_handle); + void OnApplicationErrorOnBackgroundThread(ApplicationManager* manager, + const GURL& url); + bool quit_on_shutdown_; + scoped_ptr<ApplicationLoader> loader_; + + const base::MessageLoop::Type message_loop_type_; + const std::string thread_name_; + + // Created on |thread_| during construction of |this|. Protected against + // uninitialized use by |message_loop_created_|, and protected against + // use-after-free by holding a reference to the thread-safe object. Note + // that holding a reference won't hold |thread_| from exiting. + scoped_refptr<base::TaskRunner> task_runner_; + base::WaitableEvent message_loop_created_; + + // Lives on |thread_|. + base::Closure quit_closure_; + + scoped_ptr<base::DelegateSimpleThread> thread_; + + DISALLOW_COPY_AND_ASSIGN(BackgroundApplicationLoader); +}; + +} // namespace mojo + +#endif // SHELL_ANDROID_BACKGROUND_APPLICATION_LOADER_H_
diff --git a/shell/android/background_application_loader_unittest.cc b/shell/android/background_application_loader_unittest.cc new file mode 100644 index 0000000..09bc2c8 --- /dev/null +++ b/shell/android/background_application_loader_unittest.cc
@@ -0,0 +1,56 @@ +// 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 "shell/android/background_application_loader.h" + +#include "testing/gtest/include/gtest/gtest.h" + +namespace mojo { + +namespace { + +class DummyLoader : public ApplicationLoader { + public: + DummyLoader() : simulate_app_quit_(true) {} + ~DummyLoader() override {} + + // ApplicationLoader overrides: + void Load(ApplicationManager* manager, + const GURL& url, + ScopedMessagePipeHandle shell_handle, + LoadCallback callback) override { + if (simulate_app_quit_) + base::MessageLoop::current()->Quit(); + } + + void OnApplicationError(ApplicationManager* manager, + const GURL& url) override {} + + void DontSimulateAppQuit() { simulate_app_quit_ = false; } + + private: + bool simulate_app_quit_; +}; + +} // namespace + +// Tests that the loader can start and stop gracefully. +TEST(BackgroundApplicationLoaderTest, StartStop) { + scoped_ptr<ApplicationLoader> real_loader(new DummyLoader()); + BackgroundApplicationLoader loader(real_loader.Pass(), "test", + base::MessageLoop::TYPE_DEFAULT); +} + +// Tests that the loader can load a service that is well behaved (quits +// itself). +TEST(BackgroundApplicationLoaderTest, Load) { + scoped_ptr<ApplicationLoader> real_loader(new DummyLoader()); + BackgroundApplicationLoader loader(real_loader.Pass(), "test", + base::MessageLoop::TYPE_DEFAULT); + MessagePipe dummy; + loader.Load(NULL, GURL(), dummy.handle0.Pass(), + ApplicationLoader::SimpleLoadCallback()); +} + +} // namespace mojo
diff --git a/shell/android/mojo_main.cc b/shell/android/mojo_main.cc index d19e3db..086d29f 100644 --- a/shell/android/mojo_main.cc +++ b/shell/android/mojo_main.cc
@@ -18,8 +18,8 @@ #include "base/message_loop/message_loop.h" #include "jni/MojoMain_jni.h" #include "mojo/application_manager/application_loader.h" -#include "mojo/application_manager/background_shell_application_loader.h" #include "shell/android/android_handler_loader.h" +#include "shell/android/background_application_loader.h" #include "shell/android/native_viewport_application_loader.h" #include "shell/android/ui_application_loader_android.h" #include "shell/context.h" @@ -52,7 +52,7 @@ // MojoShell application as the JNI bridge to bootstrap execution of other // Android Mojo apps that need JNI. context->application_manager()->SetLoaderForURL( - make_scoped_ptr(new BackgroundShellApplicationLoader( + make_scoped_ptr(new BackgroundApplicationLoader( make_scoped_ptr(new AndroidHandlerLoader()), "android_handler", base::MessageLoop::TYPE_DEFAULT)), GURL("mojo:android_handler"));
diff --git a/shell/context.cc b/shell/context.cc index c38de5e..cd65273 100644 --- a/shell/context.cc +++ b/shell/context.cc
@@ -17,7 +17,6 @@ #include "build/build_config.h" #include "mojo/application_manager/application_loader.h" #include "mojo/application_manager/application_manager.h" -#include "mojo/application_manager/background_shell_application_loader.h" #include "mojo/common/tracing_impl.h" #include "mojo/edk/embedder/embedder.h" #include "mojo/edk/embedder/simple_platform_support.h"