diff --git a/mojo/edk/js/core.cc b/mojo/edk/js/core.cc index 5b766d4be94d88..232f0e3755ec37 100644 --- a/mojo/edk/js/core.cc +++ b/mojo/edk/js/core.cc @@ -57,8 +57,7 @@ gin::Dictionary CreateMessagePipe(const gin::Arguments& args) { options_value->IsUndefined()) { result = MojoCreateMessagePipe(NULL, &handle0, &handle1); } else if (options_value->IsObject()) { - gin::Dictionary options_dict(args.isolate(), - options_value->ToObject(args.isolate())); + gin::Dictionary options_dict(args.isolate(), options_value->ToObject()); MojoCreateMessagePipeOptions options; // For future struct_size, we can probably infer that from the presence of // properties in options_dict. For now, it's always 8. @@ -157,8 +156,7 @@ gin::Dictionary CreateDataPipe(const gin::Arguments& args) { options_value->IsUndefined()) { result = MojoCreateDataPipe(NULL, &producer_handle, &consumer_handle); } else if (options_value->IsObject()) { - gin::Dictionary options_dict(args.isolate(), - options_value->ToObject(args.isolate())); + gin::Dictionary options_dict(args.isolate(), options_value->ToObject()); MojoCreateDataPipeOptions options; // For future struct_size, we can probably infer that from the presence of // properties in options_dict. For now, it's always 16. diff --git a/mojo/edk/js/waiting_callback.cc b/mojo/edk/js/waiting_callback.cc index 2b0b07e360d7a4..d5c48c539e3460 100644 --- a/mojo/edk/js/waiting_callback.cc +++ b/mojo/edk/js/waiting_callback.cc @@ -4,6 +4,8 @@ #include "mojo/edk/js/waiting_callback.h" +#include "base/bind.h" +#include "base/message_loop/message_loop.h" #include "gin/per_context_data.h" #include "mojo/public/cpp/environment/environment.h" @@ -50,7 +52,7 @@ void WaitingCallback::Cancel() { WaitingCallback::WaitingCallback(v8::Isolate* isolate, v8::Handle callback, gin::Handle handle_wrapper) - : wait_id_(0), handle_wrapper_(handle_wrapper.get()) { + : wait_id_(0), handle_wrapper_(handle_wrapper.get()), weak_factory_(this) { handle_wrapper_->AddCloseObserver(this); v8::Handle context = isolate->GetCurrentContext(); runner_ = gin::PerContextData::From(context)->runner()->GetWeakPtr(); @@ -66,10 +68,21 @@ void WaitingCallback::CallOnHandleReady(void* closure, MojoResult result) { static_cast(closure)->OnHandleReady(result); } -void WaitingCallback::OnHandleReady(MojoResult result) { +void WaitingCallback::ClearWaitId() { wait_id_ = 0; handle_wrapper_->RemoveCloseObserver(this); - handle_wrapper_ = NULL; + handle_wrapper_ = nullptr; +} + +void WaitingCallback::OnHandleReady(MojoResult result) { + ClearWaitId(); + CallCallback(result); +} + +void WaitingCallback::CallCallback(MojoResult result) { + // ClearWaitId must already have been called. + DCHECK(!wait_id_); + DCHECK(!handle_wrapper_); if (!runner_) return; @@ -88,7 +101,14 @@ void WaitingCallback::OnHandleReady(MojoResult result) { void WaitingCallback::OnWillCloseHandle() { Environment::GetDefaultAsyncWaiter()->CancelWait(wait_id_); - OnHandleReady(MOJO_RESULT_INVALID_ARGUMENT); + + // This may be called from GC, so we can't execute Javascript now, call + // ClearWaitId explicitly, and CallCallback asynchronously. + ClearWaitId(); + base::MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(&WaitingCallback::CallCallback, weak_factory_.GetWeakPtr(), + MOJO_RESULT_INVALID_ARGUMENT)); } } // namespace js diff --git a/mojo/edk/js/waiting_callback.h b/mojo/edk/js/waiting_callback.h index 6b2ccc73dcadef..5a9dfaffa884cb 100644 --- a/mojo/edk/js/waiting_callback.h +++ b/mojo/edk/js/waiting_callback.h @@ -5,6 +5,7 @@ #ifndef MOJO_EDK_JS_WAITING_CALLBACK_H_ #define MOJO_EDK_JS_WAITING_CALLBACK_H_ +#include "base/memory/weak_ptr.h" #include "gin/handle.h" #include "gin/runner.h" #include "gin/wrappable.h" @@ -49,10 +50,14 @@ class WaitingCallback : public gin::Wrappable, // still in progress. void OnWillCloseHandle() override; + void ClearWaitId(); + void CallCallback(MojoResult result); + base::WeakPtr runner_; MojoAsyncWaitID wait_id_; HandleWrapper* handle_wrapper_; + base::WeakPtrFactory weak_factory_; DISALLOW_COPY_AND_ASSIGN(WaitingCallback); }; diff --git a/mojo/edk/mojo_edk.gyp b/mojo/edk/mojo_edk.gyp index 05bde042f76bc8..0959aac80368cd 100644 --- a/mojo/edk/mojo_edk.gyp +++ b/mojo/edk/mojo_edk.gyp @@ -54,6 +54,7 @@ 'system/channel.h', 'system/channel_endpoint.cc', 'system/channel_endpoint.h', + 'system/channel_endpoint_client.h', 'system/channel_endpoint_id.cc', 'system/channel_endpoint_id.h', 'system/channel_info.cc', diff --git a/mojo/edk/mojo_edk_tests.gyp b/mojo/edk/mojo_edk_tests.gyp index 943cad38e6cf83..b877967726e728 100644 --- a/mojo/edk/mojo_edk_tests.gyp +++ b/mojo/edk/mojo_edk_tests.gyp @@ -169,6 +169,7 @@ 'embedder/platform_channel_pair_posix_unittest.cc', 'embedder/simple_platform_shared_buffer_unittest.cc', 'system/channel_endpoint_id_unittest.cc', + 'system/channel_manager_unittest.cc', 'system/channel_unittest.cc', 'system/core_unittest.cc', 'system/core_test_base.cc', diff --git a/mojo/edk/system/BUILD.gn b/mojo/edk/system/BUILD.gn index d0bfda78875e95..b87c76ad490b9d 100644 --- a/mojo/edk/system/BUILD.gn +++ b/mojo/edk/system/BUILD.gn @@ -17,6 +17,7 @@ component("system") { "channel.h", "channel_endpoint.cc", "channel_endpoint.h", + "channel_endpoint_client.h", "channel_endpoint_id.cc", "channel_endpoint_id.h", "channel_info.cc", @@ -103,6 +104,7 @@ test("mojo_system_unittests") { sources = [ "../test/multiprocess_test_helper_unittest.cc", "channel_endpoint_id_unittest.cc", + "channel_manager_unittest.cc", "channel_unittest.cc", "core_test_base.cc", "core_test_base.h", diff --git a/mojo/edk/system/channel.cc b/mojo/edk/system/channel.cc index 2f2a2ab6702ed7..50d4b65b9b8131 100644 --- a/mojo/edk/system/channel.cc +++ b/mojo/edk/system/channel.cc @@ -75,7 +75,6 @@ void Channel::Shutdown() { it != to_destroy.end(); ++it) { if (it->second.get()) { num_live++; - it->second->OnDisconnect(); it->second->DetachFromChannel(); } else { num_zombies++; @@ -475,7 +474,6 @@ bool Channel::OnRemoveMessagePipeEndpoint(ChannelEndpointId local_id, static_cast(remote_id.value()))); } - endpoint->OnDisconnect(); return true; } diff --git a/mojo/edk/system/channel.h b/mojo/edk/system/channel.h index 715d1f2ce749ed..e0ecc3b7f6b39c 100644 --- a/mojo/edk/system/channel.h +++ b/mojo/edk/system/channel.h @@ -196,8 +196,15 @@ class MOJO_SYSTEM_IMPL_EXPORT Channel typedef base::hash_map> IdToMessagePipeMap; - // Map from local IDs to pending/incoming endpoints (i.e., those which do not - // yet have a dispatcher attached). + // Map from local IDs to pending/incoming message pipes (i.e., those which do + // not yet have a dispatcher attached). + // TODO(vtl): This is a layering violation, since |Channel| shouldn't know + // about |MessagePipe|. However, we can't just hang on to |ChannelEndpoint|s + // (even if they have a reference to the |MessagePipe|) since their lifetimes + // are tied to the "remote" side. When |ChannelEndpoint::DetachFromChannel()| + // (eventually) results in |ChannelEndpoint::DetachFromClient()| being called. + // We really need to hang on to the "local" side of the message pipe, to which + // dispatchers will be "attached". IdToMessagePipeMap incoming_message_pipes_; // TODO(vtl): We need to keep track of remote IDs (so that we don't collide // if/when we wrap). diff --git a/mojo/edk/system/channel_endpoint.cc b/mojo/edk/system/channel_endpoint.cc index dd7cd5b54eccba..ca86b5f92d55f8 100644 --- a/mojo/edk/system/channel_endpoint.cc +++ b/mojo/edk/system/channel_endpoint.cc @@ -6,18 +6,17 @@ #include "base/logging.h" #include "mojo/edk/system/channel.h" -#include "mojo/edk/system/message_pipe.h" +#include "mojo/edk/system/channel_endpoint_client.h" #include "mojo/edk/system/transport_data.h" namespace mojo { namespace system { -ChannelEndpoint::ChannelEndpoint(MessagePipe* message_pipe, - unsigned port, +ChannelEndpoint::ChannelEndpoint(ChannelEndpointClient* client, + unsigned client_port, MessageInTransitQueue* message_queue) - : message_pipe_(message_pipe), port_(port), channel_(nullptr) { - DCHECK(message_pipe_.get() || message_queue); - DCHECK(port_ == 0 || port_ == 1); + : client_(client), client_port_(client_port), channel_(nullptr) { + DCHECK(client_.get() || message_queue); if (message_queue) paused_message_queue_.Swap(message_queue); @@ -28,33 +27,29 @@ bool ChannelEndpoint::EnqueueMessage(scoped_ptr message) { base::AutoLock locker(lock_); - if (!channel_ || !remote_id_.is_valid()) { - // We may reach here if we haven't been attached or run yet. + if (!channel_) { + // We may reach here if we haven't been attached/run yet. // TODO(vtl): We may also reach here if the channel is shut down early for - // some reason (with live message pipes on it). We can't check |state_| yet, - // until it's protected under lock, but in this case we should return false - // (and not enqueue any messages). + // some reason (with live message pipes on it). Ideally, we'd return false + // (and not enqueue the message), but we currently don't have a way to check + // this. paused_message_queue_.AddMessage(message.Pass()); return true; } - // TODO(vtl): Currently, this only works in the "running" case. - DCHECK(remote_id_.is_valid()); - return WriteMessageNoLock(message.Pass()); } -void ChannelEndpoint::DetachFromMessagePipe() { +void ChannelEndpoint::DetachFromClient() { { base::AutoLock locker(lock_); - DCHECK(message_pipe_.get()); - message_pipe_ = nullptr; + DCHECK(client_.get()); + client_ = nullptr; if (!channel_) return; DCHECK(local_id_.is_valid()); - // TODO(vtl): Once we combine "run" into "attach", |remote_id_| should valid - // here as well. + DCHECK(remote_id_.is_valid()); channel_->DetachEndpoint(this, local_id_, remote_id_); channel_ = nullptr; local_id_ = ChannelEndpointId(); @@ -82,7 +77,7 @@ void ChannelEndpoint::AttachAndRun(Channel* channel, << "Failed to write enqueue message to channel"; } - if (!message_pipe_.get()) { + if (!client_.get()) { channel_->DetachEndpoint(this, local_id_, remote_id_); channel_ = nullptr; local_id_ = ChannelEndpointId(); @@ -94,12 +89,12 @@ bool ChannelEndpoint::OnReadMessage( const MessageInTransit::View& message_view, embedder::ScopedPlatformHandleVectorPtr platform_handles) { scoped_ptr message(new MessageInTransit(message_view)); - scoped_refptr message_pipe; - unsigned port; + scoped_refptr client; + unsigned client_port; { base::AutoLock locker(lock_); DCHECK(channel_); - if (!message_pipe_.get()) { + if (!client_.get()) { // This isn't a failure per se. (It just means that, e.g., the other end // of the message point closed first.) return true; @@ -113,49 +108,44 @@ bool ChannelEndpoint::OnReadMessage( channel_)); } - // Take a ref, and call |EnqueueMessage()| outside the lock. - message_pipe = message_pipe_; - port = port_; + // Take a ref, and call |OnReadMessage()| outside the lock. + client = client_; + client_port = client_port_; } - MojoResult result = message_pipe->EnqueueMessage( - MessagePipe::GetPeerPort(port), message.Pass()); - return (result == MOJO_RESULT_OK); + return client->OnReadMessage(client_port, message.Pass()); } -void ChannelEndpoint::OnDisconnect() { - scoped_refptr message_pipe; - unsigned port; +void ChannelEndpoint::DetachFromChannel() { + scoped_refptr client; + unsigned client_port = 0; { base::AutoLock locker(lock_); - if (!message_pipe_.get()) - return; - // Take a ref, and call |Close()| outside the lock. - message_pipe = message_pipe_; - port = port_; - } - message_pipe->Close(port); -} + if (client_.get()) { + // Take a ref, and call |OnDetachFromChannel()| outside the lock. + client = client_; + client_port = client_port_; + } -void ChannelEndpoint::DetachFromChannel() { - base::AutoLock locker(lock_); - // This may already be null if we already detached from the channel in - // |DetachFromMessagePipe()| by calling |Channel::DetachEndpoint()| (and there - // are racing detaches). - if (!channel_) - return; + // |channel_| may already be null if we already detached from the channel in + // |DetachFromClient()| by calling |Channel::DetachEndpoint()| (and there + // are racing detaches). + if (channel_) { + DCHECK(local_id_.is_valid()); + DCHECK(remote_id_.is_valid()); + channel_ = nullptr; + local_id_ = ChannelEndpointId(); + remote_id_ = ChannelEndpointId(); + } + } - DCHECK(local_id_.is_valid()); - // TODO(vtl): Once we combine "run" into "attach", |remote_id_| should valid - // here as well. - channel_ = nullptr; - local_id_ = ChannelEndpointId(); - remote_id_ = ChannelEndpointId(); + if (client.get()) + client->OnDetachFromChannel(client_port); } ChannelEndpoint::~ChannelEndpoint() { - DCHECK(!message_pipe_.get()); + DCHECK(!client_.get()); DCHECK(!channel_); DCHECK(!local_id_.is_valid()); DCHECK(!remote_id_.is_valid()); diff --git a/mojo/edk/system/channel_endpoint.h b/mojo/edk/system/channel_endpoint.h index 3c415ea7e6e534..8de169b223b180 100644 --- a/mojo/edk/system/channel_endpoint.h +++ b/mojo/edk/system/channel_endpoint.h @@ -11,6 +11,7 @@ #include "base/synchronization/lock.h" #include "mojo/edk/embedder/platform_handle_vector.h" #include "mojo/edk/system/channel_endpoint_id.h" +#include "mojo/edk/system/message_in_transit.h" #include "mojo/edk/system/message_in_transit_queue.h" #include "mojo/edk/system/system_impl_export.h" @@ -18,8 +19,8 @@ namespace mojo { namespace system { class Channel; +class ChannelEndpointClient; class MessageInTransit; -class MessagePipe; // TODO(vtl): The plan: // - (Done.) Move |Channel::Endpoint| to |ChannelEndpoint|. Make it @@ -112,47 +113,43 @@ class MessagePipe; class MOJO_SYSTEM_IMPL_EXPORT ChannelEndpoint : public base::RefCountedThreadSafe { public: - // Constructor for a |ChannelEndpoint| attached to the given message pipe - // endpoint (specified by |message_pipe| and |port|). Optionally takes - // messages from |*message_queue| if |message_queue| is non-null. + // Constructor for a |ChannelEndpoint| with the given client (specified by + // |client| and |client_port|). Optionally takes messages from + // |*message_queue| if |message_queue| is non-null. // - // |message_pipe| may be null if this endpoint will never need to receive - // messages, in which case |message_queue| should not be null. In that case, - // this endpoint will simply send queued messages upon being attached to a + // |client| may be null if this endpoint will never need to receive messages, + // in which case |message_queue| should not be null. In that case, this + // endpoint will simply send queued messages upon being attached to a // |Channel| and immediately detach itself. - ChannelEndpoint(MessagePipe* message_pipe, - unsigned port, + ChannelEndpoint(ChannelEndpointClient* client, + unsigned client_port, MessageInTransitQueue* message_queue = nullptr); - // Methods called by |MessagePipe| (via |ProxyMessagePipeEndpoint|): + // Methods called by |ChannelEndpointClient|: - // TODO(vtl): This currently only works if we're "running". We'll move the - // "paused message queue" here (will this be needed when we have - // locally-allocated remote IDs?). + // Called to enqueue an outbound message. (If |AttachAndRun()| has not yet + // been called, the message will be enqueued and sent when |AttachAndRun()| is + // called.) bool EnqueueMessage(scoped_ptr message); - void DetachFromMessagePipe(); + // Called before the |ChannelEndpointClient| gives up its reference to this + // object. + void DetachFromClient(); // Methods called by |Channel|: - // Called by |Channel| when it takes a reference to this object. It will send + // Called when the |Channel| takes a reference to this object. This will send // all queue messages (in |paused_message_queue_|). // TODO(vtl): Maybe rename this "OnAttach"? void AttachAndRun(Channel* channel, ChannelEndpointId local_id, ChannelEndpointId remote_id); - // Called by |Channel| when it receives a message for the message pipe. + // Called when the |Channel| receives a message for the |ChannelEndpoint|. bool OnReadMessage(const MessageInTransit::View& message_view, embedder::ScopedPlatformHandleVectorPtr platform_handles); - // Called by |Channel| to notify that it'll no longer receive messages for the - // message pipe (i.e., |OnReadMessage()| will no longer be called). - // TODO(vtl): After more simplification, we might be able to get rid of this - // (and merge it with |DetachFromChannel()|). - void OnDisconnect(); - - // Called by |Channel| before it gives up its reference to this object. + // Called before the |Channel| gives up its reference to this object. void DetachFromChannel(); private: @@ -165,21 +162,21 @@ class MOJO_SYSTEM_IMPL_EXPORT ChannelEndpoint // Protects the members below. base::Lock lock_; - // |message_pipe_| must be valid whenever it is non-null. Before - // |*message_pipe_| gives up its reference to this object, it must call - // |DetachFromMessagePipe()|. + // |client_| must be valid whenever it is non-null. Before |*client_| gives up + // its reference to this object, it must call |DetachFromClient()|. // NOTE: This is a |scoped_refptr<>|, rather than a raw pointer, since the // |Channel| needs to keep the |MessagePipe| alive for the "proxy-proxy" case. // Possibly we'll be able to eliminate that case when we have full // multiprocess support. - // WARNING: |MessagePipe| methods must not be called under |lock_|. Thus to - // make such a call, a reference must first be taken under |lock_| and the - // lock released. - scoped_refptr message_pipe_; - unsigned port_; + // WARNING: |ChannelEndpointClient| methods must not be called under |lock_|. + // Thus to make such a call, a reference must first be taken under |lock_| and + // the lock released. + scoped_refptr client_; + unsigned client_port_; // |channel_| must be valid whenever it is non-null. Before |*channel_| gives // up its reference to this object, it must call |DetachFromChannel()|. + // |local_id_| and |remote_id_| are valid if and only |channel_| is non-null. Channel* channel_; ChannelEndpointId local_id_; ChannelEndpointId remote_id_; diff --git a/mojo/edk/system/channel_endpoint_client.h b/mojo/edk/system/channel_endpoint_client.h new file mode 100644 index 00000000000000..e14326fcf4072e --- /dev/null +++ b/mojo/edk/system/channel_endpoint_client.h @@ -0,0 +1,62 @@ +// 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 MOJO_EDK_SYSTEM_CHANNEL_ENDPOINT_CLIENT_H_ +#define MOJO_EDK_SYSTEM_CHANNEL_ENDPOINT_CLIENT_H_ + +#include "base/macros.h" +#include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" +#include "mojo/edk/system/system_impl_export.h" + +namespace mojo { +namespace system { + +class MessageInTransit; + +// Interface for receivers of messages from |ChannelEndpoint| (hence from +// |Channel|). |port| is simply the value passed to |ChannelEndpoint| on +// construction, and provides a lightweight way for an object to be the client +// of multiple |ChannelEndpoint|s. (|MessagePipe| implements this interface, in +// which case |port| is the port number for the |ProxyMessagePipeEndpoint| +// corresdponding to the |ChannelEndpoint|.) +// +// Implementations of this class should be thread-safe. |ChannelEndpointClient| +// *precedes* |ChannelEndpoint| in the lock order, so |ChannelEndpoint| should +// never call into this class with its lock held. (Instead, it should take a +// reference under its lock, release its lock, and make any needed call(s).) +// +// Note: As a consequence of this, all the client methods may be called even +// after |ChannelEndpoint::DetachFromClient()| has been called (so the +// |ChannelEndpoint| has apparently relinquished its pointer to the +// |ChannelEndpointClient|). +class MOJO_SYSTEM_IMPL_EXPORT ChannelEndpointClient + : public base::RefCountedThreadSafe { + public: + // Called by |ChannelEndpoint| in response to its |OnReadMessage()|, which is + // called by |Channel| when it receives a message for the |ChannelEndpoint|. + // (|port| is the value passed to |ChannelEndpoint|'s constructor as + // |client_port|.) + virtual bool OnReadMessage(unsigned port, + scoped_ptr message) = 0; + + // Called by |ChannelEndpoint| when the |Channel| is relinquishing its pointer + // to the |ChannelEndpoint| (and vice versa). After this is called, + // |OnReadMessage()| will no longer be called. + virtual void OnDetachFromChannel(unsigned port) = 0; + + protected: + ChannelEndpointClient() {} + + virtual ~ChannelEndpointClient() {} + friend class base::RefCountedThreadSafe; + + private: + DISALLOW_COPY_AND_ASSIGN(ChannelEndpointClient); +}; + +} // namespace system +} // namespace mojo + +#endif // MOJO_EDK_SYSTEM_CHANNEL_ENDPOINT_CLIENT_H_ diff --git a/mojo/edk/system/channel_manager.h b/mojo/edk/system/channel_manager.h index 60a3048dc3d80a..bb6371bbb42e96 100644 --- a/mojo/edk/system/channel_manager.h +++ b/mojo/edk/system/channel_manager.h @@ -32,15 +32,6 @@ class MOJO_SYSTEM_IMPL_EXPORT ChannelManager { ChannelManager(); ~ChannelManager(); - // Gets the ID for a given channel. - // - // Note: This is currently a static method and thus may be called under - // |lock_|. If this is ever made non-static (i.e., made specific to a given - // |ChannelManager|), those call sites may have to changed. - static ChannelId GetChannelId(const Channel* channel) { - return reinterpret_cast(channel); - } - // Adds |channel| to the set of |Channel|s managed by this |ChannelManager|; // |channel_thread_task_runner| should be the task runner for |channel|'s // creation (a.k.a. I/O) thread. |channel| should either already be @@ -65,6 +56,15 @@ class MOJO_SYSTEM_IMPL_EXPORT ChannelManager { void ShutdownChannel(ChannelId channel_id); private: + // Gets the ID for a given channel. + // + // Note: This is currently a static method and thus may be called under + // |lock_|. If this is ever made non-static (i.e., made specific to a given + // |ChannelManager|), those call sites may have to changed. + static ChannelId GetChannelId(const Channel* channel) { + return reinterpret_cast(channel); + } + // Gets the |ChannelInfo| for the channel specified by the given ID. (This // should *not* be called under lock.) ChannelInfo GetChannelInfo(ChannelId channel_id); diff --git a/mojo/edk/system/channel_manager_unittest.cc b/mojo/edk/system/channel_manager_unittest.cc new file mode 100644 index 00000000000000..52f3f0ebfb03ab --- /dev/null +++ b/mojo/edk/system/channel_manager_unittest.cc @@ -0,0 +1,183 @@ +// 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/edk/system/channel_manager.h" + +#include "base/callback.h" +#include "base/macros.h" +#include "base/message_loop/message_loop.h" +#include "base/message_loop/message_loop_proxy.h" +#include "base/run_loop.h" +#include "base/task_runner.h" +#include "base/test/test_timeouts.h" +#include "base/threading/platform_thread.h" +#include "base/threading/simple_thread.h" +#include "base/time/time.h" +#include "mojo/edk/embedder/platform_channel_pair.h" +#include "mojo/edk/embedder/simple_platform_support.h" +#include "mojo/edk/system/channel.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace mojo { +namespace system { +namespace { + +class ChannelManagerTest : public testing::Test { + public: + ChannelManagerTest() : message_loop_(base::MessageLoop::TYPE_IO) {} + ~ChannelManagerTest() override {} + + protected: + embedder::SimplePlatformSupport* platform_support() { + return &platform_support_; + } + base::MessageLoop* message_loop() { return &message_loop_; } + + private: + embedder::SimplePlatformSupport platform_support_; + base::MessageLoop message_loop_; + + DISALLOW_COPY_AND_ASSIGN(ChannelManagerTest); +}; + +TEST_F(ChannelManagerTest, Basic) { + ChannelManager cm; + + // Hang on to a ref to the |Channel|, so that we can check that the + // |ChannelManager| takes/releases refs to it. + scoped_refptr ch(new Channel(platform_support())); + ASSERT_TRUE(ch->HasOneRef()); + + embedder::PlatformChannelPair channel_pair; + ch->Init(RawChannel::Create(channel_pair.PassServerHandle())); + + ChannelId id = cm.AddChannel(ch, base::MessageLoopProxy::current()); + EXPECT_NE(id, 0u); + // |ChannelManager| should take a ref. + EXPECT_FALSE(ch->HasOneRef()); + + cm.WillShutdownChannel(id); + // |ChannelManager| should still have a ref. + EXPECT_FALSE(ch->HasOneRef()); + + cm.ShutdownChannel(id); + // On the "I/O" thread, so shutdown should happen synchronously. + // |ChannelManager| should have given up its ref. + EXPECT_TRUE(ch->HasOneRef()); +} + +TEST_F(ChannelManagerTest, TwoChannels) { + ChannelManager cm; + + // Hang on to a ref to each |Channel|, so that we can check that the + // |ChannelManager| takes/releases refs to them. + scoped_refptr ch1(new Channel(platform_support())); + ASSERT_TRUE(ch1->HasOneRef()); + scoped_refptr ch2(new Channel(platform_support())); + ASSERT_TRUE(ch2->HasOneRef()); + + embedder::PlatformChannelPair channel_pair; + ch1->Init(RawChannel::Create(channel_pair.PassServerHandle())); + ch2->Init(RawChannel::Create(channel_pair.PassClientHandle())); + + ChannelId id1 = cm.AddChannel(ch1, base::MessageLoopProxy::current()); + EXPECT_NE(id1, 0u); + EXPECT_FALSE(ch1->HasOneRef()); + + ChannelId id2 = cm.AddChannel(ch2, base::MessageLoopProxy::current()); + EXPECT_NE(id2, 0u); + EXPECT_NE(id2, id1); + EXPECT_FALSE(ch2->HasOneRef()); + + // Calling |WillShutdownChannel()| multiple times (on |id1|) is okay. + cm.WillShutdownChannel(id1); + cm.WillShutdownChannel(id1); + EXPECT_FALSE(ch1->HasOneRef()); + // Not calling |WillShutdownChannel()| (on |id2|) is okay too. + + cm.ShutdownChannel(id1); + EXPECT_TRUE(ch1->HasOneRef()); + cm.ShutdownChannel(id2); + EXPECT_TRUE(ch2->HasOneRef()); +} + +class OtherThread : public base::SimpleThread { + public: + // Note: We rely on the main thread keeping *exactly one* reference to + // |channel|. + OtherThread(scoped_refptr task_runner, + ChannelManager* channel_manager, + Channel* channel, + base::Closure quit_closure) + : base::SimpleThread("other_thread"), + task_runner_(task_runner), + channel_manager_(channel_manager), + channel_(channel), + quit_closure_(quit_closure) {} + ~OtherThread() override {} + + private: + void Run() override { + // See comment above constructor. + ASSERT_TRUE(channel_->HasOneRef()); + + ChannelId id = channel_manager_->AddChannel(make_scoped_refptr(channel_), + task_runner_); + EXPECT_NE(id, 0u); + // |ChannelManager| should take a ref. + EXPECT_FALSE(channel_->HasOneRef()); + + channel_manager_->WillShutdownChannel(id); + // |ChannelManager| should still have a ref. + EXPECT_FALSE(channel_->HasOneRef()); + + channel_manager_->ShutdownChannel(id); + // This doesn't happen synchronously, so we "wait" until it does. + // TODO(vtl): Possibly |Channel| should provide some notification of being + // shut down. + base::TimeTicks start_time(base::TimeTicks::Now()); + for (;;) { + if (channel_->HasOneRef()) + break; + + // Check, instead of assert, since if things go wrong, dying is more + // reliable than tearing down. + CHECK_LT(base::TimeTicks::Now() - start_time, + TestTimeouts::action_timeout()); + base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(20)); + } + + CHECK(task_runner_->PostTask(FROM_HERE, quit_closure_)); + } + + scoped_refptr task_runner_; + ChannelManager* channel_manager_; + Channel* channel_; + base::Closure quit_closure_; + + DISALLOW_COPY_AND_ASSIGN(OtherThread); +}; + +TEST_F(ChannelManagerTest, CallsFromOtherThread) { + ChannelManager cm; + + // Hang on to a ref to the |Channel|, so that we can check that the + // |ChannelManager| takes/releases refs to it. + scoped_refptr ch(new Channel(platform_support())); + ASSERT_TRUE(ch->HasOneRef()); + + embedder::PlatformChannelPair channel_pair; + ch->Init(RawChannel::Create(channel_pair.PassServerHandle())); + + base::RunLoop run_loop; + OtherThread thread(base::MessageLoopProxy::current(), &cm, ch.get(), + run_loop.QuitClosure()); + thread.Start(); + run_loop.Run(); + thread.Join(); +} + +} // namespace +} // namespace system +} // namespace mojo diff --git a/mojo/edk/system/message_pipe.cc b/mojo/edk/system/message_pipe.cc index 731fd904e4fe86..8e1be57fe700fc 100644 --- a/mojo/edk/system/message_pipe.cc +++ b/mojo/edk/system/message_pipe.cc @@ -139,7 +139,7 @@ MojoResult MessagePipe::WriteMessage( std::vector* transports, MojoWriteMessageFlags flags) { DCHECK(port == 0 || port == 1); - return EnqueueMessageInternal( + return EnqueueMessage( GetPeerPort(port), make_scoped_ptr(new MessageInTransit( MessageInTransit::kTypeMessagePipeEndpoint, @@ -209,57 +209,96 @@ bool MessagePipe::EndSerialize( void* destination, size_t* actual_size, embedder::PlatformHandleVector* /*platform_handles*/) { + DCHECK(port == 0 || port == 1); + + scoped_refptr channel_endpoint; + { + base::AutoLock locker(lock_); + DCHECK(endpoints_[port]); + + // The port being serialized must be local. + DCHECK_EQ(endpoints_[port]->GetType(), MessagePipeEndpoint::kTypeLocal); + + // There are three possibilities for the peer port (below). In all cases, we + // pass the contents of |port|'s message queue to the channel, and it'll + // (presumably) make a |ChannelEndpoint| from it. + // + // 1. The peer port is (known to be) closed. + // + // There's no reason for us to continue to exist and no need for the + // channel to give us the |ChannelEndpoint|. It only remains for us to + // "close" |port|'s |LocalMessagePipeEndpoint| and prepare for + // destruction. + // + // 2. The peer port is local (the typical case). + // + // The channel gives us back a |ChannelEndpoint|, which we hook up to a + // |ProxyMessagePipeEndpoint| to replace |port|'s + // |LocalMessagePipeEndpoint|. We continue to exist, since the peer + // port's message pipe dispatcher will continue to hold a reference to + // us. + // + // 3. The peer port is remote. + // + // We also pass its |ChannelEndpoint| to the channel, which then decides + // what to do. We have no reason to continue to exist. + // + // TODO(vtl): Factor some of this out to |ChannelEndpoint|. + + if (!endpoints_[GetPeerPort(port)]) { + // Case 1. + channel_endpoint = new ChannelEndpoint( + nullptr, 0, static_cast( + endpoints_[port].get())->message_queue()); + endpoints_[port]->Close(); + endpoints_[port].reset(); + } else if (endpoints_[GetPeerPort(port)]->GetType() == + MessagePipeEndpoint::kTypeLocal) { + // Case 2. + channel_endpoint = new ChannelEndpoint( + this, port, static_cast( + endpoints_[port].get())->message_queue()); + endpoints_[port]->Close(); + endpoints_[port].reset( + new ProxyMessagePipeEndpoint(channel_endpoint.get())); + } else { + // Case 3. + // TODO(vtl): Temporarily the same as case 2. + DLOG(WARNING) << "Direct message pipe passing across multiple channels " + "not yet implemented; will proxy"; + channel_endpoint = new ChannelEndpoint( + this, port, static_cast( + endpoints_[port].get())->message_queue()); + endpoints_[port]->Close(); + endpoints_[port].reset( + new ProxyMessagePipeEndpoint(channel_endpoint.get())); + } + } + SerializedMessagePipe* s = static_cast(destination); // Convert the local endpoint to a proxy endpoint (moving the message queue) // and attach it to the channel. s->receiver_endpoint_id = - channel->AttachAndRunEndpoint(ConvertLocalToProxy(port), false); + channel->AttachAndRunEndpoint(channel_endpoint, false); DVLOG(2) << "Serializing message pipe (remote ID = " << s->receiver_endpoint_id << ")"; *actual_size = sizeof(SerializedMessagePipe); return true; } -scoped_refptr MessagePipe::ConvertLocalToProxy(unsigned port) { - DCHECK(port == 0 || port == 1); - - base::AutoLock locker(lock_); - DCHECK(endpoints_[port]); - DCHECK_EQ(endpoints_[port]->GetType(), MessagePipeEndpoint::kTypeLocal); - - // The local peer is already closed, so just make a |ChannelEndpoint| that'll - // send the already-queued messages. - if (!endpoints_[GetPeerPort(port)]) { - scoped_refptr channel_endpoint(new ChannelEndpoint( - nullptr, 0, static_cast( - endpoints_[port].get())->message_queue())); - endpoints_[port]->Close(); - endpoints_[port].reset(); - return channel_endpoint; - } - - // TODO(vtl): Allowing this case is a temporary hack. It'll set up a - // |MessagePipe| with two proxy endpoints, which will then act as a proxy - // (rather than trying to connect the two ends directly). - DLOG_IF(WARNING, endpoints_[GetPeerPort(port)]->GetType() != - MessagePipeEndpoint::kTypeLocal) - << "Direct message pipe passing across multiple channels not yet " - "implemented; will proxy"; - - scoped_ptr old_endpoint(endpoints_[port].Pass()); - scoped_refptr channel_endpoint(new ChannelEndpoint( - this, port, static_cast(old_endpoint.get()) - ->message_queue())); - endpoints_[port].reset(new ProxyMessagePipeEndpoint(channel_endpoint.get())); - old_endpoint->Close(); - - return channel_endpoint; +bool MessagePipe::OnReadMessage(unsigned port, + scoped_ptr message) { + // This is called when the |ChannelEndpoint| for the + // |ProxyMessagePipeEndpoint| |port| receives a message (from the |Channel|). + // We need to pass this message on to its peer port (typically a + // |LocalMessagePipeEndpoint|). + return EnqueueMessage(GetPeerPort(port), message.Pass(), nullptr) == + MOJO_RESULT_OK; } -MojoResult MessagePipe::EnqueueMessage(unsigned port, - scoped_ptr message) { - return EnqueueMessageInternal(port, message.Pass(), nullptr); +void MessagePipe::OnDetachFromChannel(unsigned port) { + Close(port); } MessagePipe::MessagePipe() { @@ -273,7 +312,7 @@ MessagePipe::~MessagePipe() { DCHECK(!endpoints_[1]); } -MojoResult MessagePipe::EnqueueMessageInternal( +MojoResult MessagePipe::EnqueueMessage( unsigned port, scoped_ptr message, std::vector* transports) { diff --git a/mojo/edk/system/message_pipe.h b/mojo/edk/system/message_pipe.h index b39e5170e7b147..1c1af1eb474a1c 100644 --- a/mojo/edk/system/message_pipe.h +++ b/mojo/edk/system/message_pipe.h @@ -15,6 +15,7 @@ #include "base/memory/scoped_ptr.h" #include "base/synchronization/lock.h" #include "mojo/edk/embedder/platform_handle_vector.h" +#include "mojo/edk/system/channel_endpoint_client.h" #include "mojo/edk/system/dispatcher.h" #include "mojo/edk/system/handle_signals_state.h" #include "mojo/edk/system/memory.h" @@ -34,8 +35,7 @@ class Waiter; // |MessagePipe| is the secondary object implementing a message pipe (see the // explanatory comment in core.cc). It is typically owned by the dispatcher(s) // corresponding to the local endpoints. This class is thread-safe. -class MOJO_SYSTEM_IMPL_EXPORT MessagePipe - : public base::RefCountedThreadSafe { +class MOJO_SYSTEM_IMPL_EXPORT MessagePipe : public ChannelEndpointClient { public: // Creates a |MessagePipe| with two new |LocalMessagePipeEndpoint|s. static MessagePipe* CreateLocalLocal(); @@ -106,38 +106,30 @@ class MOJO_SYSTEM_IMPL_EXPORT MessagePipe size_t* actual_size, embedder::PlatformHandleVector* platform_handles); - // Used by |EndSerialize()|. TODO(vtl): Remove this (merge it into - // |EndSerialize()|). - scoped_refptr ConvertLocalToProxy(unsigned port); - - // This is used by |Channel| to enqueue messages (typically to a - // |LocalMessagePipeEndpoint|). Unlike |WriteMessage()|, |port| is the - // *destination* port. - MojoResult EnqueueMessage(unsigned port, - scoped_ptr message); + // |ChannelEndpointClient| methods: + bool OnReadMessage(unsigned port, + scoped_ptr message) override; + void OnDetachFromChannel(unsigned port) override; private: MessagePipe(); + virtual ~MessagePipe(); - friend class base::RefCountedThreadSafe; - ~MessagePipe(); - - // This is used internally by |WriteMessage()| and by |EnqueueMessage()|. + // This is used internally by |WriteMessage()| and by |OnReadMessage()|. // |transports| may be non-null only if it's nonempty and |message| has no // dispatchers attached. - MojoResult EnqueueMessageInternal( - unsigned port, - scoped_ptr message, - std::vector* transports); + MojoResult EnqueueMessage(unsigned port, + scoped_ptr message, + std::vector* transports); - // Helper for |EnqueueMessageInternal()|. Must be called with |lock_| held. + // Helper for |EnqueueMessage()|. Must be called with |lock_| held. MojoResult AttachTransportsNoLock( unsigned port, MessageInTransit* message, std::vector* transports); - // Used by |EnqueueMessageInternal()| to handle control messages that are - // actually meant for us. + // Used by |EnqueueMessage()| to handle control messages that are actually + // meant for us. MojoResult HandleControlMessage(unsigned port, scoped_ptr message); diff --git a/mojo/edk/system/proxy_message_pipe_endpoint.cc b/mojo/edk/system/proxy_message_pipe_endpoint.cc index 85437bbad3f0e8..4a954d982bad83 100644 --- a/mojo/edk/system/proxy_message_pipe_endpoint.cc +++ b/mojo/edk/system/proxy_message_pipe_endpoint.cc @@ -48,7 +48,7 @@ void ProxyMessagePipeEndpoint::Close() { void ProxyMessagePipeEndpoint::DetachIfNecessary() { if (channel_endpoint_.get()) { - channel_endpoint_->DetachFromMessagePipe(); + channel_endpoint_->DetachFromClient(); channel_endpoint_ = nullptr; } } diff --git a/mojo/public/VERSION b/mojo/public/VERSION index 3a8f9d8e70ca81..282b862aff7d10 100644 --- a/mojo/public/VERSION +++ b/mojo/public/VERSION @@ -1 +1 @@ -e01f9a49449381a5eb430c1fd88bf2cae73ec35a \ No newline at end of file +5aa6dbdccf1950daf0cd3014bf763f35899bccf9 \ No newline at end of file diff --git a/mojo/public/cpp/bindings/binding.h b/mojo/public/cpp/bindings/binding.h index 2c4c9ceb05dc2e..72dbde30225342 100644 --- a/mojo/public/cpp/bindings/binding.h +++ b/mojo/public/cpp/bindings/binding.h @@ -34,7 +34,6 @@ namespace mojo { // private: // Binding binding_; // }; -// template class Binding : public ErrorHandler { public: @@ -106,6 +105,11 @@ class Binding : public ErrorHandler { return internal_router_->WaitForIncomingMessage(); } + void Close() { + MOJO_DCHECK(internal_router_); + internal_router_->CloseMessagePipe(); + } + void set_error_handler(ErrorHandler* error_handler) { error_handler_ = error_handler; } @@ -118,6 +122,9 @@ class Binding : public ErrorHandler { Interface* impl() { return impl_; } Client* client() { return proxy_; } + + bool is_bound() const { return !!internal_router_; } + // Exposed for testing, should not generally be used. internal::Router* internal_router() { return internal_router_; } diff --git a/mojo/public/cpp/bindings/lib/map_serialization.h b/mojo/public/cpp/bindings/lib/map_serialization.h index 4525a00d292e9b..fba816533f240f 100644 --- a/mojo/public/cpp/bindings/lib/map_serialization.h +++ b/mojo/public/cpp/bindings/lib/map_serialization.h @@ -52,10 +52,18 @@ struct MapSerializer, H, true> { static size_t GetItemSize(const H& item) { return 0; } }; +// This template must only apply to pointer mojo entity (structs and arrays). +// This is done by ensuring that WrapperTraits::DataType is a pointer. template -struct MapSerializer { +struct MapSerializer< + S, + typename EnableIf::DataType>::value, + typename WrapperTraits::DataType>::type, + true> { + typedef + typename RemovePointer::DataType>::type S_Data; static size_t GetBaseArraySize(size_t count) { - return count * sizeof(internal::StructPointer); + return count * sizeof(StructPointer); } static size_t GetItemSize(const S& item) { return GetSerializedSize_(item); } }; @@ -63,7 +71,7 @@ struct MapSerializer { template <> struct MapSerializer { static size_t GetBaseArraySize(size_t count) { - return count * sizeof(internal::StringPointer); + return count * sizeof(StringPointer); } static size_t GetItemSize(const String& item) { return GetSerializedSize_(item); diff --git a/mojo/public/cpp/bindings/strong_binding.h b/mojo/public/cpp/bindings/strong_binding.h index 106b282e46f755..73b43b3983d312 100644 --- a/mojo/public/cpp/bindings/strong_binding.h +++ b/mojo/public/cpp/bindings/strong_binding.h @@ -5,6 +5,8 @@ #ifndef MOJO_PUBLIC_CPP_BINDINGS_STRONG_BINDING_H_ #define MOJO_PUBLIC_CPP_BINDINGS_STRONG_BINDING_H_ +#include + #include "mojo/public/c/environment/async_waiter.h" #include "mojo/public/cpp/bindings/binding.h" #include "mojo/public/cpp/bindings/error_handler.h" @@ -25,8 +27,8 @@ namespace mojo { // // class StronglyBound : public Foo { // public: -// explicit StronglyBound(ScopedMessagePipeHandle handle) -// : binding_(this, handle.Pass()) {} +// explicit StronglyBound(InterfaceRequest request) +// : binding_(this, request.Pass()) {} // // // Foo implementation here // @@ -67,6 +69,27 @@ class StrongBinding : public ErrorHandler { ~StrongBinding() override {} + void Bind( + ScopedMessagePipeHandle handle, + const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) { + assert(!binding_.is_bound()); + binding_.Bind(handle.Pass(), waiter); + } + + void Bind( + InterfacePtr* ptr, + const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) { + assert(!binding_.is_bound()); + binding_.Bind(ptr, waiter); + } + + void Bind( + InterfaceRequest request, + const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) { + assert(!binding_.is_bound()); + binding_.Bind(request.Pass(), waiter); + } + bool WaitForIncomingMethodCall() { return binding_.WaitForIncomingMethodCall(); } diff --git a/mojo/public/cpp/bindings/tests/map_unittest.cc b/mojo/public/cpp/bindings/tests/map_unittest.cc index bb3c23d45c5441..698f407db77ea9 100644 --- a/mojo/public/cpp/bindings/tests/map_unittest.cc +++ b/mojo/public/cpp/bindings/tests/map_unittest.cc @@ -6,9 +6,9 @@ #include "mojo/public/cpp/bindings/lib/array_serialization.h" #include "mojo/public/cpp/bindings/lib/bindings_internal.h" #include "mojo/public/cpp/bindings/lib/fixed_buffer.h" +#include "mojo/public/cpp/bindings/lib/validate_params.h" #include "mojo/public/cpp/bindings/map.h" #include "mojo/public/cpp/bindings/string.h" -#include "mojo/public/cpp/bindings/struct_ptr.h" #include "mojo/public/cpp/bindings/tests/container_test_util.h" #include "mojo/public/cpp/environment/environment.h" #include "testing/gtest/include/gtest/gtest.h" @@ -18,6 +18,13 @@ namespace test { namespace { +using mojo::internal::Array_Data; +using mojo::internal::ArrayValidateParams; +using mojo::internal::FixedBuffer; +using mojo::internal::Map_Data; +using mojo::internal::NoValidateParams; +using mojo::internal::String_Data; + struct StringIntData { const char* string_data; int int_data; @@ -268,140 +275,52 @@ TEST_F(MapTest, MapArrayClone) { } } -// Data class for an end-to-end test of serialization. Because making a more -// limited test case tickles a clang compiler bug, we copy a minimal version of -// what our current cpp bindings do. -namespace internal { - -class ArrayOfMap_Data { - public: - static ArrayOfMap_Data* New(mojo::internal::Buffer* buf) { - return new (buf->Allocate(sizeof(ArrayOfMap_Data))) ArrayOfMap_Data(); - } - - mojo::internal::StructHeader header_; - - mojo::internal::ArrayPointer*> - first; - mojo::internal::ArrayPointer< - mojo::internal::Map_Data*>*> second; - - private: - ArrayOfMap_Data() { - header_.num_bytes = sizeof(*this); - header_.num_fields = 2; - } - ~ArrayOfMap_Data(); // NOT IMPLEMENTED -}; -static_assert(sizeof(ArrayOfMap_Data) == 24, "Bad sizeof(ArrayOfMap_Data)"); - -} // namespace internal - -class ArrayOfMapImpl; -typedef mojo::StructPtr ArrayOfMapImplPtr; - -class ArrayOfMapImpl { - public: - typedef internal::ArrayOfMap_Data Data_; - static ArrayOfMapImplPtr New() { - ArrayOfMapImplPtr rv; - mojo::internal::StructHelper::Initialize(&rv); - return rv.Pass(); - } - - mojo::Array> first; - mojo::Array>> second; -}; - -size_t GetSerializedSize_(const ArrayOfMapImplPtr& input) { - if (!input) - return 0; - size_t size = sizeof(internal::ArrayOfMap_Data); - size += GetSerializedSize_(input->first); - size += GetSerializedSize_(input->second); - return size; -} - -void Serialize_(ArrayOfMapImplPtr input, - mojo::internal::Buffer* buf, - internal::ArrayOfMap_Data** output) { - if (input) { - internal::ArrayOfMap_Data* result = internal::ArrayOfMap_Data::New(buf); - mojo::SerializeArray_>>( - mojo::internal::Forward(input->first), buf, &result->first.ptr); - MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING( - !result->first.ptr, - mojo::internal::VALIDATION_ERROR_UNEXPECTED_NULL_POINTER, - "null first field in ArrayOfMapImpl struct"); - mojo::SerializeArray_>>>( - mojo::internal::Forward(input->second), buf, &result->second.ptr); - MOJO_INTERNAL_DLOG_SERIALIZATION_WARNING( - !result->second.ptr, - mojo::internal::VALIDATION_ERROR_UNEXPECTED_NULL_POINTER, - "null second field in ArrayOfMapImpl struct"); - *output = result; - } else { - *output = nullptr; +TEST_F(MapTest, ArrayOfMap) { + { + Array> array(1); + array[0].insert(1, 42); + + size_t size = GetSerializedSize_(array); + FixedBuffer buf(size); + Array_Data*>* data; + SerializeArray_>>( + array.Pass(), &buf, &data); + + Array> deserialized_array; + Deserialize_(data, &deserialized_array); + + ASSERT_EQ(1u, deserialized_array.size()); + ASSERT_EQ(1u, deserialized_array[0].size()); + ASSERT_EQ(42, deserialized_array[0].at(1)); } -} -void Deserialize_(internal::ArrayOfMap_Data* input, ArrayOfMapImplPtr* output) { - if (input) { - ArrayOfMapImplPtr result(ArrayOfMapImpl::New()); - Deserialize_(input->first.ptr, &result->first); - Deserialize_(input->second.ptr, &result->second); - *output = result.Pass(); - } else { - output->reset(); + { + Array>> array(1); + Array map_value(2); + map_value[0] = false; + map_value[1] = true; + array[0].insert("hello world", map_value.Pass()); + + size_t size = GetSerializedSize_(array); + FixedBuffer buf(size); + Array_Data*>*>* data; + SerializeArray_>>>( + array.Pass(), &buf, &data); + + Array>> deserialized_array; + Deserialize_(data, &deserialized_array); + + ASSERT_EQ(1u, deserialized_array.size()); + ASSERT_EQ(1u, deserialized_array[0].size()); + ASSERT_FALSE(deserialized_array[0].at("hello world")[0]); + ASSERT_TRUE(deserialized_array[0].at("hello world")[1]); } } -TEST_F(MapTest, ArrayOfMap) { - Array> first_array(1); - first_array[0].insert(1, 42); - - Array>> second_array(1); - Array map_value(2); - map_value[0] = false; - map_value[1] = true; - second_array[0].insert("hello world", map_value.Pass()); - - ArrayOfMapImplPtr to_pass(ArrayOfMapImpl::New()); - to_pass->first = first_array.Pass(); - to_pass->second = second_array.Pass(); - - size_t size = GetSerializedSize_(to_pass); - mojo::internal::FixedBuffer buf(size); - internal::ArrayOfMap_Data* data; - Serialize_(mojo::internal::Forward(to_pass), &buf, &data); - - ArrayOfMapImplPtr to_receive(ArrayOfMapImpl::New()); - Deserialize_(data, &to_receive); - - ASSERT_EQ(1u, to_receive->first.size()); - ASSERT_EQ(1u, to_receive->first[0].size()); - ASSERT_EQ(42, to_receive->first[0].at(1)); - - ASSERT_EQ(1u, to_receive->second.size()); - ASSERT_EQ(1u, to_receive->second[0].size()); - ASSERT_FALSE(to_receive->second[0].at("hello world")[0]); - ASSERT_TRUE(to_receive->second[0].at("hello world")[1]); -} - } // namespace } // namespace test } // namespace mojo diff --git a/mojo/public/cpp/system/tests/macros_unittest.cc b/mojo/public/cpp/system/tests/macros_unittest.cc index f884baaf1866b4..89dd76435c9472 100644 --- a/mojo/public/cpp/system/tests/macros_unittest.cc +++ b/mojo/public/cpp/system/tests/macros_unittest.cc @@ -67,6 +67,8 @@ static_assert(MOJO_ARRAYSIZE(kGlobalArray) == 5u, TEST(MacrosCppTest, ArraySize) { double local_array[4] = {6.7, 7.8, 8.9, 9.0}; + // MSVS considers this local variable unused since MOJO_ARRAYSIZE only takes + // the size of the type of the local and not the values itself. MOJO_ALLOW_UNUSED_LOCAL(local_array); EXPECT_EQ(4u, MOJO_ARRAYSIZE(local_array)); } diff --git a/mojo/public/cpp/utility/lib/run_loop.cc b/mojo/public/cpp/utility/lib/run_loop.cc index 5e01a6344ac753..9707e7ecf76600 100644 --- a/mojo/public/cpp/utility/lib/run_loop.cc +++ b/mojo/public/cpp/utility/lib/run_loop.cc @@ -219,7 +219,7 @@ bool RunLoop::RemoveFirstInvalidHandle(const WaitState& wait_state) { handler->OnHandleError(wait_state.handles[i], result); return true; } - assert(MOJO_RESULT_DEADLINE_EXCEEDED == result); + assert(MOJO_RESULT_DEADLINE_EXCEEDED == result || MOJO_RESULT_OK == result); } return false; } diff --git a/mojo/public/dart/BUILD.gn b/mojo/public/dart/BUILD.gn index 89af889199be26..0acb310cb90810 100644 --- a/mojo/public/dart/BUILD.gn +++ b/mojo/public/dart/BUILD.gn @@ -54,10 +54,12 @@ copy("bindings") { "core.dart", "mojo_init.dart", "src/buffer.dart", + "src/client.dart", "src/codec.dart", "src/data_pipe.dart", "src/handle.dart", "src/handle_watcher.dart", + "src/interface.dart", "src/message_pipe.dart", "src/types.dart", ] diff --git a/mojo/public/dart/bindings.dart b/mojo/public/dart/bindings.dart index 6a3be5f458ddf8..1ae96971e336de 100644 --- a/mojo/public/dart/bindings.dart +++ b/mojo/public/dart/bindings.dart @@ -4,10 +4,13 @@ library bindings; +import 'core.dart' as core; import 'dart:async'; import 'dart:convert'; import 'dart:core'; import 'dart:mirrors'; import 'dart:typed_data'; +part 'src/client.dart'; part 'src/codec.dart'; +part 'src/interface.dart'; diff --git a/mojo/public/dart/mojo_init.dart b/mojo/public/dart/mojo_init.dart index 2b93af69fc57f1..2a1cd6d1785d8d 100644 --- a/mojo/public/dart/mojo_init.dart +++ b/mojo/public/dart/mojo_init.dart @@ -17,3 +17,7 @@ Future mojoInit() { _mojoSystemThunksMake(core.mojoSystemThunksSet); return core.MojoHandleWatcher.Start(); } + +void mojoShutdown() { + core.MojoHandleWatcher.Stop(); +} diff --git a/mojo/public/dart/src/client.dart b/mojo/public/dart/src/client.dart new file mode 100644 index 00000000000000..274fa478ac2b68 --- /dev/null +++ b/mojo/public/dart/src/client.dart @@ -0,0 +1,103 @@ +// 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. + +part of bindings; + +abstract class Client { + core.MojoMessagePipeEndpoint _endpoint; + core.MojoHandle _handle; + List _sendQueue; + List _completerQueue; + bool _isOpen = false; + + void handleResponse(MessageReader reader); + + Client(this._endpoint) { + _sendQueue = []; + _completerQueue = []; + _handle = new core.MojoHandle(_endpoint.handle); + } + + void open() { + _handle.listen((int mojoSignal) { + if (core.MojoHandleSignals.isReadable(mojoSignal)) { + // Query how many bytes are available. + var result = _endpoint.query(); + if (!result.status.isOk && !result.status.isResourceExhausted) { + // If something else happens, it means the handle wasn't really ready + // for reading, which indicates a bug in MojoHandle or the + // handle watcher. + throw new Exception("message pipe query failed: ${result.status}"); + } + + // Read the data. + var bytes = new ByteData(result.bytesRead); + var handles = new List(result.handlesRead); + result = _endpoint.read(bytes, result.bytesRead, handles); + if (!result.status.isOk && !result.status.isResourceExhausted) { + throw new Exception("message pipe read failed: ${result.status}"); + } + var message = new Message(bytes, handles); + var reader = new MessageReader(message); + + handleResponse(reader); + } + if (core.MojoHandleSignals.isWritable(mojoSignal)) { + if (_sendQueue.length > 0) { + List messageCompleter = _sendQueue.removeAt(0); + _endpoint.write(messageCompleter[0].buffer); + if (!_endpoint.status.isOk) { + throw new Exception("message pipe write failed"); + } + if (messageCompleter[1] != null) { + _completerQueue.add(messageCompleter[1]); + } + } + if ((_sendQueue.length == 0) && _handle.writeEnabled()) { + _handle.disableWriteEvents(); + } + } + if (core.MojoHandleSignals.isNone(mojoSignal)) { + // The handle watcher will send MojoHandleSignals.NONE if the other + // endpoint of the pipe is closed. + _handle.close(); + } + }); + _isOpen = true; + } + + void close() { + assert(isOpen); + _handle.close(); + _isOpen = false; + } + + void enqueueMessage(Type t, int name, Object msg) { + var builder = new MessageBuilder(name, align(getEncodedSize(t))); + builder.encodeStruct(t, msg); + var message = builder.finish(); + _sendQueue.add([message, null]); + if ((_sendQueue.length > 0) && !_handle.writeEnabled()) { + _handle.enableWriteEvents(); + } + } + + Future enqueueMessageWithRequestID(Type t, int name, int id, Object msg) { + var builder = new MessageWithRequestIDBuilder( + name, align(getEncodedSize(t)), id); + builder.encodeStruct(t, msg); + var message = builder.finish(); + + var completer = new Completer(); + _sendQueue.add([message, completer]); + if ((_sendQueue.length > 0) && !_handle.writeEnabled()) { + _handle.enableWriteEvents(); + } + return completer.future; + } + + // Need a getter for this for access in subclasses. + List get completerQueue => _completerQueue; + bool get isOpen => _isOpen; +} diff --git a/mojo/public/dart/src/codec.dart b/mojo/public/dart/src/codec.dart index 003fcc1d26aeed..286df12da49756 100644 --- a/mojo/public/dart/src/codec.dart +++ b/mojo/public/dart/src/codec.dart @@ -66,7 +66,7 @@ int getEncodedSize(Object typeOrInstance) { class MojoDecoder { ByteData buffer; - List handles; + List handles; int base; int next; @@ -151,7 +151,7 @@ class MojoDecoder { return new MojoDecoder(buffer, handles, offset); } - int decodeHandle() { + core.RawMojoHandle decodeHandle() { return handles[readUint32()]; } @@ -234,7 +234,7 @@ class MojoDecoder { class MojoEncoder { ByteData buffer; - List handles; + List handles; int base; int next; int extent; @@ -341,7 +341,7 @@ class MojoEncoder { return new MojoEncoder(buffer, handles, pointer, extent); } - void encodeHandle(int handle) { + void encodeHandle(core.RawMojoHandle handle) { handles.add(handle); writeUint32(handles.length - 1); } @@ -456,7 +456,7 @@ const int kMessageIsResponse = 1 << 1; class Message { ByteData buffer; - List handles; + List handles; Message(this.buffer, this.handles); @@ -475,12 +475,14 @@ class Message { class MessageBuilder { MojoEncoder encoder; - List handles; + List handles; + + MessageBuilder._(); MessageBuilder(int name, int payloadSize) { int numBytes = kMessageHeaderSize + payloadSize; var buffer = new ByteData(numBytes); - handles = []; + handles = []; encoder = new MojoEncoder(buffer, handles, 0, kMessageHeaderSize); encoder.writeUint32(kMessageHeaderSize); @@ -517,20 +519,19 @@ class MessageBuilder { class MessageWithRequestIDBuilder extends MessageBuilder { MessageWithRequestIDBuilder( - int name, int payloadSize, int flags, int requestID) { + int name, int payloadSize, int requestID, [int flags = 0]) + : super._() { int numBytes = kMessageWithRequestIDHeaderSize + payloadSize; - buffer = new ByteData(numBytes); - handles = []; - base = 0; + var buffer = new ByteData(numBytes); + handles = []; - encoder = createEncoder(0, kMessageWithRequestIDHeaderSize); + encoder = new MojoEncoder( + buffer, handles, 0, kMessageWithRequestIDHeaderSize); encoder.writeUint32(kMessageWithRequestIDHeaderSize); encoder.writeUint32(3); // num_fields. encoder.writeUint32(name); encoder.writeUint32(flags); encoder.writeUint64(requestID); - base = encoder.next; - buffer = encoder.buffer; } } @@ -564,7 +565,7 @@ class MessageReader { abstract class MojoType { static const int encodedSize = 0; - static T decode(MojoDecoder decoder) { return null } + static T decode(MojoDecoder decoder) { return null; } static void encode(MojoEncoder encoder, T val) {} } @@ -704,7 +705,7 @@ class PointerTo { class NullablePointerTo extends PointerTo { - static const int encodedSize = PointerTo.encodedSize; + NullablePointerTo(Object val) : super(val); } @@ -725,14 +726,14 @@ class ArrayOf { class NullableArrayOf extends ArrayOf { - static const int encodedSize = ArrayOf.encodedSize; + NullableArrayOf(Object val, [int length = 0]) : super(val, length); } -class Handle implements MojoType { +class Handle implements MojoType { static const int encodedSize = 4; - static int decode(MojoDecoder decoder) => decoder.decodeHandle(); - static void encode(MojoEncoder encoder, int val) { + static core.RawMojoHandle decode(MojoDecoder decoder) => decoder.decodeHandle(); + static void encode(MojoEncoder encoder, core.RawMojoHandle val) { encoder.encodeHandle(val); } } diff --git a/mojo/public/dart/src/handle.dart b/mojo/public/dart/src/handle.dart index bf9d87b74d632b..bf2f74e0efccb9 100644 --- a/mojo/public/dart/src/handle.dart +++ b/mojo/public/dart/src/handle.dart @@ -5,6 +5,7 @@ part of core; class _MojoHandleNatives { + static int register(MojoHandle handle) native "MojoHandle_Register"; static int close(int handle) native "MojoHandle_Close"; static int wait(int handle, int signals, int deadline) native "MojoHandle_Wait"; @@ -62,9 +63,17 @@ class RawMojoHandle { handles, signals, handles.length, deadline); } - static bool isValid(RawMojoHandle h) => (h.h != INVALID); + static MojoResult register(MojoHandle handle) { + return new MojoResult(_MojoHandleNatives.register(handle)); + } + + bool get isValid => (h != INVALID); String toString() => "$h"; + + bool operator ==(RawMojoHandle other) { + return h == other.h; + } } @@ -92,25 +101,24 @@ class MojoHandle extends Stream { MojoHandle(this._handle) : _signals = MojoHandleSignals.READABLE, - _eventHandlerAdded = false, - _receivePort = new ReceivePort() { - _sendPort = _receivePort.sendPort; - _controller = new StreamController(sync: true, - onListen: _onSubscriptionStateChange, - onCancel: _onSubscriptionStateChange, - onPause: _onPauseStateChange, - onResume: _onPauseStateChange); - _controller.addStream(_receivePort); + _eventHandlerAdded = false { + MojoResult result = RawMojoHandle.register(this); + if (!result.isOk) { + throw new Exception("Failed to register the MojoHandle"); + } } void close() { if (_eventHandlerAdded) { MojoHandleWatcher.close(_handle); + _eventHandlerAdded = false; } else { // If we're not in the handle watcher, then close the handle manually. _handle.close(); } - _receivePort.close(); + if (_receivePort != null) { + _receivePort.close(); + } } // We wrap the callback provided by clients in listen() with some code to @@ -125,11 +133,11 @@ class MojoHandle extends Stream { // The callback could have closed the handle. If so, don't add it back to // the MojoHandleWatcher. - if (RawMojoHandle.isValid(_handle)) { + if (_handle.isValid) { assert(!_eventHandlerAdded); var res = MojoHandleWatcher.add(_handle, _sendPort, _signals); if (!res.isOk) { - throw new Exception("Failed to re-add handle: $_handle"); + throw new Exception("Failed to re-add handle: $res"); } _eventHandlerAdded = true; } @@ -139,6 +147,14 @@ class MojoHandle extends Stream { StreamSubscription listen( void onData(int event), {Function onError, void onDone(), bool cancelOnError}) { + _receivePort = new ReceivePort(); + _sendPort = _receivePort.sendPort; + _controller = new StreamController(sync: true, + onListen: _onSubscriptionStateChange, + onCancel: _onSubscriptionStateChange, + onPause: _onPauseStateChange, + onResume: _onPauseStateChange); + _controller.addStream(_receivePort); assert(!_eventHandlerAdded); var res = MojoHandleWatcher.add(_handle, _sendPort, _signals); @@ -185,7 +201,10 @@ class MojoHandle extends Stream { void _onPauseStateChange() { if (_controller.isPaused) { if (_eventHandlerAdded) { - MojoHandleWatcher.remove(_handle); + var res = MojoHandleWatcher.remove(_handle); + if (!res.isOk) { + throw new Exception("MojoHandleWatcher add failed: $res"); + } _eventHandlerAdded = false; } } else { diff --git a/mojo/public/dart/src/handle_watcher.dart b/mojo/public/dart/src/handle_watcher.dart index 47f36756bba177..5f056549d8517d 100644 --- a/mojo/public/dart/src/handle_watcher.dart +++ b/mojo/public/dart/src/handle_watcher.dart @@ -103,12 +103,6 @@ class MojoHandleWatcher { watcher._routeEvent(res); // Remove the handle from the list. watcher._removeHandle(handle); - } else if (res == MojoResult.kFailedPrecondition) { - // None of the handles can ever be satisfied, including the control - // handle. This probably means we are going down. Clean up and - // shutdown. - watcher._pruneClosedHandles(); - watcher._shutdown = true; } else { // Some handle was closed, but not by us. // We have to go through the list and find it. @@ -159,7 +153,7 @@ class MojoHandleWatcher { _close(result[0]); break; case SHUTDOWN: - _shutdown = true; + _shutdownHandleWatcher(); break; default: throw new Exception("Invalid Command: $command"); @@ -204,7 +198,7 @@ class MojoHandleWatcher { } } - void _close(int mojoHandle) { + void _close(int mojoHandle, {bool pruning : false}) { int idx = _handleIndices[mojoHandle]; if (idx == null) { throw new Exception("Close on a non-existent handle: idx = $idx."); @@ -215,13 +209,19 @@ class MojoHandleWatcher { _tempHandle.h = _handles[idx]; _tempHandle.close(); _tempHandle.h = RawMojoHandle.INVALID; + if (pruning) { + // If this handle is being pruned, notify the application isolate + // by sending MojoHandleSignals.NONE. + _ports[idx].send(MojoHandleSignals.NONE); + } _removeHandle(mojoHandle); } void _toggleWrite(int mojoHandle) { int idx = _handleIndices[mojoHandle]; if (idx == null) { - throw new Exception("Toggle write on a non-existent handle: idx = $idx."); + throw new Exception( + "Toggle write on a non-existent handle: $mojoHandle."); } if (idx == 0) { throw new Exception("The control handle (idx = 0) cannot be toggled."); @@ -240,10 +240,17 @@ class MojoHandleWatcher { _tempHandle.h = RawMojoHandle.INVALID; } for (var h in closed) { - _close(h); + _close(h, pruning: true); } } + void _shutdownHandleWatcher() { + _shutdown = true; + _tempHandle.h = _controlHandle; + _tempHandle.close(); + _tempHandle.h = RawMojoHandle.INVALID; + } + static MojoResult _sendControlData(RawMojoHandle mojoHandle, SendPort port, int data) { @@ -251,14 +258,17 @@ class MojoHandleWatcher { if (controlHandle == RawMojoHandle.INVALID) { throw new Exception("Found invalid control handle"); } + + int rawHandle = RawMojoHandle.INVALID; + if (mojoHandle != null) { + rawHandle = mojoHandle.h; + } var result = _MojoHandleWatcherNatives.sendControlData( - controlHandle, mojoHandle.h, port, data); + controlHandle, rawHandle, port, data); return new MojoResult(result); } static Future Start() { - // 5. Return Future giving true on success. - // Make a control message pipe, MojoMessagePipe pipe = new MojoMessagePipe(); int consumerHandle = pipe.endpoints[0].handle.h; @@ -275,7 +285,13 @@ class MojoHandleWatcher { } static void Stop() { - _sendControlData(RawMojoHandle.INVALID, null, _encodeCommand(SHUTDOWN)); + // Send the shutdown command. + _sendControlData(null, null, _encodeCommand(SHUTDOWN)); + + // Close the control handle. + int controlHandle = _MojoHandleWatcherNatives.getControlHandle(); + var handle = new RawMojoHandle(controlHandle); + handle.close(); } static MojoResult close(RawMojoHandle mojoHandle) { diff --git a/mojo/public/dart/src/interface.dart b/mojo/public/dart/src/interface.dart new file mode 100644 index 00000000000000..298ab725bf9d27 --- /dev/null +++ b/mojo/public/dart/src/interface.dart @@ -0,0 +1,86 @@ +// 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. + +part of bindings; + +abstract class Interface { + core.MojoMessagePipeEndpoint _endpoint; + core.MojoHandle _handle; + List _sendQueue; + + Message handleMessage(MessageReader reader, Function messageHandler); + + Interface(this._endpoint) { + _sendQueue = []; + _handle = new core.MojoHandle(_endpoint.handle); + } + + StreamSubscription listen(Function messageHandler) { + return _handle.listen((int mojoSignal) { + if (core.MojoHandleSignals.isReadable(mojoSignal)) { + // Query how many bytes are available. + var result = _endpoint.query(); + if (!result.status.isOk && !result.status.isResourceExhausted) { + // If something else happens, it means the handle wasn't really ready + // for reading, which indicates a bug in MojoHandle or the + // event listener. + throw new Exception("message pipe query failed: ${result.status}"); + } + + // Read the data and view as a message. + var bytes = new ByteData(result.bytesRead); + var handles = new List(result.handlesRead); + result = _endpoint.read(bytes, result.bytesRead, handles); + if (!result.status.isOk && !result.status.isResourceExhausted) { + // If something else happens, it means the handle wasn't really ready + // for reading, which indicates a bug in MojoHandle or the + // event listener. + throw new Exception("message pipe read failed: ${result.status}"); + } + var message = new Message(bytes, handles); + var reader = new MessageReader(message); + + // Prepare the response. + var response_message = handleMessage(reader, messageHandler); + // If there's a response, queue it up for sending. + if (response_message != null) { + _sendQueue.add(response_message); + if ((_sendQueue.length > 0) && !_handle.writeEnabled()) { + _handle.enableWriteEvents(); + } + } + } + if (core.MojoHandleSignals.isWritable(mojoSignal)) { + if (_sendQueue.length > 0) { + var response_message = _sendQueue.removeAt(0); + _endpoint.write(response_message.buffer); + if (!_endpoint.status.isOk) { + throw new Exception("message pipe write failed"); + } + } + if ((_sendQueue.length == 0) && _handle.writeEnabled()) { + _handle.disableWriteEvents(); + } + } + if (core.MojoHandleSignals.isNone(mojoSignal)) { + // The handle watcher will send MojoHandleSignals.NONE when the other + // endpoint of the pipe is closed. + _handle.close(); + } + }); + } + + Message buildResponse(Type t, int name, Object response) { + var builder = new MessageBuilder(name, align(getEncodedSize(t))); + builder.encodeStruct(t, response); + return builder.finish(); + } + + Message buildResponseWithID(Type t, int name, int id, Object response) { + var builder = new MessageWithRequestIDBuilder( + name, align(getEncodedSize(t)), id); + builder.encodeStruct(t, response); + return builder.finish(); + } +} diff --git a/mojo/public/dart/src/mojo_dart_core.cc b/mojo/public/dart/src/mojo_dart_core.cc index 12e5858732bdeb..27b2e3fbe7a535 100644 --- a/mojo/public/dart/src/mojo_dart_core.cc +++ b/mojo/public/dart/src/mojo_dart_core.cc @@ -76,6 +76,73 @@ static void MojoSystemThunks_Set(Dart_NativeArguments arguments) { } +struct CloserCallbackPeer { + MojoHandle handle; +}; + + +static void MojoHandleCloserCallback(void* isolate_data, + Dart_WeakPersistentHandle handle, + void* peer) { + CloserCallbackPeer* callback_peer = + reinterpret_cast(peer); + if (callback_peer->handle != MOJO_HANDLE_INVALID) { + MojoClose(callback_peer->handle); + } + delete callback_peer; +} + + +// Setup a weak persistent handle for a Dart MojoHandle that calls MojoClose +// on the handle when the MojoHandle is GC'd or the VM is going down. +static void MojoHandle_Register(Dart_NativeArguments arguments) { + // An instance of Dart class MojoHandle. + Dart_Handle mojo_handle_instance = Dart_GetNativeArgument(arguments, 0); + if (!Dart_IsInstance(mojo_handle_instance)) { + SetInvalidArgumentReturn(arguments); + return; + } + // TODO(zra): Here, we could check that mojo_handle_instance is really a + // MojoHandle instance, but with the Dart API it's not too easy to get a Type + // object from the class name outside of the root library. For now, we'll rely + // on the existence of the right fields to be sufficient. + + Dart_Handle raw_mojo_handle_instance = Dart_GetField( + mojo_handle_instance, Dart_NewStringFromCString("_handle")); + if (Dart_IsError(raw_mojo_handle_instance)) { + SetInvalidArgumentReturn(arguments); + return; + } + + Dart_Handle mojo_handle = Dart_GetField( + raw_mojo_handle_instance, Dart_NewStringFromCString("h")); + if (Dart_IsError(mojo_handle)) { + SetInvalidArgumentReturn(arguments); + return; + } + + int64_t raw_handle = static_cast(MOJO_HANDLE_INVALID); + Dart_Handle result = Dart_IntegerToInt64(mojo_handle, &raw_handle); + if (Dart_IsError(result)) { + SetInvalidArgumentReturn(arguments); + return; + } + + if (raw_handle == static_cast(MOJO_HANDLE_INVALID)) { + SetInvalidArgumentReturn(arguments); + return; + } + + CloserCallbackPeer* callback_peer = new CloserCallbackPeer(); + callback_peer->handle = static_cast(raw_handle); + Dart_NewWeakPersistentHandle(mojo_handle_instance, + reinterpret_cast(callback_peer), + sizeof(CloserCallbackPeer), + MojoHandleCloserCallback); + Dart_SetIntegerReturnValue(arguments, static_cast(MOJO_RESULT_OK)); +} + + static void MojoHandle_Close(Dart_NativeArguments arguments) { int64_t handle; CHECK_INTEGER_ARGUMENT(arguments, 0, &handle, InvalidArgument); @@ -613,8 +680,7 @@ static void MojoHandleWatcher_SendControlData(Dart_NativeArguments arguments) { cd.data = data; const void* bytes = reinterpret_cast(&cd); MojoResult res = MojoWriteMessage( - control_handle, bytes, sizeof(cd), NULL, 0, 0); - + control_handle, bytes, sizeof(cd), NULL, 0, 0); Dart_SetIntegerReturnValue(arguments, static_cast(res)); } @@ -647,7 +713,7 @@ static void MojoHandleWatcher_SetControlHandle(Dart_NativeArguments arguments) { int64_t control_handle; CHECK_INTEGER_ARGUMENT(arguments, 0, &control_handle, InvalidArgument); - if (mojo_control_handle == MOJO_HANDLE_INVALID) { + if (mojo_control_handle == static_cast(MOJO_HANDLE_INVALID)) { mojo_control_handle = control_handle; } @@ -674,6 +740,7 @@ static void MojoHandleWatcher_GetControlHandle(Dart_NativeArguments arguments) { V(MojoMessagePipe_Create) \ V(MojoMessagePipe_Write) \ V(MojoMessagePipe_Read) \ + V(MojoHandle_Register) \ V(MojoHandle_WaitMany) \ V(MojoHandleWatcher_SendControlData) \ V(MojoHandleWatcher_RecvControlData) \ diff --git a/mojo/public/dart/src/types.dart b/mojo/public/dart/src/types.dart index 658e477bedfe4f..4353d74b5ce616 100644 --- a/mojo/public/dart/src/types.dart +++ b/mojo/public/dart/src/types.dart @@ -91,6 +91,30 @@ class MojoResult { bool get isDataLoss => (this == DATA_LOSS); bool get isBusy => (this == BUSY); bool get isShouldWait => (this == SHOULD_WAIT); + + String toString() { + switch (value) { + case kOk: return "OK"; + case kCancelled: return "CANCELLED"; + case kUnknown: return "UNKNOWN"; + case kInvalidArgument: return "INVALID_ARGUMENT"; + case kDeadlineExceeded: return "DEADLINE_EXCEEDED"; + case kNotFound: return "NOT_FOUND"; + case kAlreadyExists: return "ALREADY_EXISTS"; + case kPermissionDenied: return "PERMISSION_DENIED"; + case kResourceExhausted: return "RESOURCE_EXHAUSTED"; + case kFailedPrecondition: return "FAILED_PRECONDITION"; + case kAborted: return "ABORTED"; + case kOutOfRange: return "OUT_OF_RANGE"; + case kUnimplemented: return "UNIMPLEMENTED"; + case kInternal: return "INTERNAL"; + case kUnavailable: return "UNAVAILABLE"; + case kDataLoss: return "DATA_LOSS"; + case kBusy: return "BUSY"; + case kShouldWait: return "SHOULD_WAIT"; + default: return ""; + } + } } @@ -100,6 +124,7 @@ class MojoHandleSignals { static const int WRITABLE = 1 << 1; static const int READWRITE = READABLE | WRITABLE; + static bool isNone(int mask) => mask == NONE; static bool isReadable(int mask) => (mask & READABLE) == READABLE; static bool isWritable(int mask) => (mask & WRITABLE) == WRITABLE; static bool isReadWrite(int mask) => (mask & READWRITE) == READWRITE; diff --git a/mojo/public/interfaces/bindings/tests/test_structs.mojom b/mojo/public/interfaces/bindings/tests/test_structs.mojom index b525a2f8f28d3d..87ada0755d7604 100644 --- a/mojo/public/interfaces/bindings/tests/test_structs.mojom +++ b/mojo/public/interfaces/bindings/tests/test_structs.mojom @@ -136,4 +136,9 @@ struct MapValueTypes { map> f3; map?>> f4; map, 1>> f5; + map f6; + // TODO(hansmuller, yzshen): Uncomment these once the JavaScript bindings + // generator is fixed. + // map> f7; + // map>> f8; }; diff --git a/mojo/public/js/connection.js b/mojo/public/js/connection.js index d9b64aaae5276e..748ca5c30b0598 100644 --- a/mojo/public/js/connection.js +++ b/mojo/public/js/connection.js @@ -4,21 +4,22 @@ define("mojo/public/js/connection", [ "mojo/public/js/connector", + "mojo/public/js/core", "mojo/public/js/router", -], function(connector, router) { +], function(connector, core, routerModule) { - function Connection( - handle, localFactory, remoteFactory, routerFactory, connectorFactory) { - if (routerFactory === undefined) - routerFactory = router.Router; - this.router_ = new routerFactory(handle, connectorFactory); - this.remote = remoteFactory && new remoteFactory(this.router_); - this.local = localFactory && new localFactory(this.remote); - this.router_.setIncomingReceiver(this.local); + function BaseConnection(localStub, remoteProxy, router) { + this.router_ = router; + this.local = localStub; + this.remote = remoteProxy; + + this.router_.setIncomingReceiver(localStub); + if (this.remote) + this.remote.receiver_ = router; // Validate incoming messages: remote responses and local requests. - var validateRequest = localFactory && localFactory.prototype.validator; - var validateResponse = remoteFactory.prototype.validator; + var validateRequest = localStub && localStub.validator; + var validateResponse = remoteProxy && remoteProxy.validator; var payloadValidators = []; if (validateRequest) payloadValidators.push(validateRequest); @@ -27,17 +28,28 @@ define("mojo/public/js/connection", [ this.router_.setPayloadValidators(payloadValidators); } - Connection.prototype.close = function() { + BaseConnection.prototype.close = function() { this.router_.close(); this.router_ = null; this.local = null; this.remote = null; }; - Connection.prototype.encounteredError = function() { + BaseConnection.prototype.encounteredError = function() { return this.router_.encounteredError(); }; + function Connection( + handle, localFactory, remoteFactory, routerFactory, connectorFactory) { + var routerClass = routerFactory || routerModule.Router; + var router = new routerClass(handle, connectorFactory); + var remoteProxy = remoteFactory && new remoteFactory(router); + var localStub = localFactory && new localFactory(remoteProxy); + BaseConnection.call(this, localStub, remoteProxy, router); + } + + Connection.prototype = Object.create(BaseConnection.prototype); + // The TestConnection subclass is only intended to be used in unit tests. function TestConnection(handle, localFactory, remoteFactory) { @@ -45,14 +57,42 @@ define("mojo/public/js/connection", [ handle, localFactory, remoteFactory, - router.TestRouter, + routerModule.TestRouter, connector.TestConnector); } TestConnection.prototype = Object.create(Connection.prototype); + function createOpenConnection(stub, proxy) { + var messagePipe = core.createMessagePipe(); + // TODO(hansmuller): Check messagePipe.result. + var router = new routerModule.Router(messagePipe.handle0); + var connection = new BaseConnection(stub, proxy, router); + connection.messagePipeHandle = messagePipe.handle1; + return connection; + } + + function getProxyConnection(proxy, proxyInterface) { + if (!proxy.connection_) { + var stub = proxyInterface.client && new proxyInterface.client.stubClass; + proxy.connection_ = createOpenConnection(stub, proxy); + } + return proxy.connection_; + } + + function getStubConnection(stub, proxyInterface) { + if (!stub.connection_) { + var proxy = proxyInterface.client && new proxyInterface.client.proxyClass; + stub.connection_ = createOpenConnection(stub, proxy); + } + return stub.connection_; + } + + var exports = {}; exports.Connection = Connection; exports.TestConnection = TestConnection; + exports.getProxyConnection = getProxyConnection; + exports.getStubConnection = getStubConnection; return exports; }); diff --git a/mojo/public/js/struct_unittests.js b/mojo/public/js/struct_unittests.js index 092d2399779069..c72f8eb4e0547d 100644 --- a/mojo/public/js/struct_unittests.js +++ b/mojo/public/js/struct_unittests.js @@ -143,12 +143,24 @@ define([ function testMapValueTypes() { var mapFieldsStruct = new testStructs.MapValueTypes({ - f0: new Map([["a", ["b", "c"]], ["d", ["e"]]]), // array> - f1: new Map([["a", null], ["b", ["c", "d"]]]), // array?> - f2: new Map([["a", [null]], ["b", [null, "d"]]]), // array> - f3: new Map([["a", ["1", "2"]], ["b", ["1", "2"]]]), // array> - f4: new Map([["a", [["1"]]], ["b", [null]]]), // array?> - f5: new Map([["a", [["1", "2"]]]]), // array, 1>> + // array> + f0: new Map([["a", ["b", "c"]], ["d", ["e"]]]), + // array?> + f1: new Map([["a", null], ["b", ["c", "d"]]]), + // array> + f2: new Map([["a", [null]], ["b", [null, "d"]]]), + // array> + f3: new Map([["a", ["1", "2"]], ["b", ["1", "2"]]]), + // array?> + f4: new Map([["a", [["1"]]], ["b", [null]]]), + // array, 1>> + f5: new Map([["a", [["1", "2"]]]]), + // map + f6: new Map([["a", null]]), + // map> + // f7: new Map([["a", new Map([["b", "c"]])]]), + // map>> + // f8: new Map([["a", [new Map([["b", "c"]])]]]), }); var decodedStruct = structEncodeDecode(mapFieldsStruct); expect(decodedStruct.f0).toEqual(mapFieldsStruct.f0); @@ -157,6 +169,9 @@ define([ expect(decodedStruct.f3).toEqual(mapFieldsStruct.f3); expect(decodedStruct.f4).toEqual(mapFieldsStruct.f4); expect(decodedStruct.f5).toEqual(mapFieldsStruct.f5); + expect(decodedStruct.f6).toEqual(mapFieldsStruct.f6); + // expect(decodedStruct.f7).toEqual(mapFieldsStruct.f7); + // expect(decodedStruct.f8).toEqual(mapFieldsStruct.f8); } testConstructors(); diff --git a/mojo/public/mojo_application.gni b/mojo/public/mojo_application.gni index b14adbfba8c6d2..83b96a266971e3 100644 --- a/mojo/public/mojo_application.gni +++ b/mojo/public/mojo_application.gni @@ -88,7 +88,7 @@ template("mojo_native_application") { check_includes = invoker.check_includes } if (defined(invoker.configs)) { - configs = invoker.configs + configs += invoker.configs } if (defined(invoker.data)) { data = invoker.data diff --git a/mojo/public/python/mojo/bindings/reflection.py b/mojo/public/python/mojo/bindings/reflection.py index 3193f6e37d89a7..5ca38bfa2410bd 100644 --- a/mojo/public/python/mojo/bindings/reflection.py +++ b/mojo/public/python/mojo/bindings/reflection.py @@ -42,11 +42,11 @@ def __new__(mcs, name, bases, dictionary): raise ValueError('incorrect value: %r' % value) return type.__new__(mcs, name, bases, dictionary) - def __setattr__(mcs, key, value): - raise AttributeError, 'can\'t set attribute' + def __setattr__(cls, key, value): + raise AttributeError('can\'t set attribute') - def __delattr__(mcs, key): - raise AttributeError, 'can\'t delete attribute' + def __delattr__(cls, key): + raise AttributeError('can\'t delete attribute') class MojoStructType(type): @@ -131,12 +131,12 @@ def Deserialize(cls, data, handles): return type.__new__(mcs, name, bases, dictionary) # Prevent adding new attributes, or mutating constants. - def __setattr__(mcs, key, value): - raise AttributeError, 'can\'t set attribute' + def __setattr__(cls, key, value): + raise AttributeError('can\'t set attribute') # Prevent deleting constants. - def __delattr__(mcs, key): - raise AttributeError, 'can\'t delete attribute' + def __delattr__(cls, key): + raise AttributeError('can\'t delete attribute') class MojoInterfaceType(type): @@ -195,16 +195,16 @@ def __new__(mcs, name, bases, dictionary): return interface_class @property - def manager(mcs): - return mcs._interface_manager + def manager(cls): + return cls._interface_manager # Prevent adding new attributes, or mutating constants. - def __setattr__(mcs, key, value): - raise AttributeError, 'can\'t set attribute' + def __setattr__(cls, key, value): + raise AttributeError('can\'t set attribute') # Prevent deleting constants. - def __delattr__(mcs, key): - raise AttributeError, 'can\'t delete attribute' + def __delattr__(cls, key): + raise AttributeError('can\'t delete attribute') class InterfaceProxy(object): diff --git a/mojo/public/sky/convert_amd_modules_to_sky.py b/mojo/public/sky/convert_amd_modules_to_sky.py index 871278432d2e52..4a7ed8d611a535 100755 --- a/mojo/public/sky/convert_amd_modules_to_sky.py +++ b/mojo/public/sky/convert_amd_modules_to_sky.py @@ -70,15 +70,15 @@ def Parse(amd_module): AddImportNames(module, m.group(1)) state = "body" continue - raise Exception, "Unknown import declaration:" + line + raise Exception("Unknown import declaration:" + line) if state == "body": if end_body_regexp.search(line): module.body = "\n".join(body_lines) return module body_lines.append(line) continue - raise Exception, "Unknown parser state" - raise Exception, "End of file reached with finding a module" + raise Exception("Unknown parser state") + raise Exception("End of file reached with finding a module") def main(): parser = argparse.ArgumentParser() diff --git a/mojo/public/tools/bindings/generators/dart_templates/enum_definition.tmpl b/mojo/public/tools/bindings/generators/dart_templates/enum_definition.tmpl new file mode 100644 index 00000000000000..bfab117866b916 --- /dev/null +++ b/mojo/public/tools/bindings/generators/dart_templates/enum_definition.tmpl @@ -0,0 +1,12 @@ +{%- macro enum_def(prefix, enum) -%} +{%- set prev_enum = 0 %} +{%- for field in enum.fields %} +{%- if field.value %} +{{prefix}}final int {{enum.name}}_{{field.name}} = {{field.value|expression_to_text}}; +{%- elif loop.first %} +{{prefix}}final int {{enum.name}}_{{field.name}} = 0; +{%- else %} +{{prefix}}final int {{enum.name}}_{{field.name}} = {{enum.fields[loop.index0 - 1].name}} + 1; +{%- endif %} +{%- endfor %} +{%- endmacro %} diff --git a/mojo/public/tools/bindings/generators/dart_templates/interface_definition.tmpl b/mojo/public/tools/bindings/generators/dart_templates/interface_definition.tmpl new file mode 100644 index 00000000000000..ddc1de801e1789 --- /dev/null +++ b/mojo/public/tools/bindings/generators/dart_templates/interface_definition.tmpl @@ -0,0 +1,107 @@ +{%- for method in interface.methods %} +const int k{{interface.name}}_{{method.name}}_name = {{method.ordinal}}; +{%- endfor %} + + +class {{interface.name}}Client extends bindings.Client { + {{interface.name}}Client(core.MojoMessagePipeEndpoint endpoint) : super(endpoint); + +{%- for method in interface.methods %} +{%- if method.response_parameters == None %} + void {{method.name|stylize_method}}( +{%- for parameter in method.parameters -%} + {{parameter.kind|dart_decl_type}} {{parameter.name}}{% if not loop.last %}, {% endif %} +{%- endfor -%} + ) { + assert(isOpen); + var params = new {{interface.name}}_{{method.name}}_Params(); +{%- for parameter in method.parameters %} + params.{{parameter.name}} = {{parameter.name}}; +{%- endfor %} + enqueueMessage({{interface.name}}_{{method.name}}_Params, + k{{interface.name}}_{{method.name}}_name, + params); + } +{% else %} + Future<{{interface.name}}_{{method.name}}_ResponseParams> {{method.name|stylize_method}}( +{%- for parameter in method.parameters -%} + {{parameter.kind|dart_decl_type}} {{parameter.name}}{% if not loop.last %}, {% endif %} +{%- endfor -%} + ) { + assert(isOpen); + var params = new {{interface.name}}_{{method.name}}_Params(); +{%- for parameter in method.parameters %} + params.{{parameter.name}} = {{parameter.name}}; +{%- endfor %} + return enqueueMessageWithRequestID( + {{interface.name}}_{{method.name}}_Params, + k{{interface.name}}_{{method.name}}_name, + bindings.kMessageExpectsResponse, + params); + } +{%- endif %} +{%- endfor %} + + void handleResponse(bindings.MessageReader reader) { + switch (reader.name) { +{%- for method in interface.methods %} +{%- if method.response_parameters != None %} + case k{{interface.name}}_{{method.name}}_name: + var r = reader.decodeStruct({{interface.name}}_{{method.name}}_ResponseParams); + Completer c = completerQueue.removeAt(0); + c.complete(r); + break; +{%- endif %} +{%- endfor %} + default: + throw new Exception("Unexpected message name"); + break; + } + } +} + + +class {{interface.name}}Interface extends bindings.Interface { + {{interface.name}}Interface(core.MojoMessagePipeEndpoint endpoint) : super(endpoint); + + bindings.Message handleMessage(bindings.MessageReader reader, + Function messageHandler) { + switch (reader.name) { +{%- for method in interface.methods %} + case k{{interface.name}}_{{method.name}}_name: + var params = reader.decodeStruct({{interface.name}}_{{method.name}}_Params); +{%- if method.response_parameters == None %} + messageHandler(params); +{%- else %} + var response = messageHandler(params); + return buildResponseWithID( + {{interface.name}}_{{method.name}}_ResponseParams, + k{{interface.name}}_{{method.name}}_name, + bindings.kMessageIsResponse, + response); +{%- endif %} + break; +{%- endfor %} + default: + throw new Exception("Unexpected message name"); + break; + } + return null; + } +} + + +{#--- TODO(zra): Validation #} + + +{#--- Interface Constants #} +{% for constant in interface.constants %} +final {{constant.name}} = {{constant.value|expression_to_text}}; +{%- endfor %} + + +{#--- Interface Enums #} +{%- from "enum_definition.tmpl" import enum_def -%} +{%- for enum in interface.enums %} + {{ enum_def("", enum) }} +{%- endfor %} diff --git a/mojo/public/tools/bindings/generators/dart_templates/module.lib.tmpl b/mojo/public/tools/bindings/generators/dart_templates/module.lib.tmpl new file mode 100644 index 00000000000000..17e1b7820661bf --- /dev/null +++ b/mojo/public/tools/bindings/generators/dart_templates/module.lib.tmpl @@ -0,0 +1,15 @@ +// 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. + +library {{module.name}}; + +import 'dart:async'; + +import 'package:mojo/public/dart/core.dart' as core; +import 'package:mojo/public/dart/bindings.dart' as bindings; +{%- for import in imports %} +import 'package:{{import.module.path}}.dart' as {{import.unique_name}}; +{%- endfor %} + +{%- include "module_definition.tmpl" %} diff --git a/mojo/public/tools/bindings/generators/dart_templates/module_definition.tmpl b/mojo/public/tools/bindings/generators/dart_templates/module_definition.tmpl new file mode 100644 index 00000000000000..e425463eb5b486 --- /dev/null +++ b/mojo/public/tools/bindings/generators/dart_templates/module_definition.tmpl @@ -0,0 +1,20 @@ +{#--- Constants #} +{%- for constant in module.constants %} +final {{constant.name}} = {{constant.value|expression_to_text}}; +{%- endfor %} + +{#--- Enums #} +{%- from "enum_definition.tmpl" import enum_def %} +{%- for enum in enums %} +{{ enum_def("", enum) }} +{%- endfor %} + +{#--- Struct definitions #} +{% for struct in structs %} +{%- include "struct_definition.tmpl" %} +{%- endfor -%} + +{#--- Interface definitions #} +{%- for interface in interfaces -%} +{%- include "interface_definition.tmpl" %} +{%- endfor %} diff --git a/mojo/public/tools/bindings/generators/dart_templates/struct_definition.tmpl b/mojo/public/tools/bindings/generators/dart_templates/struct_definition.tmpl new file mode 100644 index 00000000000000..632a99c637a418 --- /dev/null +++ b/mojo/public/tools/bindings/generators/dart_templates/struct_definition.tmpl @@ -0,0 +1,74 @@ +{#--- Begin #} + +class {{struct.name}} implements bindings.MojoType<{{struct.name}}> { +{#--- Enums #} +{%- from "enum_definition.tmpl" import enum_def %} +{% for enum in struct.enums %} + {{enum_def(" static ", enum)}} +{%- endfor %} + + +{#--- Constants #} +{% for constant in struct.constants %} + static final {{constant.name}} = {{constant.value|expression_to_text}}; +{%- endfor %} + + +{#--- initDefaults() #} +{%- for packed_field in struct.packed.packed_fields %} + {{packed_field.field.kind|dart_decl_type}} {{packed_field.field.name}} = {{packed_field.field|default_value}}; +{%- endfor %} + + {{struct.name}}(); + +{#--- Encoding and decoding #} + + static const int encodedSize = + bindings.kStructHeaderSize + {{struct.packed|payload_size}}; + + static {{struct.name}} decode(bindings.MojoDecoder decoder) { + var packed; + var val = new {{struct.name}}(); + var numberOfBytes = decoder.readUint32(); + var numberOfFields = decoder.readUint32(); +{%- for byte in struct.bytes %} +{%- if byte.packed_fields|length > 1 %} + packed = decoder.readUint8(); +{%- for packed_field in byte.packed_fields %} + val.{{packed_field.field.name}} = (((packed >> {{packed_field.bit}}) & 1) != 0) ? true : false; +{%- endfor %} +{%- else %} +{%- for packed_field in byte.packed_fields %} + val.{{packed_field.field.name}} = decoder.{{packed_field.field.kind|decode_snippet}}; +{%- endfor %} +{%- endif %} +{%- if byte.is_padding %} + decoder.skip(1); +{%- endif %} +{%- endfor %} + return val; + } + + static void encode(bindings.MojoEncoder encoder, {{struct.name}} val) { + var packed; + encoder.writeUint32({{struct.name}}.encodedSize); + encoder.writeUint32({{struct.packed.packed_fields|length}}); + +{%- for byte in struct.bytes %} +{%- if byte.packed_fields|length > 1 %} + packed = 0; +{%- for packed_field in byte.packed_fields %} + packed |= (val.{{packed_field.field.name}} & 1) << {{packed_field.bit}}; +{%- endfor %} + encoder.writeUint8(packed); +{%- else %} +{%- for packed_field in byte.packed_fields %} + encoder.{{packed_field.field.kind|encode_snippet}}val.{{packed_field.field.name}}); +{%- endfor %} +{%- endif %} +{%- if byte.is_padding %} + encoder.skip(1); +{%- endif %} +{%- endfor %} + } +} \ No newline at end of file diff --git a/mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl b/mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl index 27c7cf353cf4e0..9462e8fbca8bd7 100644 --- a/mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl +++ b/mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl @@ -6,6 +6,10 @@ this.receiver_ = receiver; } + {{interface.name}}Proxy.prototype.getConnection$ = function() { + return connection.getProxyConnection(this, {{interface.name}}); + } + {%- for method in interface.methods %} {{interface.name}}Proxy.prototype.{{method.name|stylize_method}} = function( {%- for parameter in method.parameters -%} @@ -14,7 +18,19 @@ ) { var params = new {{interface.name}}_{{method.name}}_Params(); {%- for parameter in method.parameters %} +{%- if parameter|is_interface_parameter %} + if ({{parameter.name}} instanceof {{parameter.kind|js_type}}.stubClass) + params.{{parameter.name}} = {{parameter.name}}.getConnection$().messagePipeHandle; + else + params.{{parameter.name}} = {{parameter.name}}; +{%- elif parameter|is_interface_request_parameter %} + if ({{parameter.name}} instanceof {{parameter.kind.kind|js_type}}.proxyClass) + params.{{parameter.name}} = {{parameter.name}}.getConnection$().messagePipeHandle; + else + params.{{parameter.name}} = {{parameter.name}}; +{%- else %} params.{{parameter.name}} = {{parameter.name}}; +{%- endif %} {%- endfor %} {%- if method.response_parameters == None %} @@ -45,8 +61,25 @@ }; {%- endfor %} - function {{interface.name}}Stub() { + function {{interface.name}}Stub(delegate) { + this.delegate$ = delegate; + } + + {{interface.name}}Stub.prototype.getConnection$ = function() { + return connection.getStubConnection(this, {{interface.name}}); + } + +{%- for method in interface.methods %} +{% macro stub_method_parameters() -%} +{%- for parameter in method.parameters -%} + {{parameter.name}}{% if not loop.last %}, {% endif %} +{%- endfor %} +{%- endmacro %} + {{interface.name}}Stub.prototype.{{method.name|stylize_method}} = function({{stub_method_parameters()}}) { + if (this.delegate$.{{method.name|stylize_method}}) + return this.delegate$.{{method.name|stylize_method}}({{stub_method_parameters()}}); } +{%- endfor %} {{interface.name}}Stub.prototype.accept = function(message) { var reader = new codec.MessageReader(message); @@ -57,7 +90,7 @@ var params = reader.decodeStruct({{interface.name}}_{{method.name}}_Params); this.{{method.name|stylize_method}}( {%- for parameter in method.parameters -%} -params.{{parameter.name}}{% if not loop.last %}, {% endif %} + params.{{parameter.name}}{% if not loop.last %}, {% endif %} {%- endfor %}); return true; {%- endif %} @@ -100,24 +133,6 @@ params.{{parameter.name}}{% if not loop.last %}, {% endif -%} } }; - function {{interface.name}}DelegatingStub() { - } - - {{interface.name}}DelegatingStub.prototype = - Object.create({{interface.name}}Stub.prototype); - - {{interface.name}}DelegatingStub.prototype.callDelegateMethod$ = function(methodName, args) { - var method = this.delegate$ && this.delegate$[methodName]; - return method && method.apply(this.delegate$, args); - } - -{%- for method in interface.methods %} -{%- set method_name = method.name|stylize_method %} - {{interface.name}}DelegatingStub.prototype.{{method_name}} = function() { - return this.callDelegateMethod$("{{method_name}}", arguments); - } -{%- endfor %} - {#--- Validation #} function validate{{interface.name}}Request(messageValidator) { @@ -171,7 +186,6 @@ params.{{parameter.name}}{% if not loop.last %}, {% endif -%} name: '{{namespace|replace(".","::")}}::{{interface.name}}', proxyClass: {{interface.name}}Proxy, stubClass: {{interface.name}}Stub, - delegatingStubClass: {{interface.name}}DelegatingStub, validateRequest: validate{{interface.name}}Request, {%- if interface|has_callbacks %} validateResponse: validate{{interface.name}}Response, diff --git a/mojo/public/tools/bindings/generators/js_templates/module.amd.tmpl b/mojo/public/tools/bindings/generators/js_templates/module.amd.tmpl index 697eabe3994fbb..c76d2e9875a5d2 100644 --- a/mojo/public/tools/bindings/generators/js_templates/module.amd.tmpl +++ b/mojo/public/tools/bindings/generators/js_templates/module.amd.tmpl @@ -4,11 +4,12 @@ define("{{module.path}}", [ "mojo/public/js/codec", + "mojo/public/js/connection", "mojo/public/js/validator", {%- for import in imports %} "{{import.module.path}}", {%- endfor %} -], function(codec, validator +], function(codec, connection, validator {%- for import in imports -%} , {{import.unique_name}} {%- endfor -%} diff --git a/mojo/public/tools/bindings/generators/js_templates/module.sky.tmpl b/mojo/public/tools/bindings/generators/js_templates/module.sky.tmpl index 14b0d678382a17..5864cfbb0a49d1 100644 --- a/mojo/public/tools/bindings/generators/js_templates/module.sky.tmpl +++ b/mojo/public/tools/bindings/generators/js_templates/module.sky.tmpl @@ -3,6 +3,7 @@ found in the LICENSE file. --> + {%- for import in imports %} diff --git a/mojo/public/tools/bindings/generators/mojom_dart_generator.py b/mojo/public/tools/bindings/generators/mojom_dart_generator.py new file mode 100644 index 00000000000000..6c1fed3428fcca --- /dev/null +++ b/mojo/public/tools/bindings/generators/mojom_dart_generator.py @@ -0,0 +1,295 @@ +# Copyright 2013 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. + +"""Generates Dart source files from a mojom.Module.""" + +import mojom.generate.generator as generator +import mojom.generate.module as mojom +import mojom.generate.pack as pack +from mojom.generate.template_expander import UseJinja + +_kind_to_dart_default_value = { + mojom.BOOL: "false", + mojom.INT8: "0", + mojom.UINT8: "0", + mojom.INT16: "0", + mojom.UINT16: "0", + mojom.INT32: "0", + mojom.UINT32: "0", + mojom.FLOAT: "0.0", + mojom.HANDLE: "null", + mojom.DCPIPE: "null", + mojom.DPPIPE: "null", + mojom.MSGPIPE: "null", + mojom.SHAREDBUFFER: "null", + mojom.NULLABLE_HANDLE: "null", + mojom.NULLABLE_DCPIPE: "null", + mojom.NULLABLE_DPPIPE: "null", + mojom.NULLABLE_MSGPIPE: "null", + mojom.NULLABLE_SHAREDBUFFER: "null", + mojom.INT64: "0", + mojom.UINT64: "0", + mojom.DOUBLE: "0.0", + mojom.STRING: "null", + mojom.NULLABLE_STRING: "null" +} + + +_kind_to_dart_decl_type = { + mojom.BOOL: "bool", + mojom.INT8: "int", + mojom.UINT8: "int", + mojom.INT16: "int", + mojom.UINT16: "int", + mojom.INT32: "int", + mojom.UINT32: "int", + mojom.FLOAT: "double", + mojom.HANDLE: "RawMojoHandle", + mojom.DCPIPE: "RawMojoHandle", + mojom.DPPIPE: "RawMojoHandle", + mojom.MSGPIPE: "RawMojoHandle", + mojom.SHAREDBUFFER: "RawMojoHandle", + mojom.NULLABLE_HANDLE: "RawMojoHandle", + mojom.NULLABLE_DCPIPE: "RawMojoHandle", + mojom.NULLABLE_DPPIPE: "RawMojoHandle", + mojom.NULLABLE_MSGPIPE: "RawMojoHandle", + mojom.NULLABLE_SHAREDBUFFER: "RawMojoHandle", + mojom.INT64: "int", + mojom.UINT64: "int", + mojom.DOUBLE: "double", + mojom.STRING: "String", + mojom.NULLABLE_STRING: "String" +} + + +def DartType(kind): + if kind.imported_from: + return kind.imported_from["unique_name"] + "." + kind.name + return kind.name + + +def DartDefaultValue(field): + if field.default: + if mojom.IsStructKind(field.kind): + assert field.default == "default" + return "new %s()" % DartType(field.kind) + return ExpressionToText(field.default) + if field.kind in mojom.PRIMITIVES: + return _kind_to_dart_default_value[field.kind] + if mojom.IsStructKind(field.kind): + return "null" + if mojom.IsArrayKind(field.kind): + return "null" + if mojom.IsMapKind(field.kind): + return "null" + if mojom.IsInterfaceKind(field.kind) or \ + mojom.IsInterfaceRequestKind(field.kind): + return _kind_to_dart_default_value[mojom.MSGPIPE] + if mojom.IsEnumKind(field.kind): + return "0" + + +def DartDeclType(kind): + if kind in mojom.PRIMITIVES: + return _kind_to_dart_decl_type[kind] + if mojom.IsStructKind(kind): + return DartType(kind) + if mojom.IsArrayKind(kind): + array_type = DartDeclType(kind.kind) + return "List<" + array_type + ">" + if mojom.IsMapKind(kind): + key_type = DartDeclType(kind.key_kind) + value_type = DartDeclType(kind.value_kind) + return "Map<"+ key_type + ", " + value_type + ">" + if mojom.IsInterfaceKind(kind) or \ + mojom.IsInterfaceRequestKind(kind): + return _kind_to_dart_decl_type[mojom.MSGPIPE] + if mojom.IsEnumKind(kind): + return "int" + +def DartPayloadSize(packed): + packed_fields = packed.packed_fields + if not packed_fields: + return 0 + last_field = packed_fields[-1] + offset = last_field.offset + last_field.size + pad = pack.GetPad(offset, 8) + return offset + pad + + +_kind_to_codec_type = { + mojom.BOOL: "bindings.Uint8", + mojom.INT8: "bindings.Int8", + mojom.UINT8: "bindings.Uint8", + mojom.INT16: "bindings.Int16", + mojom.UINT16: "bindings.Uint16", + mojom.INT32: "bindings.Int32", + mojom.UINT32: "bindings.Uint32", + mojom.FLOAT: "bindings.Float", + mojom.HANDLE: "bindings.Handle", + mojom.DCPIPE: "bindings.Handle", + mojom.DPPIPE: "bindings.Handle", + mojom.MSGPIPE: "bindings.Handle", + mojom.SHAREDBUFFER: "bindings.Handle", + mojom.NULLABLE_HANDLE: "bindings.NullableHandle", + mojom.NULLABLE_DCPIPE: "bindings.NullableHandle", + mojom.NULLABLE_DPPIPE: "bindings.NullableHandle", + mojom.NULLABLE_MSGPIPE: "bindings.NullableHandle", + mojom.NULLABLE_SHAREDBUFFER: "bindings.NullableHandle", + mojom.INT64: "bindings.Int64", + mojom.UINT64: "bindings.Uint64", + mojom.DOUBLE: "bindings.Double", + mojom.STRING: "bindings.MojoString", + mojom.NULLABLE_STRING: "bindings.NullableMojoString", +} + + +def CodecType(kind): + if kind in mojom.PRIMITIVES: + return _kind_to_codec_type[kind] + if mojom.IsStructKind(kind): + pointer_type = "NullablePointerTo" if mojom.IsNullableKind(kind) \ + else "PointerTo" + return "new bindings.%s(%s)" % (pointer_type, DartType(kind)) + if mojom.IsArrayKind(kind): + array_type = "NullableArrayOf" if mojom.IsNullableKind(kind) else "ArrayOf" + array_length = "" if kind.length is None else ", %d" % kind.length + element_type = "bindings.PackedBool" if mojom.IsBoolKind(kind.kind) \ + else CodecType(kind.kind) + return "new bindings.%s(%s%s)" % (array_type, element_type, array_length) + if mojom.IsInterfaceKind(kind) or mojom.IsInterfaceRequestKind(kind): + return CodecType(mojom.MSGPIPE) + if mojom.IsEnumKind(kind): + return _kind_to_codec_type[mojom.INT32] + return kind + +def MapCodecType(kind): + return "bindings.PackedBool" if mojom.IsBoolKind(kind) else CodecType(kind) + +def DartDecodeSnippet(kind): + if kind in mojom.PRIMITIVES: + return "decodeStruct(%s)" % CodecType(kind) + if mojom.IsStructKind(kind): + return "decodeStructPointer(%s)" % DartType(kind) + if mojom.IsMapKind(kind): + return "decodeMapPointer(%s, %s)" % \ + (MapCodecType(kind.key_kind), MapCodecType(kind.value_kind)) + if mojom.IsArrayKind(kind) and mojom.IsBoolKind(kind.kind): + return "decodeArrayPointer(bindings.PackedBool)" + if mojom.IsArrayKind(kind): + return "decodeArrayPointer(%s)" % CodecType(kind.kind) + if mojom.IsInterfaceKind(kind) or mojom.IsInterfaceRequestKind(kind): + return DartDecodeSnippet(mojom.MSGPIPE) + if mojom.IsEnumKind(kind): + return DartDecodeSnippet(mojom.INT32) + + +def DartEncodeSnippet(kind): + if kind in mojom.PRIMITIVES: + return "encodeStruct(%s, " % CodecType(kind) + if mojom.IsStructKind(kind): + return "encodeStructPointer(%s, " % DartType(kind) + if mojom.IsMapKind(kind): + return "encodeMapPointer(%s, %s, " % \ + (MapCodecType(kind.key_kind), MapCodecType(kind.value_kind)) + if mojom.IsArrayKind(kind) and mojom.IsBoolKind(kind.kind): + return "encodeArrayPointer(bindings.PackedBool, "; + if mojom.IsArrayKind(kind): + return "encodeArrayPointer(%s, " % CodecType(kind.kind) + if mojom.IsInterfaceKind(kind) or mojom.IsInterfaceRequestKind(kind): + return DartEncodeSnippet(mojom.MSGPIPE) + if mojom.IsEnumKind(kind): + return DartEncodeSnippet(mojom.INT32) + + +def TranslateConstants(token): + if isinstance(token, (mojom.EnumValue, mojom.NamedValue)): + # Both variable and enum constants are constructed like: + # NamespaceUid.Struct.Enum_CONSTANT_NAME + name = "" + if token.imported_from: + name = token.imported_from["unique_name"] + "." + if token.parent_kind: + name = name + token.parent_kind.name + "." + if isinstance(token, mojom.EnumValue): + name = name + token.enum.name + "_" + return name + token.name + + if isinstance(token, mojom.BuiltinValue): + if token.value == "double.INFINITY" or token.value == "float.INFINITY": + return "double.INFINITY"; + if token.value == "double.NEGATIVE_INFINITY" or \ + token.value == "float.NEGATIVE_INFINITY": + return "double.NEGATIVE_INFINITY"; + if token.value == "double.NAN" or token.value == "float.NAN": + return "double.NAN"; + + # Strip leading '+'. + if token[0] == '+': + token = token[1:] + + return token + + +def ExpressionToText(value): + return TranslateConstants(value) + + +class Generator(generator.Generator): + + dart_filters = { + "default_value": DartDefaultValue, + "payload_size": DartPayloadSize, + "decode_snippet": DartDecodeSnippet, + "encode_snippet": DartEncodeSnippet, + "expression_to_text": ExpressionToText, + "dart_decl_type": DartDeclType, + "stylize_method": generator.StudlyCapsToCamel, + } + + def GetParameters(self): + return { + "namespace": self.module.namespace, + "imports": self.GetImports(), + "kinds": self.module.kinds, + "enums": self.module.enums, + "module": self.module, + "structs": self.GetStructs() + self.GetStructsFromMethods(), + "interfaces": self.module.interfaces, + "imported_interfaces": self.GetImportedInterfaces(), + } + + @UseJinja("dart_templates/module.lib.tmpl", filters=dart_filters) + def GenerateLibModule(self): + return self.GetParameters() + + def GenerateFiles(self, args): + self.Write(self.GenerateLibModule(), + self.MatchMojomFilePath("%s.dart" % self.module.name)) + + def GetImports(self): + used_names = set() + for each_import in self.module.imports: + simple_name = each_import["module_name"].split(".")[0] + + # Since each import is assigned a variable in JS, they need to have unique + # names. + unique_name = simple_name + counter = 0 + while unique_name in used_names: + counter += 1 + unique_name = simple_name + str(counter) + + used_names.add(unique_name) + each_import["unique_name"] = unique_name + counter += 1 + return self.module.imports + + def GetImportedInterfaces(self): + interface_to_import = {}; + for each_import in self.module.imports: + for each_interface in each_import["module"].interfaces: + name = each_interface.name + interface_to_import[name] = each_import["unique_name"] + "." + name + return interface_to_import; diff --git a/mojo/public/tools/bindings/generators/mojom_js_generator.py b/mojo/public/tools/bindings/generators/mojom_js_generator.py index 47fea232a0df69..7ff6759c507798 100644 --- a/mojo/public/tools/bindings/generators/mojom_js_generator.py +++ b/mojo/public/tools/bindings/generators/mojom_js_generator.py @@ -249,7 +249,6 @@ def TranslateConstants(token): def ExpressionToText(value): return TranslateConstants(value) - def IsArrayPointerField(field): return mojom.IsArrayKind(field.kind) @@ -265,6 +264,12 @@ def IsMapPointerField(field): def IsHandleField(field): return mojom.IsAnyHandleKind(field.kind) +def IsInterfaceRequestParameter(parameter): + return mojom.IsInterfaceRequestKind(parameter.kind) + +def IsInterfaceParameter(parameter): + return mojom.IsInterfaceKind(parameter.kind) + class Generator(generator.Generator): @@ -282,6 +287,8 @@ class Generator(generator.Generator): "is_string_pointer_field": IsStringPointerField, "is_handle_field": IsHandleField, "js_type": JavaScriptType, + "is_interface_request_parameter": IsInterfaceRequestParameter, + "is_interface_parameter": IsInterfaceParameter, "stylize_method": generator.StudlyCapsToCamel, "validate_array_params": JavaScriptValidateArrayParams, "validate_handle_params": JavaScriptValidateHandleParams, diff --git a/mojo/public/tools/bindings/mojom.gni b/mojo/public/tools/bindings/mojom.gni index 5a7932e1974b7f..69f4b768ff58a2 100644 --- a/mojo/public/tools/bindings/mojom.gni +++ b/mojo/public/tools/bindings/mojom.gni @@ -47,6 +47,11 @@ template("mojom") { "$generator_root/generators/cpp_templates/struct_macros.tmpl", "$generator_root/generators/cpp_templates/wrapper_class_declaration.tmpl", "$generator_root/generators/cpp_templates/wrapper_class_definition.tmpl", + "$generator_root/generators/dart_templates/enum_definition.tmpl", + "$generator_root/generators/dart_templates/interface_definition.tmpl", + "$generator_root/generators/dart_templates/module.lib.tmpl", + "$generator_root/generators/dart_templates/module_definition.tmpl", + "$generator_root/generators/dart_templates/struct_definition.tmpl", "$generator_root/generators/java_templates/constant_definition.tmpl", "$generator_root/generators/java_templates/constants.java.tmpl", "$generator_root/generators/java_templates/enum.java.tmpl", @@ -66,6 +71,7 @@ template("mojom") { "$generator_root/generators/python_templates/module_macros.tmpl", "$generator_root/generators/python_templates/module.py.tmpl", "$generator_root/generators/mojom_cpp_generator.py", + "$generator_root/generators/mojom_dart_generator.py", "$generator_root/generators/mojom_js_generator.py", "$generator_root/generators/mojom_java_generator.py", "$generator_root/generators/mojom_python_generator.py", @@ -88,6 +94,9 @@ template("mojom") { "{{source_gen_dir}}/{{source_name_part}}.mojom.h", "{{source_gen_dir}}/{{source_name_part}}.mojom-internal.h", ] + generator_dart_outputs = [ + "{{source_gen_dir}}/{{source_name_part}}.mojom.dart", + ] generator_java_outputs = [ "{{source_gen_dir}}/{{source_name_part}}.mojom.srcjar", ] @@ -114,6 +123,7 @@ template("mojom") { inputs = generator_sources sources = invoker.sources outputs = generator_cpp_outputs + + generator_dart_outputs + generator_java_outputs + generator_js_outputs + generator_python_outputs diff --git a/mojo/public/tools/bindings/mojom_bindings_generator.py b/mojo/public/tools/bindings/mojom_bindings_generator.py index cccc42a514e6ac..7dc90671c0581e 100755 --- a/mojo/public/tools/bindings/mojom_bindings_generator.py +++ b/mojo/public/tools/bindings/mojom_bindings_generator.py @@ -50,6 +50,9 @@ def LoadGenerators(generators_string): if generator_name.lower() == "c++": generator_name = os.path.join(script_dir, "generators", "mojom_cpp_generator.py") + if generator_name.lower() == "dart": + generator_name = os.path.join(script_dir, "generators", + "mojom_dart_generator.py") elif generator_name.lower() == "javascript": generator_name = os.path.join(script_dir, "generators", "mojom_js_generator.py") @@ -170,7 +173,7 @@ def main(): help="output directory for generated files") parser.add_argument("-g", "--generators", dest="generators_string", metavar="GENERATORS", - default="c++,javascript,java,python", + default="c++,dart,javascript,java,python", help="comma-separated list of generators") parser.add_argument("--debug_print_intermediate", action="store_true", help="print the intermediate representation") diff --git a/mojo/public/tools/bindings/pylib/mojom/generate/data.py b/mojo/public/tools/bindings/pylib/mojom/generate/data.py index 85c9105679ab64..4aca497cc387f0 100644 --- a/mojo/public/tools/bindings/pylib/mojom/generate/data.py +++ b/mojo/public/tools/bindings/pylib/mojom/generate/data.py @@ -128,9 +128,17 @@ def KindFromData(kinds, data, scope): elif data.startswith('r:'): kind = mojom.InterfaceRequest(KindFromData(kinds, data[2:], scope)) elif data.startswith('m['): - # Isolate the two types from their brackets - first_kind = data[2:data.find(']')] - second_kind = data[data.rfind('[')+1:data.rfind(']')] + # Isolate the two types from their brackets. + + # It is not allowed to use map as key, so there shouldn't be nested ']'s + # inside the key type spec. + key_end = data.find(']') + assert key_end != -1 and key_end < len(data) - 1 + assert data[key_end+1] == '[' and data[-1] == ']' + + first_kind = data[2:key_end] + second_kind = data[key_end+2:-1] + kind = mojom.Map(KindFromData(kinds, first_kind, scope), KindFromData(kinds, second_kind, scope)) else: diff --git a/mojo/public/tools/download_shell_binary.py b/mojo/public/tools/download_shell_binary.py index e28034c0fe7547..2a0890c5f6d007 100755 --- a/mojo/public/tools/download_shell_binary.py +++ b/mojo/public/tools/download_shell_binary.py @@ -25,6 +25,7 @@ depot_tools_path = find_depot_tools.add_depot_tools_to_path() gsutil_exe = os.path.join(depot_tools_path, "third_party", "gsutil", "gsutil") + def download(): version_path = os.path.join(current_path, "../VERSION") with open(version_path) as version_file: @@ -38,17 +39,39 @@ def download(): except IOError: pass # If the stamp file does not exist we need to download a new binary. - platform = "linux-x64" # TODO: configurate + platform = "linux-x64" # TODO: configurate basename = platform + ".zip" gs_path = "gs://mojo/shell/" + version + "/" + basename with tempfile.NamedTemporaryFile() as temp_zip_file: - subprocess.check_call([gsutil_exe, "--bypass_prodaccess", - "cp", gs_path, temp_zip_file.name]) + # We're downloading from a public bucket which does not need authentication, + # but the user might have busted credential files somewhere such as ~/.boto + # that the gsutil script will try (and fail) to use. Setting these + # environment variables convinces gsutil not to attempt to use these, but + # also generates a useless warning about failing to load the file. We want + # to discard this warning but still preserve all output in the case of an + # actual failure. So, we run the script and capture all output and then + # throw the output away if the script succeeds (return code 0). + env = os.environ.copy() + env["AWS_CREDENTIAL_FILE"] = "" + env["BOTO_CONFIG"] = "" + try: + subprocess.check_output( + [gsutil_exe, + "--bypass_prodaccess", + "cp", + gs_path, + temp_zip_file.name], + stderr=subprocess.STDOUT, + env=env) + except subprocess.CalledProcessError as e: + print e.output + sys.exit(1) + with zipfile.ZipFile(temp_zip_file.name) as z: zi = z.getinfo("mojo_shell") - mode = zi.external_attr >> 16L + mode = zi.external_attr >> 16 z.extract(zi, prebuilt_file_path) os.chmod(os.path.join(prebuilt_file_path, "mojo_shell"), mode) @@ -56,9 +79,10 @@ def download(): stamp_file.write(version) return 0 + def main(): parser = argparse.ArgumentParser(description="Download mojo_shell binary " - "from google storage") + "from google storage") parser.parse_args() return download() diff --git a/mojo/services/public/cpp/view_manager/BUILD.gn b/mojo/services/public/cpp/view_manager/BUILD.gn index 7192ef7c8f7a78..dffa4f1c3e495c 100644 --- a/mojo/services/public/cpp/view_manager/BUILD.gn +++ b/mojo/services/public/cpp/view_manager/BUILD.gn @@ -18,9 +18,9 @@ source_set("view_manager") { "view_manager_context.h", "view_manager_delegate.h", "view_observer.h", + "view_property.h", "view_tracker.cc", "view_tracker.h", - "window_manager_delegate.h", ] public_deps = [ @@ -29,6 +29,7 @@ source_set("view_manager") { deps = [ "//base", "//mojo/public/c/gles2", + "//mojo/public/cpp/application", "//mojo/public/cpp/bindings:bindings", "//mojo/public/interfaces/application", "//mojo/services/public/interfaces/geometry", @@ -41,10 +42,6 @@ source_set("view_manager") { source_set("common") { sources = [ - "ids.h", "types.h", ] - public_deps = [ - "//base", - ] } diff --git a/mojo/services/public/cpp/view_manager/lib/view.cc b/mojo/services/public/cpp/view_manager/lib/view.cc index b371000ee798c2..1ed7a62b638711 100644 --- a/mojo/services/public/cpp/view_manager/lib/view.cc +++ b/mojo/services/public/cpp/view_manager/lib/view.cc @@ -209,6 +209,9 @@ void View::SetBounds(const Rect& bounds) { if (!OwnsView(manager_, this)) return; + if (bounds_.Equals(bounds)) + return; + if (manager_) static_cast(manager_)->SetBounds(id_, bounds); LocalSetBounds(bounds_, bounds); @@ -225,8 +228,8 @@ void View::SetVisible(bool value) { FOR_EACH_OBSERVER(ViewObserver, observers_, OnViewVisibilityChanged(this)); } -void View::SetProperty(const std::string& name, - const std::vector* value) { +void View::SetSharedProperty(const std::string& name, + const std::vector* value) { std::vector old_value; std::vector* old_value_ptr = nullptr; auto it = properties_.find(name); @@ -248,8 +251,9 @@ void View::SetProperty(const std::string& name, properties_.erase(it); } - FOR_EACH_OBSERVER(ViewObserver, observers_, - OnViewPropertyChanged(this, name, old_value_ptr, value)); + FOR_EACH_OBSERVER( + ViewObserver, observers_, + OnViewSharedPropertyChanged(this, name, old_value_ptr, value)); } bool View::IsDrawn() const { @@ -266,6 +270,13 @@ void View::RemoveObserver(ViewObserver* observer) { observers_.RemoveObserver(observer); } +const View* View::GetRoot() const { + const View* root = this; + for (const View* parent = this; parent; parent = parent->parent()) + root = parent; + return root; +} + void View::AddChild(View* child) { // TODO(beng): not necessarily valid to all connections, but possibly to the // embeddee in an embedder-embeddee relationship. @@ -382,10 +393,27 @@ View::~View() { FOR_EACH_OBSERVER(ViewObserver, observers_, OnViewDestroying(this)); if (parent_) parent_->LocalRemoveChild(this); + + // We may still have children. This can happen if the embedder destroys the + // root while we're still alive. + while (!children_.empty()) { + View* child = children_.front(); + LocalRemoveChild(child); + DCHECK(children_.empty() || children_.front() != child); + } + // TODO(beng): It'd be better to do this via a destruction observer in the // ViewManagerClientImpl. if (manager_) static_cast(manager_)->RemoveView(id_); + + // Clear properties. + for (auto& pair : prop_map_) { + if (pair.second.deallocator) + (*pair.second.deallocator)(pair.second.value); + } + prop_map_.clear(); + FOR_EACH_OBSERVER(ViewObserver, observers_, OnViewDestroyed(this)); } @@ -396,10 +424,38 @@ View::View(ViewManager* manager) : manager_(manager), id_(static_cast(manager_)->CreateView()), parent_(NULL), - visible_(true), + visible_(false), drawn_(false) { } +int64 View::SetLocalPropertyInternal(const void* key, + const char* name, + PropertyDeallocator deallocator, + int64 value, + int64 default_value) { + int64 old = GetLocalPropertyInternal(key, default_value); + if (value == default_value) { + prop_map_.erase(key); + } else { + Value prop_value; + prop_value.name = name; + prop_value.value = value; + prop_value.deallocator = deallocator; + prop_map_[key] = prop_value; + } + FOR_EACH_OBSERVER(ViewObserver, observers_, + OnViewLocalPropertyChanged(this, key, old)); + return old; +} + +int64 View::GetLocalPropertyInternal(const void* key, + int64 default_value) const { + std::map::const_iterator iter = prop_map_.find(key); + if (iter == prop_map_.end()) + return default_value; + return iter->second.value; +} + void View::LocalDestroy() { delete this; } diff --git a/mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.cc b/mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.cc index 9421e180ed388d..e70ec2bda312cb 100644 --- a/mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.cc +++ b/mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.cc @@ -341,7 +341,7 @@ void ViewManagerClientImpl::OnViewDrawnStateChanged(Id view_id, bool drawn) { ViewPrivate(view).LocalSetDrawn(drawn); } -void ViewManagerClientImpl::OnViewPropertyChanged( +void ViewManagerClientImpl::OnViewSharedPropertyChanged( Id view_id, const String& name, Array new_data) { @@ -354,7 +354,7 @@ void ViewManagerClientImpl::OnViewPropertyChanged( data_ptr = &data; } - view->SetProperty(name, data_ptr); + view->SetSharedProperty(name, data_ptr); } } diff --git a/mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.h b/mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.h index 2c941da7761294..2707ad6c92a553 100644 --- a/mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.h +++ b/mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.h @@ -108,9 +108,9 @@ class ViewManagerClientImpl : public ViewManager, void OnViewDeleted(Id view_id) override; void OnViewVisibilityChanged(Id view_id, bool visible) override; void OnViewDrawnStateChanged(Id view_id, bool drawn) override; - void OnViewPropertyChanged(Id view_id, - const String& name, - Array new_data) override; + void OnViewSharedPropertyChanged(Id view_id, + const String& name, + Array new_data) override; void OnViewInputEvent(Id view_id, EventPtr event, const Callback& callback) override; diff --git a/mojo/services/public/cpp/view_manager/tests/BUILD.gn b/mojo/services/public/cpp/view_manager/tests/BUILD.gn index c52727582fcbdc..d04bb34806cb06 100644 --- a/mojo/services/public/cpp/view_manager/tests/BUILD.gn +++ b/mojo/services/public/cpp/view_manager/tests/BUILD.gn @@ -19,6 +19,7 @@ test("mojo_view_manager_lib_unittests") { "//mojo/application_manager", "//mojo/converters/geometry", "//mojo/environment:chromium", + "//mojo/public/cpp/application", "//mojo/services/public/cpp/geometry", "//mojo/services/public/interfaces/geometry", "//mojo/shell:test_support", diff --git a/mojo/services/public/cpp/view_manager/tests/view_manager_unittest.cc b/mojo/services/public/cpp/view_manager/tests/view_manager_unittest.cc index 620307eb64075c..540b14581757c1 100644 --- a/mojo/services/public/cpp/view_manager/tests/view_manager_unittest.cc +++ b/mojo/services/public/cpp/view_manager/tests/view_manager_unittest.cc @@ -63,12 +63,11 @@ class ConnectApplicationLoader : public ApplicationLoader, // Overridden from ApplicationLoader: void Load(ApplicationManager* manager, const GURL& url, - scoped_refptr callbacks) override { - ScopedMessagePipeHandle shell_handle = callbacks->RegisterApplication(); - if (!shell_handle.is_valid()) - return; - scoped_ptr app(new ApplicationImpl(this, - shell_handle.Pass())); + ScopedMessagePipeHandle shell_handle, + LoadCallback callback) override { + ASSERT_TRUE(shell_handle.is_valid()); + scoped_ptr app( + new ApplicationImpl(this, shell_handle.Pass())); apps_.push_back(app.release()); } @@ -277,6 +276,7 @@ class ViewManagerTest : public testing::Test { View* CreateViewInParent(View* parent) { ViewManager* parent_manager = ViewPrivate(parent).view_manager(); View* view = View::Create(parent_manager); + view->SetVisible(true); parent->AddChild(view); return view; } @@ -348,6 +348,7 @@ TEST_F(ViewManagerTest, DISABLED_SetUp) {} TEST_F(ViewManagerTest, DISABLED_Embed) { View* view = View::Create(window_manager()); + view->SetVisible(true); window_manager()->GetRoots().front()->AddChild(view); ViewManager* embedded = Embed(window_manager(), view); EXPECT_TRUE(NULL != embedded); @@ -362,8 +363,10 @@ TEST_F(ViewManagerTest, DISABLED_Embed) { // TODO(sky): Update client lib to match server. TEST_F(ViewManagerTest, DISABLED_EmbeddedDoesntSeeChild) { View* view = View::Create(window_manager()); + view->SetVisible(true); window_manager()->GetRoots().front()->AddChild(view); View* nested = View::Create(window_manager()); + nested->SetVisible(true); view->AddChild(nested); ViewManager* embedded = Embed(window_manager(), view); @@ -376,6 +379,7 @@ TEST_F(ViewManagerTest, DISABLED_EmbeddedDoesntSeeChild) { // http://crbug.com/396300 TEST_F(ViewManagerTest, DISABLED_ViewManagerDestroyed_CleanupView) { View* view = View::Create(window_manager()); + view->SetVisible(true); window_manager()->GetRoots().front()->AddChild(view); ViewManager* embedded = Embed(window_manager(), view); @@ -406,6 +410,7 @@ TEST_F(ViewManagerTest, DISABLED_ViewManagerDestroyed_CleanupView) { // are reflected to another. TEST_F(ViewManagerTest, DISABLED_SetBounds) { View* view = View::Create(window_manager()); + view->SetVisible(true); window_manager()->GetRoots().front()->AddChild(view); ViewManager* embedded = Embed(window_manager(), view); @@ -424,6 +429,7 @@ TEST_F(ViewManagerTest, DISABLED_SetBounds) { // connection are refused. TEST_F(ViewManagerTest, DISABLED_SetBoundsSecurity) { View* view = View::Create(window_manager()); + view->SetVisible(true); window_manager()->GetRoots().front()->AddChild(view); ViewManager* embedded = Embed(window_manager(), view); @@ -444,6 +450,7 @@ TEST_F(ViewManagerTest, DISABLED_SetBoundsSecurity) { // Verifies that a view can only be destroyed by the connection that created it. TEST_F(ViewManagerTest, DISABLED_DestroySecurity) { View* view = View::Create(window_manager()); + view->SetVisible(true); window_manager()->GetRoots().front()->AddChild(view); ViewManager* embedded = Embed(window_manager(), view); @@ -461,8 +468,10 @@ TEST_F(ViewManagerTest, DISABLED_DestroySecurity) { TEST_F(ViewManagerTest, DISABLED_MultiRoots) { View* view1 = View::Create(window_manager()); + view1->SetVisible(true); window_manager()->GetRoots().front()->AddChild(view1); View* view2 = View::Create(window_manager()); + view2->SetVisible(true); window_manager()->GetRoots().front()->AddChild(view2); ViewManager* embedded1 = Embed(window_manager(), view1); ViewManager* embedded2 = Embed(window_manager(), view2); @@ -471,6 +480,7 @@ TEST_F(ViewManagerTest, DISABLED_MultiRoots) { TEST_F(ViewManagerTest, DISABLED_EmbeddingIdentity) { View* view = View::Create(window_manager()); + view->SetVisible(true); window_manager()->GetRoots().front()->AddChild(view); ViewManager* embedded = Embed(window_manager(), view); EXPECT_EQ(kWindowManagerURL, embedded->GetEmbedderURL()); @@ -478,13 +488,16 @@ TEST_F(ViewManagerTest, DISABLED_EmbeddingIdentity) { TEST_F(ViewManagerTest, DISABLED_Reorder) { View* view1 = View::Create(window_manager()); + view1->SetVisible(true); window_manager()->GetRoots().front()->AddChild(view1); ViewManager* embedded = Embed(window_manager(), view1); View* view11 = View::Create(embedded); + view11->SetVisible(true); embedded->GetRoots().front()->AddChild(view11); View* view12 = View::Create(embedded); + view12->SetVisible(true); embedded->GetRoots().front()->AddChild(view12); View* view1_in_wm = window_manager()->GetViewById(view1->id()); @@ -538,6 +551,7 @@ class VisibilityChangeObserver : public ViewObserver { TEST_F(ViewManagerTest, DISABLED_Visible) { View* view1 = View::Create(window_manager()); + view1->SetVisible(true); window_manager()->GetRoots().front()->AddChild(view1); // Embed another app and verify initial state. @@ -600,6 +614,7 @@ class DrawnChangeObserver : public ViewObserver { TEST_F(ViewManagerTest, DISABLED_Drawn) { View* view1 = View::Create(window_manager()); + view1->SetVisible(true); window_manager()->GetRoots().front()->AddChild(view1); // Embed another app and verify initial state. @@ -632,4 +647,7 @@ TEST_F(ViewManagerTest, DISABLED_Drawn) { // - focus between views unknown to one of the connections. // - focus between views unknown to either connection. +// TODO(sky): need test of root being destroyed with existing views. See +// 434555 for specific case. + } // namespace mojo diff --git a/mojo/services/public/cpp/view_manager/tests/view_unittest.cc b/mojo/services/public/cpp/view_manager/tests/view_unittest.cc index eb7276cd781282..b0bd08b6b768d3 100644 --- a/mojo/services/public/cpp/view_manager/tests/view_unittest.cc +++ b/mojo/services/public/cpp/view_manager/tests/view_unittest.cc @@ -9,6 +9,7 @@ #include "mojo/services/public/cpp/view_manager/lib/view_private.h" #include "mojo/services/public/cpp/view_manager/util.h" #include "mojo/services/public/cpp/view_manager/view_observer.h" +#include "mojo/services/public/cpp/view_manager/view_property.h" #include "testing/gtest/include/gtest/gtest.h" namespace mojo { @@ -102,6 +103,88 @@ TEST_F(ViewTest, DrawnAndVisible) { EXPECT_FALSE(v11.IsDrawn()); } +namespace { +DEFINE_VIEW_PROPERTY_KEY(int, kIntKey, -2); +DEFINE_VIEW_PROPERTY_KEY(const char*, kStringKey, "squeamish"); +} + +TEST_F(ViewTest, Property) { + TestView v; + + // Non-existent properties should return the default values. + EXPECT_EQ(-2, v.GetLocalProperty(kIntKey)); + EXPECT_EQ(std::string("squeamish"), v.GetLocalProperty(kStringKey)); + + // A set property value should be returned again (even if it's the default + // value). + v.SetLocalProperty(kIntKey, INT_MAX); + EXPECT_EQ(INT_MAX, v.GetLocalProperty(kIntKey)); + v.SetLocalProperty(kIntKey, -2); + EXPECT_EQ(-2, v.GetLocalProperty(kIntKey)); + v.SetLocalProperty(kIntKey, INT_MIN); + EXPECT_EQ(INT_MIN, v.GetLocalProperty(kIntKey)); + + v.SetLocalProperty(kStringKey, static_cast(NULL)); + EXPECT_EQ(NULL, v.GetLocalProperty(kStringKey)); + v.SetLocalProperty(kStringKey, "squeamish"); + EXPECT_EQ(std::string("squeamish"), v.GetLocalProperty(kStringKey)); + v.SetLocalProperty(kStringKey, "ossifrage"); + EXPECT_EQ(std::string("ossifrage"), v.GetLocalProperty(kStringKey)); + + // ClearProperty should restore the default value. + v.ClearLocalProperty(kIntKey); + EXPECT_EQ(-2, v.GetLocalProperty(kIntKey)); + v.ClearLocalProperty(kStringKey); + EXPECT_EQ(std::string("squeamish"), v.GetLocalProperty(kStringKey)); +} + +namespace { + +class TestProperty { + public: + TestProperty() {} + virtual ~TestProperty() { last_deleted_ = this; } + static TestProperty* last_deleted() { return last_deleted_; } + + private: + static TestProperty* last_deleted_; + DISALLOW_COPY_AND_ASSIGN(TestProperty); +}; + +TestProperty* TestProperty::last_deleted_ = NULL; + +DEFINE_OWNED_VIEW_PROPERTY_KEY(TestProperty, kOwnedKey, NULL); + +} // namespace + +TEST_F(ViewTest, OwnedProperty) { + TestProperty* p3 = NULL; + { + TestView v; + EXPECT_EQ(NULL, v.GetLocalProperty(kOwnedKey)); + TestProperty* p1 = new TestProperty(); + v.SetLocalProperty(kOwnedKey, p1); + EXPECT_EQ(p1, v.GetLocalProperty(kOwnedKey)); + EXPECT_EQ(NULL, TestProperty::last_deleted()); + + TestProperty* p2 = new TestProperty(); + v.SetLocalProperty(kOwnedKey, p2); + EXPECT_EQ(p2, v.GetLocalProperty(kOwnedKey)); + EXPECT_EQ(p1, TestProperty::last_deleted()); + + v.ClearLocalProperty(kOwnedKey); + EXPECT_EQ(NULL, v.GetLocalProperty(kOwnedKey)); + EXPECT_EQ(p2, TestProperty::last_deleted()); + + p3 = new TestProperty(); + v.SetLocalProperty(kOwnedKey, p3); + EXPECT_EQ(p3, v.GetLocalProperty(kOwnedKey)); + EXPECT_EQ(p2, TestProperty::last_deleted()); + } + + EXPECT_EQ(p3, TestProperty::last_deleted()); +} + // ViewObserver -------------------------------------------------------- typedef testing::Test ViewObserverTest; @@ -614,12 +697,12 @@ TEST_F(ViewObserverTest, SetVisible) { namespace { -class PropertyChangeObserver : public ViewObserver { +class SharedPropertyChangeObserver : public ViewObserver { public: - explicit PropertyChangeObserver(View* view) : view_(view) { + explicit SharedPropertyChangeObserver(View* view) : view_(view) { view_->AddObserver(this); } - virtual ~PropertyChangeObserver() { view_->RemoveObserver(this); } + virtual ~SharedPropertyChangeObserver() { view_->RemoveObserver(this); } Changes GetAndClearChanges() { Changes changes; @@ -629,16 +712,15 @@ class PropertyChangeObserver : public ViewObserver { private: // Overridden from ViewObserver: - void OnViewPropertyChanged(View* view, - const std::string& name, - const std::vector* old_data, - const std::vector* new_data) override { + void OnViewSharedPropertyChanged( + View* view, + const std::string& name, + const std::vector* old_data, + const std::vector* new_data) override { changes_.push_back(base::StringPrintf( - "view=%s property changed key=%s old_value=%s new_value=%s", - ViewIdToString(view->id()).c_str(), - name.c_str(), - VectorToString(old_data).c_str(), - VectorToString(new_data).c_str())); + "view=%s shared property changed key=%s old_value=%s new_value=%s", + ViewIdToString(view->id()).c_str(), name.c_str(), + VectorToString(old_data).c_str(), VectorToString(new_data).c_str())); } std::string VectorToString(const std::vector* data) { @@ -653,50 +735,110 @@ class PropertyChangeObserver : public ViewObserver { View* view_; Changes changes_; - DISALLOW_COPY_AND_ASSIGN(PropertyChangeObserver); + DISALLOW_COPY_AND_ASSIGN(SharedPropertyChangeObserver); }; } // namespace -TEST_F(ViewObserverTest, SetProperty) { +TEST_F(ViewObserverTest, SetLocalProperty) { TestView v1; std::vector one(1, '1'); { // Change visibility from true to false and make sure we get notifications. - PropertyChangeObserver observer(&v1); - v1.SetProperty("one", &one); + SharedPropertyChangeObserver observer(&v1); + v1.SetSharedProperty("one", &one); Changes changes = observer.GetAndClearChanges(); ASSERT_EQ(1U, changes.size()); - EXPECT_EQ("view=0,1 property changed key=one old_value=NULL new_value=1", - changes[0]); - EXPECT_EQ(1U, v1.properties().size()); + EXPECT_EQ( + "view=0,1 shared property changed key=one old_value=NULL new_value=1", + changes[0]); + EXPECT_EQ(1U, v1.shared_properties().size()); } { // Set visible to existing value and verify no notifications. - PropertyChangeObserver observer(&v1); - v1.SetProperty("one", &one); + SharedPropertyChangeObserver observer(&v1); + v1.SetSharedProperty("one", &one); EXPECT_TRUE(observer.GetAndClearChanges().empty()); - EXPECT_EQ(1U, v1.properties().size()); + EXPECT_EQ(1U, v1.shared_properties().size()); } { // Set the value to NULL to delete it. // Change visibility from true to false and make sure we get notifications. - PropertyChangeObserver observer(&v1); - v1.SetProperty("one", NULL); + SharedPropertyChangeObserver observer(&v1); + v1.SetSharedProperty("one", NULL); Changes changes = observer.GetAndClearChanges(); ASSERT_EQ(1U, changes.size()); - EXPECT_EQ("view=0,1 property changed key=one old_value=1 new_value=NULL", - changes[0]); - EXPECT_EQ(0U, v1.properties().size()); + EXPECT_EQ( + "view=0,1 shared property changed key=one old_value=1 new_value=NULL", + changes[0]); + EXPECT_EQ(0U, v1.shared_properties().size()); } { // Setting a null property to null shouldn't update us. - PropertyChangeObserver observer(&v1); - v1.SetProperty("one", NULL); + SharedPropertyChangeObserver observer(&v1); + v1.SetSharedProperty("one", NULL); EXPECT_TRUE(observer.GetAndClearChanges().empty()); - EXPECT_EQ(0U, v1.properties().size()); + EXPECT_EQ(0U, v1.shared_properties().size()); + } +} + +namespace { + +typedef std::pair PropertyChangeInfo; + +class LocalPropertyChangeObserver : public ViewObserver { + public: + explicit LocalPropertyChangeObserver(View* view) + : view_(view), + property_key_(nullptr), + old_property_value_(-1) { + view_->AddObserver(this); + } + virtual ~LocalPropertyChangeObserver() { view_->RemoveObserver(this); } + + PropertyChangeInfo PropertyChangeInfoAndClear() { + PropertyChangeInfo result(property_key_, old_property_value_); + property_key_ = NULL; + old_property_value_ = -3; + return result; + } + + private: + void OnViewLocalPropertyChanged(View* window, + const void* key, + intptr_t old) override { + property_key_ = key; + old_property_value_ = old; } + + View* view_; + const void* property_key_; + intptr_t old_property_value_; + + DISALLOW_COPY_AND_ASSIGN(LocalPropertyChangeObserver); +}; + +} // namespace + +TEST_F(ViewObserverTest, LocalPropertyChanged) { + TestView v1; + LocalPropertyChangeObserver o(&v1); + + static const ViewProperty prop = {-2}; + + v1.SetLocalProperty(&prop, 1); + EXPECT_EQ(PropertyChangeInfo(&prop, -2), o.PropertyChangeInfoAndClear()); + v1.SetLocalProperty(&prop, -2); + EXPECT_EQ(PropertyChangeInfo(&prop, 1), o.PropertyChangeInfoAndClear()); + v1.SetLocalProperty(&prop, 3); + EXPECT_EQ(PropertyChangeInfo(&prop, -2), o.PropertyChangeInfoAndClear()); + v1.ClearLocalProperty(&prop); + EXPECT_EQ(PropertyChangeInfo(&prop, 3), o.PropertyChangeInfoAndClear()); + + // Sanity check to see if |PropertyChangeInfoAndClear| really clears. + EXPECT_EQ(PropertyChangeInfo( + reinterpret_cast(NULL), -3), o.PropertyChangeInfoAndClear()); } } // namespace mojo diff --git a/mojo/services/public/cpp/view_manager/types.h b/mojo/services/public/cpp/view_manager/types.h index d72236ff00b484..4f246b57dc38d9 100644 --- a/mojo/services/public/cpp/view_manager/types.h +++ b/mojo/services/public/cpp/view_manager/types.h @@ -5,7 +5,7 @@ #ifndef MOJO_SERVICES_PUBLIC_CPP_VIEW_MANAGER_TYPES_H_ #define MOJO_SERVICES_PUBLIC_CPP_VIEW_MANAGER_TYPES_H_ -#include "base/basictypes.h" +#include // Typedefs for the transport types. These typedefs match that of the mojom // file, see it for specifics. diff --git a/mojo/services/public/cpp/view_manager/view.h b/mojo/services/public/cpp/view_manager/view.h index 72f044ce0b1d6e..56d930f8073b36 100644 --- a/mojo/services/public/cpp/view_manager/view.h +++ b/mojo/services/public/cpp/view_manager/view.h @@ -23,14 +23,20 @@ class View; class ViewManager; class ViewObserver; +// Defined in view_property.h (which we do not include) +template +struct ViewProperty; + // Views are owned by the ViewManager. // TODO(beng): Right now, you'll have to implement a ViewObserver to track // destruction and NULL any pointers you have. // Investigate some kind of smart pointer or weak pointer for these. class View { public: - typedef std::vector Children; + using Children = std::vector; + // Creates and returns a new View (which is owned by the ViewManager). Views + // are initially hidden, use SetVisible(true) to show. static View* Create(ViewManager* view_manager); // Destroys this view and all its children. @@ -45,15 +51,45 @@ class View { const Rect& bounds() const { return bounds_; } void SetBounds(const Rect& bounds); - // Visibility (also see IsDrawn()). + // Visibility (also see IsDrawn()). When created views are hidden. bool visible() const { return visible_; } void SetVisible(bool value); - const std::map>& properties() const { + // Returns the set of string to bag of byte properties. These properties are + // shared with the view manager. + const std::map>& shared_properties() const { return properties_; } // Sets a property. If |data| is null, this property is deleted. - void SetProperty(const std::string& name, const std::vector* data); + void SetSharedProperty(const std::string& name, + const std::vector* data); + + // Sets the |value| of the given window |property|. Setting to the default + // value (e.g., NULL) removes the property. The caller is responsible for the + // lifetime of any object set as a property on the View. + // + // These properties are not visible to the view manager. + template + void SetLocalProperty(const ViewProperty* property, T value); + + // Returns the value of the given window |property|. Returns the + // property-specific default value if the property was not previously set. + // + // These properties are only visible in the current process and are not + // shared with other mojo services. + template + T GetLocalProperty(const ViewProperty* property) const; + + // Sets the |property| to its default value. Useful for avoiding a cast when + // setting to NULL. + // + // These properties are only visible in the current process and are not + // shared with other mojo services. + template + void ClearLocalProperty(const ViewProperty* property); + + // Type of a function to delete a property that this view owns. + typedef void (*PropertyDeallocator)(int64 value); // A View is drawn if the View and all its ancestors are visible and the // View is attached to the root. @@ -67,6 +103,7 @@ class View { View* parent() { return parent_; } const View* parent() const { return parent_; } const Children& children() const { return children_; } + const View* GetRoot() const; void AddChild(View* child); void RemoveChild(View* child); @@ -101,6 +138,14 @@ class View { explicit View(ViewManager* manager); + // Called by the public {Set,Get,Clear}Property functions. + int64 SetLocalPropertyInternal(const void* key, + const char* name, + PropertyDeallocator deallocator, + int64 value, + int64 default_value); + int64 GetLocalPropertyInternal(const void* key, int64 default_value) const; + void LocalDestroy(); void LocalAddChild(View* child); void LocalRemoveChild(View* child); @@ -126,6 +171,17 @@ class View { // state. This field is only used if the view has no parent (eg it's a root). bool drawn_; + // Value struct to keep the name and deallocator for this property. + // Key cannot be used for this purpose because it can be char* or + // WindowProperty<>. + struct Value { + const char* name; + int64 value; + PropertyDeallocator deallocator; + }; + + std::map prop_map_; + DISALLOW_COPY_AND_ASSIGN(View); }; diff --git a/mojo/services/public/cpp/view_manager/view_observer.h b/mojo/services/public/cpp/view_manager/view_observer.h index 480f6601620272..fd31c972951078 100644 --- a/mojo/services/public/cpp/view_manager/view_observer.h +++ b/mojo/services/public/cpp/view_manager/view_observer.h @@ -63,10 +63,26 @@ class ViewObserver { virtual void OnViewVisibilityChanging(View* view) {} virtual void OnViewVisibilityChanged(View* view) {} - virtual void OnViewPropertyChanged(View* view, - const std::string& name, - const std::vector* old_data, - const std::vector* new_data) {} + // Invoked when this View's shared properties have changed. This can either + // be caused by SetSharedProperty() being called locally, or by us receiving + // a mojo message that this property has changed. If this property has been + // added, |old_data| is null. If this property was removed, |new_data| is + // null. + virtual void OnViewSharedPropertyChanged( + View* view, + const std::string& name, + const std::vector* old_data, + const std::vector* new_data) {} + + // Invoked when SetProperty() or ClearProperty() is called on the window. + // |key| is either a WindowProperty* (SetProperty, ClearProperty). Either + // way, it can simply be compared for equality with the property + // constant. |old| is the old property value, which must be cast to the + // appropriate type before use. + virtual void OnViewLocalPropertyChanged( + View* view, + const void* key, + intptr_t old) {} virtual void OnViewEmbeddedAppDisconnected(View* view) {} diff --git a/mojo/services/public/cpp/view_manager/view_property.h b/mojo/services/public/cpp/view_manager/view_property.h new file mode 100644 index 00000000000000..11e5e498305b66 --- /dev/null +++ b/mojo/services/public/cpp/view_manager/view_property.h @@ -0,0 +1,140 @@ +// 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 MOJO_SERVICES_PUBLIC_CPP_VIEW_MANAGER_VIEW_PROPERTY_H_ +#define MOJO_SERVICES_PUBLIC_CPP_VIEW_MANAGER_VIEW_PROPERTY_H_ + +#include "base/basictypes.h" + +// This header should be included by code that defines ViewProperties. It +// should not be included by code that only gets and sets ViewProperties. +// +// To define a new ViewProperty: +// +// #include "mojo/services/public/cpp/view_manager/view_property.h" +// +// DECLARE_EXPORTED_VIEW_PROPERTY_TYPE(FOO_EXPORT, MyType); +// namespace foo { +// // Use this to define an exported property that is premitive, +// // or a pointer you don't want automatically deleted. +// DEFINE_VIEW_PROPERTY_KEY(MyType, kMyKey, MyDefault); +// +// // Use this to define an exported property whose value is a heap +// // allocated object, and has to be owned and freed by the view. +// DEFINE_OWNED_VIEW_PROPERTY_KEY(gfx::Rect, kRestoreBoundsKey, nullptr); +// +// // Use this to define a non exported property that is primitive, +// // or a pointer you don't want to automatically deleted, and is used +// // only in a specific file. This will define the property in an unnamed +// // namespace which cannot be accessed from another file. +// DEFINE_LOCAL_VIEW_PROPERTY_KEY(MyType, kMyKey, MyDefault); +// +// } // foo namespace +// +// To define a new type used for ViewProperty. +// +// // outside all namespaces: +// DECLARE_EXPORTED_VIEW_PROPERTY_TYPE(FOO_EXPORT, MyType) +// +// If a property type is not exported, use DECLARE_VIEW_PROPERTY_TYPE(MyType) +// which is a shorthand for DECLARE_EXPORTED_VIEW_PROPERTY_TYPE(, MyType). + +namespace mojo { +namespace { + +// No single new-style cast works for every conversion to/from int64, so we +// need this helper class. A third specialization is needed for bool because +// MSVC warning C4800 (forcing value to bool) is not suppressed by an explicit +// cast (!). +template +class ViewPropertyCaster { + public: + static int64 ToInt64(T x) { return static_cast(x); } + static T FromInt64(int64 x) { return static_cast(x); } +}; +template +class ViewPropertyCaster { + public: + static int64 ToInt64(T* x) { return reinterpret_cast(x); } + static T* FromInt64(int64 x) { return reinterpret_cast(x); } +}; +template <> +class ViewPropertyCaster { + public: + static int64 ToInt64(bool x) { return static_cast(x); } + static bool FromInt64(int64 x) { return x != 0; } +}; + +} // namespace + +template +struct ViewProperty { + T default_value; + const char* name; + View::PropertyDeallocator deallocator; +}; + +template +void View::SetLocalProperty(const ViewProperty* property, T value) { + int64 old = SetLocalPropertyInternal( + property, property->name, + value == property->default_value ? nullptr : property->deallocator, + ViewPropertyCaster::ToInt64(value), + ViewPropertyCaster::ToInt64(property->default_value)); + if (property->deallocator && + old != ViewPropertyCaster::ToInt64(property->default_value)) { + (*property->deallocator)(old); + } +} + +template +T View::GetLocalProperty(const ViewProperty* property) const { + return ViewPropertyCaster::FromInt64(GetLocalPropertyInternal( + property, ViewPropertyCaster::ToInt64(property->default_value))); +} + +template +void View::ClearLocalProperty(const ViewProperty* property) { + SetLocalProperty(property, property->default_value); +} + +} // namespace mojo + +// Macros to instantiate the property getter/setter template functions. +#define DECLARE_EXPORTED_VIEW_PROPERTY_TYPE(EXPORT, T) \ + template EXPORT void mojo::View::SetLocalProperty( \ + const mojo::ViewProperty*, T); \ + template EXPORT T mojo::View::GetLocalProperty(const mojo::ViewProperty*) \ + const; \ + template EXPORT void mojo::View::ClearLocalProperty( \ + const mojo::ViewProperty*); +#define DECLARE_VIEW_PROPERTY_TYPE(T) DECLARE_EXPORTED_VIEW_PROPERTY_TYPE(, T) + +#define DEFINE_VIEW_PROPERTY_KEY(TYPE, NAME, DEFAULT) \ + COMPILE_ASSERT(sizeof(TYPE) <= sizeof(int64), property_type_too_large); \ + namespace { \ + const mojo::ViewProperty NAME##_Value = {DEFAULT, #NAME, nullptr}; \ + } \ + const mojo::ViewProperty* const NAME = &NAME##_Value; + +#define DEFINE_LOCAL_VIEW_PROPERTY_KEY(TYPE, NAME, DEFAULT) \ + COMPILE_ASSERT(sizeof(TYPE) <= sizeof(int64), property_type_too_large); \ + namespace { \ + const mojo::ViewProperty NAME##_Value = {DEFAULT, #NAME, nullptr}; \ + const mojo::ViewProperty* const NAME = &NAME##_Value; \ + } + +#define DEFINE_OWNED_VIEW_PROPERTY_KEY(TYPE, NAME, DEFAULT) \ + namespace { \ + void Deallocator##NAME(int64 p) { \ + enum { type_must_be_complete = sizeof(TYPE) }; \ + delete mojo::ViewPropertyCaster::FromInt64(p); \ + } \ + const mojo::ViewProperty NAME##_Value = {DEFAULT, \ + #NAME, \ + &Deallocator##NAME}; \ + } \ + const mojo::ViewProperty* const NAME = &NAME##_Value; + +#endif // MOJO_SERVICES_PUBLIC_CPP_VIEW_MANAGER_VIEW_PROPERTY_H_ diff --git a/mojo/services/public/interfaces/gpu/command_buffer.mojom b/mojo/services/public/interfaces/gpu/command_buffer.mojom index 2b938c1cbcc359..df4c03607e1177 100644 --- a/mojo/services/public/interfaces/gpu/command_buffer.mojom +++ b/mojo/services/public/interfaces/gpu/command_buffer.mojom @@ -21,9 +21,14 @@ interface CommandBufferSyncClient { DidMakeProgress(CommandBufferState? state); }; +interface CommandBufferSyncPointClient { + DidInsertSyncPoint(uint32 sync_point); +}; + [Client=CommandBufferClient] interface CommandBuffer { Initialize(CommandBufferSyncClient? sync_client, + CommandBufferSyncPointClient? sync_point_client, handle? shared_state); SetGetBuffer(int32 buffer); Flush(int32 put_offset); @@ -31,6 +36,12 @@ interface CommandBuffer { RegisterTransferBuffer( int32 id, handle? transfer_buffer, uint32 size); DestroyTransferBuffer(int32 id); + + // InsertSyncPoint returns the sync point returned via DidInsertSyncPoint. + // If |retire| is true, the sync point is retired on insertion. Otherwise, + // explicitly call RetireSyncPoint to retire it. + InsertSyncPoint(bool retire); + RetireSyncPoint(uint32 sync_point); Echo() => (); // TODO(piman): sync points diff --git a/mojo/services/public/interfaces/view_manager/view_manager.mojom b/mojo/services/public/interfaces/view_manager/view_manager.mojom index 25e1f0686237c6..a79d4c6945ad88 100644 --- a/mojo/services/public/interfaces/view_manager/view_manager.mojom +++ b/mojo/services/public/interfaces/view_manager/view_manager.mojom @@ -191,7 +191,7 @@ interface ViewManagerClient { // Invoked when a view property is changed. If this change is a removal, // |new_data| is null. - OnViewPropertyChanged(uint32 view, string name, array? new_data); + OnViewSharedPropertyChanged(uint32 view, string name, array? new_data); // Invoked when an event is targeted at the specified view. OnViewInputEvent(uint32 view, mojo.Event event) => (); diff --git a/mojo/services/public/interfaces/window_manager/window_manager_internal.mojom b/mojo/services/public/interfaces/window_manager/window_manager_internal.mojom index 54867f9f443360..c453abd926d21b 100644 --- a/mojo/services/public/interfaces/window_manager/window_manager_internal.mojom +++ b/mojo/services/public/interfaces/window_manager/window_manager_internal.mojom @@ -27,4 +27,11 @@ interface WindowManagerInternalClient { // Sets the native viewport size. SetViewportSize(mojo.Size size); + + // Clones the tree rooted at |view_id|. When the animation completes the clone + // is destroyed. + // TODO(sky): add actual animation. + // TODO(sky): I think this only makes sense when destroying (view is + // already visible), should it be named to indicate this? + CloneAndAnimate(uint32 view_id); }; diff --git a/mojo/services/public/js/mojo.js b/mojo/services/public/js/mojo.js index ad0ed0c3a0984b..f6078337e9a212 100644 --- a/mojo/services/public/js/mojo.js +++ b/mojo/services/public/js/mojo.js @@ -60,7 +60,7 @@ define("mojo/services/public/js/mojo", [ var serviceConnection = new connection.Connection( serviceHandle, - provider.service.delegatingStubClass, + provider.service.stubClass, provider.service.client && provider.service.client.proxyClass); serviceConnection.local.connection$ = serviceConnection; @@ -79,7 +79,7 @@ define("mojo/services/public/js/mojo", [ this.handle_ = messagePipeHandle; this.connection_ = new connection.Connection( this.handle_, - service.ServiceProvider.client.delegatingStubClass, + service.ServiceProvider.client.stubClass, service.ServiceProvider.proxyClass); this.connection_.local.delegate$ = { connectToService: connectToServiceImpl.bind(this) @@ -113,7 +113,7 @@ define("mojo/services/public/js/mojo", [ var pipe = core.createMessagePipe(); this.connection_.remote.connectToService(service.name, pipe.handle1); - var clientClass = client && service.client.delegatingStubClass; + var clientClass = client && service.client.stubClass; var serviceConnection = new connection.Connection(pipe.handle0, clientClass, service.proxyClass); if (serviceConnection.local)