Remove Client relationship between mojo.Shell/mojo.Application
Instead of mojo.Shell and mojo.Application being clients of each other
they are now separate interfaces. An implementation of mojo.Application
receives a handle to the shell in its Initialize call, as described in
this doc:
https://docs.google.com/document/d/1xjt_TPjTu0elix8fNdBgWmnjJdJAtqSr1XDS_C-Ct8E/edit?usp=sharing
An analogous change is made to the content handler definition.
In C++, this is handled by the mojo::ApplicationImpl class which stores
shell handle in its implementation of Initialize. Thus the only change
for most C++ applications is the meaning of the handle in MojoMain is
different, although mains that use the ApplicationRunners do the same
thing with it. Connecting to other apps is largely the same although
instead of grabbing the ApplicationImpl's client() to talk to the shell
code must now use the ApplicationImpl::shell() getter.
In JavaScript, the initialization sequence is a bit different although
this is hidden mostly in the Application class which calls initialize()
on its subclass with the same set of parameters. Connecting to another
application looks different, especially for sky code using the shell
proxy handle exposed via internals. Hans has some ideas about how to
make this a bit nicer.
Python apps similarly need to change their startup sequence a bit. I
didn't find a common library to take care of this dance, although it's
possible I just missed it.
Other languages probably a bit of reworking - I fixed everything here
that is covered by mojob.py test but some might be missing.
This patch also uses typed handles (i.e. InterfacePtr<> or
InterfaceRequest<> or their type aliases) wherever possible instead of
ScopedMessagePipeHandle for clarity.
R=davemoore@chromium.org
Review URL: https://codereview.chromium.org/868463008
diff --git a/mojo/public/cpp/application/BUILD.gn b/mojo/public/cpp/application/BUILD.gn
index 7e0d804..05708c7 100644
--- a/mojo/public/cpp/application/BUILD.gn
+++ b/mojo/public/cpp/application/BUILD.gn
@@ -83,6 +83,7 @@
]
mojo_sdk_deps = [
+ "mojo/public/interfaces/application",
"mojo/public/cpp/environment:standalone",
"mojo/public/cpp/system",
"mojo/public/cpp/utility",
diff --git a/mojo/public/cpp/application/application_impl.h b/mojo/public/cpp/application/application_impl.h
index eb8c489..b60ca84 100644
--- a/mojo/public/cpp/application/application_impl.h
+++ b/mojo/public/cpp/application/application_impl.h
@@ -50,9 +50,10 @@
// app.AddService<BarImpl>(&context);
//
//
-class ApplicationImpl : public InterfaceImpl<Application> {
+class ApplicationImpl : public Application {
public:
- ApplicationImpl(ApplicationDelegate* delegate, ShellPtr shell);
+ ApplicationImpl(ApplicationDelegate* delegate,
+ InterfaceRequest<Application> request);
~ApplicationImpl() override;
Shell* shell() const { return shell_.get(); }
@@ -72,11 +73,16 @@
ConnectToApplication(application_url)->ConnectToService(ptr);
}
- // Unbind the shell from this application and return its handle.
- ShellPtr UnbindShell();
-
// Application implementation.
- void Initialize(Array<String> args) override;
+ void Initialize(ShellPtr shell, Array<String> args) override;
+
+ // Block until the Application is initialized, if it is not already.
+ void WaitForInitialize();
+
+ // Unbinds the Shell and Application connections. Must be called after
+ // Initialize.
+ void UnbindConnections(InterfaceRequest<Application>* application_request,
+ ShellPtr* shell);
// Quits the main run loop for this application.
static void Terminate();
@@ -103,6 +109,7 @@
ServiceRegistryList incoming_service_registries_;
ServiceRegistryList outgoing_service_registries_;
ApplicationDelegate* delegate_;
+ Binding<Application> binding_;
ShellPtr shell_;
ShellPtrWatcher* shell_watch_;
std::vector<std::string> args_;
diff --git a/mojo/public/cpp/application/application_test_base.h b/mojo/public/cpp/application/application_test_base.h
index d9a2af0..10763be 100644
--- a/mojo/public/cpp/application/application_test_base.h
+++ b/mojo/public/cpp/application/application_test_base.h
@@ -9,7 +9,7 @@
#include "mojo/public/cpp/bindings/array.h"
#include "mojo/public/cpp/bindings/string.h"
#include "mojo/public/cpp/system/macros.h"
-#include "mojo/public/interfaces/application/shell.mojom.h"
+#include "mojo/public/interfaces/application/application.mojom.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace mojo {
@@ -23,7 +23,7 @@
// Run all application tests. This must be called after the environment is
// initialized, to support construction of a default run loop.
-MojoResult RunAllTests(ShellPtr shell);
+MojoResult RunAllTests(MojoHandle application_request_handle);
// A GTEST base class for application testing executed in mojo_shell.
class ApplicationTestBase : public testing::Test {
@@ -37,10 +37,6 @@
// Get the ApplicationDelegate for the application to be tested.
virtual ApplicationDelegate* GetApplicationDelegate();
- // A testing::Test::SetUp helper to override the application command
- // line arguments.
- void SetUpWithArgs(const Array<String>& args);
-
// testing::Test:
void SetUp() override;
void TearDown() override;
diff --git a/mojo/public/cpp/application/lib/application_impl.cc b/mojo/public/cpp/application/lib/application_impl.cc
index 89a9ab0..4d1f8dc 100644
--- a/mojo/public/cpp/application/lib/application_impl.cc
+++ b/mojo/public/cpp/application/lib/application_impl.cc
@@ -24,13 +24,12 @@
MOJO_DISALLOW_COPY_AND_ASSIGN(ShellPtrWatcher);
};
-ApplicationImpl::ApplicationImpl(ApplicationDelegate* delegate, ShellPtr shell)
+ApplicationImpl::ApplicationImpl(ApplicationDelegate* delegate,
+ InterfaceRequest<Application> request)
: initialized_(false),
delegate_(delegate),
- shell_(shell.Pass()),
- shell_watch_(new ShellPtrWatcher(this)) {
- shell_.set_client(this);
- shell_.set_error_handler(shell_watch_);
+ binding_(this, request.Pass()),
+ shell_watch_(nullptr) {
}
bool ApplicationImpl::HasArg(const std::string& arg) const {
@@ -57,7 +56,7 @@
ApplicationConnection* ApplicationImpl::ConnectToApplication(
const String& application_url) {
- MOJO_CHECK(initialized_);
+ MOJO_CHECK(shell_);
ServiceProviderPtr local_services;
InterfaceRequest<ServiceProvider> local_request = GetProxy(&local_services);
ServiceProviderPtr remote_services;
@@ -73,20 +72,26 @@
return registry;
}
-ShellPtr ApplicationImpl::UnbindShell() {
- MOJO_CHECK(shell_);
- ShellPtr unbound_shell;
- unbound_shell.Bind(shell_.PassMessagePipe());
- return unbound_shell.Pass();
-}
-
-void ApplicationImpl::Initialize(Array<String> args) {
- MOJO_CHECK(!initialized_);
- initialized_ = true;
+void ApplicationImpl::Initialize(ShellPtr shell, Array<String> args) {
+ shell_ = shell.Pass();
+ shell_watch_ = new ShellPtrWatcher(this);
+ shell_.set_error_handler(shell_watch_);
args_ = args.To<std::vector<std::string>>();
delegate_->Initialize(this);
}
+void ApplicationImpl::WaitForInitialize() {
+ if (!shell_)
+ binding_.WaitForIncomingMethodCall();
+}
+
+void ApplicationImpl::UnbindConnections(
+ InterfaceRequest<Application>* application_request,
+ ShellPtr* shell) {
+ *application_request = binding_.Unbind();
+ shell->Bind(shell_.PassMessagePipe());
+}
+
void ApplicationImpl::AcceptConnection(
const String& requestor_url,
InterfaceRequest<ServiceProvider> services,
diff --git a/mojo/public/cpp/application/lib/application_runner.cc b/mojo/public/cpp/application/lib/application_runner.cc
index dbfdd4f..0737dc7 100644
--- a/mojo/public/cpp/application/lib/application_runner.cc
+++ b/mojo/public/cpp/application/lib/application_runner.cc
@@ -23,13 +23,12 @@
assert(!delegate_);
}
-MojoResult ApplicationRunner::Run(MojoHandle shell_handle) {
+MojoResult ApplicationRunner::Run(MojoHandle app_request_handle) {
Environment env;
{
RunLoop loop;
- ShellPtr shell;
- shell.Bind(MakeScopedHandle(MessagePipeHandle(shell_handle)));
- ApplicationImpl app(delegate_, shell.Pass());
+ ApplicationImpl app(delegate_, MakeRequest<Application>(MakeScopedHandle(
+ MessagePipeHandle(app_request_handle))));
loop.Run();
}
diff --git a/mojo/public/cpp/application/lib/application_test_base.cc b/mojo/public/cpp/application/lib/application_test_base.cc
index 9130c82..72058a4 100644
--- a/mojo/public/cpp/application/lib/application_test_base.cc
+++ b/mojo/public/cpp/application/lib/application_test_base.cc
@@ -4,42 +4,54 @@
#include "mojo/public/cpp/application/application_test_base.h"
-#include "mojo/public/cpp/application/application_delegate.h"
#include "mojo/public/cpp/application/application_impl.h"
+#include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/environment/environment.h"
#include "mojo/public/cpp/system/message_pipe.h"
+#include "mojo/public/interfaces/application/application.mojom.h"
namespace mojo {
namespace test {
namespace {
-
-// This shell handle is shared by multiple test application instances.
-ShellPtr g_shell;
// Share the application command-line arguments with multiple application tests.
Array<String> g_args;
-class ArgumentGrabber : public InterfaceImpl<Application> {
- public:
- ArgumentGrabber(Array<String>* args, ShellPtr shell)
- : args_(args), shell_(shell.Pass()) {
- shell_.set_client(this);
+// Application request handle passed from the shell in MojoMain, stored in
+// between SetUp()/TearDown() so we can (re-)intialize new ApplicationImpls.
+InterfaceRequest<Application> g_application_request;
+
+// Shell pointer passed in the initial mojo.Application.Initialize() call,
+// stored in between initial setup and the first test and between SetUp/TearDown
+// calls so we can (re-)initialize new ApplicationImpls.
+ShellPtr g_shell;
+
+void InitializeArgs(int argc, std::vector<const char*> argv) {
+ MOJO_CHECK(g_args.is_null());
+ for (const char* arg : argv) {
+ if (arg)
+ g_args.push_back(arg);
}
+}
+
+class ShellAndArgumentGrabber : public Application {
+ public:
+ ShellAndArgumentGrabber(Array<String>* args,
+ InterfaceRequest<Application> application_request)
+ : args_(args), binding_(this, application_request.Pass()) {}
void WaitForInitialize() {
// Initialize is always the first call made on Application.
- shell_.WaitForIncomingMethodCall();
- }
-
- ShellPtr UnbindShell() {
- ShellPtr unbound_shell;
- unbound_shell.Bind(shell_.PassMessagePipe());
- return unbound_shell.Pass();
+ binding_.WaitForIncomingMethodCall();
}
private:
// Application implementation.
- void Initialize(Array<String> args) override { *args_ = args.Pass(); }
+ void Initialize(ShellPtr shell, Array<String> args) override {
+ *args_ = args.Pass();
+ g_application_request = binding_.Unbind();
+ g_shell = shell.Pass();
+ }
void AcceptConnection(const String& requestor_url,
InterfaceRequest<ServiceProvider> services,
@@ -50,42 +62,31 @@
void RequestQuit() override { MOJO_CHECK(false); }
Array<String>* args_;
- ShellPtr shell_;
+ Binding<Application> binding_;
};
-ShellPtr PassShellHandle() {
- MOJO_CHECK(g_shell);
- return g_shell.Pass();
-}
-
-void SetShellHandle(ShellPtr shell) {
- MOJO_CHECK(shell);
- MOJO_CHECK(!g_shell);
- g_shell = shell.Pass();
-}
-
-void InitializeArgs(int argc, std::vector<const char*> argv) {
- MOJO_CHECK(g_args.is_null());
- for (const char* arg : argv) {
- if (arg)
- g_args.push_back(arg);
- }
-}
-
} // namespace
const Array<String>& Args() {
return g_args;
}
-MojoResult RunAllTests(ShellPtr shell) {
+MojoResult RunAllTests(MojoHandle application_request_handle) {
{
// This loop is used for init, and then destroyed before running tests.
Environment::InstantiateDefaultRunLoop();
+ // Grab the shell handle and GTEST commandline arguments.
+ // GTEST command line arguments are supported amid application arguments:
+ // $ mojo_shell mojo:example_apptests
+ // --args-for='mojo:example_apptests arg1 --gtest_filter=foo arg2'
Array<String> args;
- ArgumentGrabber grab(&args, shell.Pass());
- grab.WaitForInitialize();
+ ShellAndArgumentGrabber grabber(
+ &args, MakeRequest<Application>(MakeScopedHandle(
+ MessagePipeHandle(application_request_handle))));
+ grabber.WaitForInitialize();
+ MOJO_CHECK(g_shell);
+ MOJO_CHECK(g_application_request.is_pending());
// InitGoogleTest expects (argc + 1) elements, including a terminating null.
// It also removes GTEST arguments from |argv| and updates the |argc| count.
@@ -98,7 +99,6 @@
argv[argc] = nullptr;
testing::InitGoogleTest(&argc, const_cast<char**>(&(argv[0])));
- SetShellHandle(grab.UnbindShell());
InitializeArgs(argc, argv);
Environment::DestroyDefaultRunLoop();
@@ -106,8 +106,9 @@
int result = RUN_ALL_TESTS();
- shell = mojo::test::PassShellHandle();
- shell.reset();
+ // Shut down our message pipes before exiting.
+ (void)g_application_request.PassMessagePipe();
+ (void)g_shell.PassMessagePipe();
return (result == 0) ? MOJO_RESULT_OK : MOJO_RESULT_UNKNOWN;
}
@@ -122,26 +123,28 @@
return &default_application_delegate_;
}
-void ApplicationTestBase::SetUpWithArgs(const Array<String>& args) {
+void ApplicationTestBase::SetUp() {
// A run loop is recommended for ApplicationImpl initialization and
// communication.
if (ShouldCreateDefaultRunLoop())
Environment::InstantiateDefaultRunLoop();
+ MOJO_CHECK(g_application_request.is_pending());
+ MOJO_CHECK(g_shell);
+
// New applications are constructed for each test to avoid persisting state.
application_impl_ = new ApplicationImpl(GetApplicationDelegate(),
- PassShellHandle());
+ g_application_request.Pass());
// Fake application initialization with the given command line arguments.
- application_impl_->Initialize(args.Clone());
-}
-
-void ApplicationTestBase::SetUp() {
- SetUpWithArgs(Args());
+ application_impl_->Initialize(g_shell.Pass(), g_args.Clone());
}
void ApplicationTestBase::TearDown() {
- SetShellHandle(application_impl_->UnbindShell());
+ MOJO_CHECK(!g_application_request.is_pending());
+ MOJO_CHECK(!g_shell);
+
+ application_impl_->UnbindConnections(&g_application_request, &g_shell);
delete application_impl_;
if (ShouldCreateDefaultRunLoop())
Environment::DestroyDefaultRunLoop();
diff --git a/mojo/public/cpp/application/lib/application_test_main.cc b/mojo/public/cpp/application/lib/application_test_main.cc
index 3f96a47..128d8ae 100644
--- a/mojo/public/cpp/application/lib/application_test_main.cc
+++ b/mojo/public/cpp/application/lib/application_test_main.cc
@@ -5,13 +5,10 @@
#include "mojo/public/c/system/main.h"
#include "mojo/public/cpp/application/application_test_base.h"
#include "mojo/public/cpp/environment/environment.h"
-#include "mojo/public/interfaces/application/shell.mojom.h"
-MojoResult MojoMain(MojoHandle shell_handle) {
+MojoResult MojoMain(MojoHandle handle) {
// An Environment instance is needed to construct run loops.
mojo::Environment environment;
- mojo::ShellPtr shell;
- shell.Bind(MakeScopedHandle(mojo::MessagePipeHandle(shell_handle)));
- return mojo::test::RunAllTests(shell.Pass());
+ return mojo::test::RunAllTests(handle);
}
