Skip to content

Commit

Permalink
Couple of changes for viewmanager
Browse files Browse the repository at this point in the history 
Removes client_change_id. It isn't needed.
Makes it so notifications are not sent to the connection that
originated the change.

BUG=365012
TEST=covered by tests
R=ben@chromium.org

Review URL: https://codereview.chromium.org/287603002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@270224 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
sky@chromium.org committed May 13, 2014
1 parent a9e82a2 commit 94e20d4
Show file tree
Hide file tree
Showing 8 changed files with 298 additions and 389 deletions.
133 changes: 51 additions & 82 deletions mojo/services/public/cpp/view_manager/lib/view_manager_synchronizer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ class ViewManagerTransaction {
}

bool committed() const { return committed_; }
TransportChangeId client_change_id() const { return client_change_id_; }

// General callback to be used for commits to the service.
void OnActionCompleted(bool success) {
Expand All @@ -89,7 +88,6 @@ class ViewManagerTransaction {
ViewManagerTransaction(TransactionType transaction_type,
ViewManagerSynchronizer* synchronizer)
: transaction_type_(transaction_type),
client_change_id_(synchronizer->GetNextClientChangeId()),
committed_(false),
synchronizer_(synchronizer) {
}
Expand All @@ -109,7 +107,6 @@ class ViewManagerTransaction {

private:
const TransactionType transaction_type_;
const uint32_t client_change_id_;
bool committed_;
ViewManagerSynchronizer* synchronizer_;

Expand Down Expand Up @@ -154,7 +151,6 @@ class DestroyViewTransaction : public ViewManagerTransaction {
virtual void DoCommit() OVERRIDE {
service()->DeleteView(
view_id_,
client_change_id(),
base::Bind(&ViewManagerTransaction::OnActionCompleted,
base::Unretained(this)));
}
Expand Down Expand Up @@ -208,7 +204,6 @@ class DestroyViewTreeNodeTransaction : public ViewManagerTransaction {
virtual void DoCommit() OVERRIDE {
service()->DeleteNode(
node_id_,
client_change_id(),
base::Bind(&ViewManagerTransaction::OnActionCompleted,
base::Unretained(this)));
}
Expand Down Expand Up @@ -245,15 +240,13 @@ class HierarchyTransaction : public ViewManagerTransaction {
parent_id_,
child_id_,
GetAndAdvanceNextServerChangeId(),
client_change_id(),
base::Bind(&ViewManagerTransaction::OnActionCompleted,
base::Unretained(this)));
break;
case TYPE_REMOVE:
service()->RemoveNodeFromParent(
child_id_,
GetAndAdvanceNextServerChangeId(),
client_change_id(),
base::Bind(&ViewManagerTransaction::OnActionCompleted,
base::Unretained(this)));
break;
Expand Down Expand Up @@ -288,7 +281,6 @@ class SetActiveViewTransaction : public ViewManagerTransaction {
service()->SetView(
node_id_,
view_id_,
client_change_id(),
base::Bind(&ViewManagerTransaction::OnActionCompleted,
base::Unretained(this)));
}
Expand All @@ -307,7 +299,6 @@ ViewManagerSynchronizer::ViewManagerSynchronizer(ViewManager* view_manager)
connected_(false),
connection_id_(0),
next_id_(1),
next_client_change_id_(0),
next_server_change_id_(0),
sync_factory_(this),
init_loop_(NULL) {
Expand Down Expand Up @@ -414,80 +405,67 @@ void ViewManagerSynchronizer::OnNodeHierarchyChanged(
uint32_t node_id,
uint32_t new_parent_id,
uint32_t old_parent_id,
TransportChangeId server_change_id,
TransportChangeId client_change_id) {
if (client_change_id == 0) {
// Change originated from another client.
ViewTreeNode* new_parent =
view_manager_->tree()->GetChildById(new_parent_id);
ViewTreeNode* old_parent =
view_manager_->tree()->GetChildById(old_parent_id);
ViewTreeNode* node = NULL;
if (old_parent) {
// Existing node, mapped in this connection's tree.
// TODO(beng): verify this is actually true.
node = view_manager_->GetNodeById(node_id);
DCHECK_EQ(node->parent(), old_parent);
} else {
// New node, originating from another connection.
node = ViewTreeNodePrivate::LocalCreate();
ViewTreeNodePrivate private_node(node);
private_node.set_view_manager(view_manager_);
private_node.set_id(node_id);
ViewManagerPrivate(view_manager_).AddNode(node->id(), node);

// TODO(beng): view changes.
}
if (new_parent)
ViewTreeNodePrivate(new_parent).LocalAddChild(node);
else
ViewTreeNodePrivate(old_parent).LocalRemoveChild(node);
}
TransportChangeId server_change_id) {
next_server_change_id_ = server_change_id + 1;
}

void ViewManagerSynchronizer::OnNodeViewReplaced(uint32_t node_id,
uint32_t new_view_id,
uint32_t old_view_id,
uint32_t client_change_id) {
if (client_change_id == 0) {
ViewTreeNode* node = view_manager_->GetNodeById(node_id);
View* new_view = view_manager_->GetViewById(new_view_id);
if (!new_view && new_view_id != 0) {
// This client wasn't aware of this View until now.
new_view = ViewPrivate::LocalCreate();
ViewPrivate private_view(new_view);
private_view.set_view_manager(view_manager_);
private_view.set_id(new_view_id);
private_view.set_node(node);
ViewManagerPrivate(view_manager_).AddView(new_view->id(), new_view);
}
View* old_view = view_manager_->GetViewById(old_view_id);
DCHECK_EQ(old_view, node->active_view());
ViewTreeNodePrivate(node).LocalSetActiveView(new_view);
ViewTreeNode* new_parent =
view_manager_->tree()->GetChildById(new_parent_id);
ViewTreeNode* old_parent =
view_manager_->tree()->GetChildById(old_parent_id);
ViewTreeNode* node = NULL;
if (old_parent) {
// Existing node, mapped in this connection's tree.
// TODO(beng): verify this is actually true.
node = view_manager_->GetNodeById(node_id);
DCHECK_EQ(node->parent(), old_parent);
} else {
// New node, originating from another connection.
node = ViewTreeNodePrivate::LocalCreate();
ViewTreeNodePrivate private_node(node);
private_node.set_view_manager(view_manager_);
private_node.set_id(node_id);
ViewManagerPrivate(view_manager_).AddNode(node->id(), node);

// TODO(beng): view changes.
}
if (new_parent)
ViewTreeNodePrivate(new_parent).LocalAddChild(node);
else
ViewTreeNodePrivate(old_parent).LocalRemoveChild(node);
}

void ViewManagerSynchronizer::OnNodeDeleted(uint32_t node_id,
uint32_t server_change_id,
uint32_t client_change_id) {
uint32_t server_change_id) {
next_server_change_id_ = server_change_id + 1;
if (client_change_id == 0) {
ViewTreeNode* node = view_manager_->GetNodeById(node_id);
if (node)
ViewTreeNodePrivate(node).LocalDestroy();
}

ViewTreeNode* node = view_manager_->GetNodeById(node_id);
if (node)
ViewTreeNodePrivate(node).LocalDestroy();
}

void ViewManagerSynchronizer::OnViewDeleted(uint32_t view_id,
uint32_t server_change_id,
uint32_t client_change_id) {
next_server_change_id_ = server_change_id + 1;
if (client_change_id == 0) {
View* view = view_manager_->GetViewById(view_id);
if (view)
ViewPrivate(view).LocalDestroy();
void ViewManagerSynchronizer::OnNodeViewReplaced(uint32_t node_id,
uint32_t new_view_id,
uint32_t old_view_id) {
ViewTreeNode* node = view_manager_->GetNodeById(node_id);
View* new_view = view_manager_->GetViewById(new_view_id);
if (!new_view && new_view_id != 0) {
// This client wasn't aware of this View until now.
new_view = ViewPrivate::LocalCreate();
ViewPrivate private_view(new_view);
private_view.set_view_manager(view_manager_);
private_view.set_id(new_view_id);
private_view.set_node(node);
ViewManagerPrivate(view_manager_).AddView(new_view->id(), new_view);
}
View* old_view = view_manager_->GetViewById(old_view_id);
DCHECK_EQ(old_view, node->active_view());
ViewTreeNodePrivate(node).LocalSetActiveView(new_view);
}

void ViewManagerSynchronizer::OnViewDeleted(uint32_t view_id) {
View* view = view_manager_->GetViewById(view_id);
if (view)
ViewPrivate(view).LocalDestroy();
}

////////////////////////////////////////////////////////////////////////////////
Expand All @@ -506,15 +484,6 @@ void ViewManagerSynchronizer::Sync() {
}
}

uint32_t ViewManagerSynchronizer::GetNextClientChangeId() {
// TODO(beng): deal with change id collisions? Important in the "never ack'ed
// change" case mentioned in OnNodeHierarchyChanged().
// "0" is a special value passed to other connected clients, so we can't use
// it.
next_client_change_id_ = std::max(1u, next_client_change_id_ + 1);
return next_client_change_id_;
}

void ViewManagerSynchronizer::RemoveFromPendingQueue(
ViewManagerTransaction* transaction) {
DCHECK_EQ(transaction, pending_transactions_.front());
Expand Down
20 changes: 4 additions & 16 deletions mojo/services/public/cpp/view_manager/lib/view_manager_synchronizer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,29 +60,18 @@ class ViewManagerSynchronizer : public IViewManagerClient {
uint32 node_id,
uint32 new_parent_id,
uint32 old_parent_id,
TransportChangeId server_change_id,
TransportChangeId client_change_id) OVERRIDE;
TransportChangeId server_change_id) OVERRIDE;
virtual void OnNodeDeleted(TransportNodeId node_id,
TransportChangeId server_change_id,
TransportChangeId client_change_id) OVERRIDE;
TransportChangeId server_change_id) OVERRIDE;
virtual void OnNodeViewReplaced(uint32_t node,
uint32_t new_view_id,
uint32_t old_view_id,
TransportChangeId client_change_id) OVERRIDE;
virtual void OnViewDeleted(uint32_t node_id,
uint32_t server_change_id,
uint32_t client_change_id) OVERRIDE;
uint32_t old_view_id) OVERRIDE;
virtual void OnViewDeleted(uint32_t view_id) OVERRIDE;

// Sync the client model with the service by enumerating the pending
// transaction queue and applying them in order.
void Sync();

// Used by individual transactions to generate a connection-specific change
// id.
// TODO(beng): What happens when there are more than sizeof(int) changes in
// the queue?
TransportChangeId GetNextClientChangeId();

// Removes |transaction| from the pending queue. |transaction| must be at the
// front of the queue.
void RemoveFromPendingQueue(ViewManagerTransaction* transaction);
Expand All @@ -93,7 +82,6 @@ class ViewManagerSynchronizer : public IViewManagerClient {
bool connected_;
TransportConnectionId connection_id_;
uint16_t next_id_;
TransportChangeId next_client_change_id_;
TransportChangeId next_server_change_id_;

Transactions pending_transactions_;
Expand Down
48 changes: 19 additions & 29 deletions mojo/services/public/interfaces/view_manager/view_manager.mojom
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,11 @@ struct INode {
uint32 view_id;
};

// Functions that mutate the hierarchy take two change ids:
// |server_change_id|: this is an ever increasing integer used to identify the
// change. Every hierarchy change increases this value. The server only accepts
// changes where the supplied |server_change_id| matches the expected next
// value. This ensures changes are made in a well defined order. The client is
// told the |server_change_id| when the connection is initially established and
// subsequentely in any changes.
//
// |client_change_id|: this id is for use by the client to further identify the
// change. This value is supplied in the mutation notifications to clients.
// Connections that did not originate the change get a value of 0.
// Functions that mutate the hierarchy take a change id. This is an ever
// increasing integer used to identify the change. Every hierarchy change
// increases this value. The server only accepts changes where the supplied
// |server_change_id| matches the expected next value. This ensures changes are
// made in a well defined order.
//
// Nodes and Views are identified by a uint32. The upper 16 bits are the
// connection id, and the lower 16 the id assigned by the client. Functions
Expand All @@ -36,8 +30,8 @@ interface IViewManager {

// Deletes a node. This does not recurse. Children are removed from the node
// before it is destroyed. Deletion is always allowed and implicitly advances
// the server_change_id.
DeleteNode(uint32 node_id, uint32 change_id) => (bool success);
// |server_change_id|.
DeleteNode(uint32 node_id) => (bool success);

// Reparents a node. See description above class for details of |change_id|.
// This fails for any of the following reasons:
Expand All @@ -47,15 +41,13 @@ interface IViewManager {
// . |child| is already a child of |parent|.
AddNode(uint32 parent,
uint32 child,
uint32 server_change_id,
uint32 client_change_id) => (bool success);
uint32 server_change_id) => (bool success);

// Removes a view from its current parent. See description above class for
// details of |change_id|. This fails if the node is not valid,
// |server_change_id| doesn't match, or the node already has no parent.
RemoveNodeFromParent(uint32 node_id,
uint32 server_change_id,
uint32 client_change_id) => (bool success);
uint32 server_change_id) => (bool success);

// Returns the nodes comprising the tree starting at |node_id|. |node_id| is
// the first result in the return value, unless |node_id| is invalid, in which
Expand All @@ -68,46 +60,44 @@ interface IViewManager {
CreateView(uint16 view_id) => (bool success);

// Deletes the view with the specified id.
DeleteView(uint32 view_id, uint32 change_id) => (bool success);
DeleteView(uint32 view_id) => (bool success);

// Sets the view a node is showing.
SetView(uint32 node_id, uint32 view_id, uint32 change_id) => (bool success);
SetView(uint32 node_id, uint32 view_id) => (bool success);

// Shows the specified image (png encoded) in the specified view.
SetViewContents(uint32 view_id,
handle<shared_buffer> buffer,
uint32 buffer_size);
};

// Changes to nodes/views are not sent to the connection that originated the
// change. For example, if connection 1 attaches a view to a node (SetView())
// connection 1 does not receive OnNodeViewReplaced().
interface IViewManagerClient {
// Invoked once the connection has been established. |connection_id| is the id
// that uniquely identifies this connection. |next_server_change_id| is the
// id of the next change the server is expecting.
OnConnectionEstablished(uint16 connection_id,
uint32 next_server_change_id);
OnConnectionEstablished(uint16 connection_id, uint32 next_server_change_id);

// Invoked when a change is done to the hierarchy. A value of 0 is used to
// identify a null node. For example, if the old_parent is NULL, 0 is
// supplied. See description above ViewManager for details on the change ids.
OnNodeHierarchyChanged(uint32 node,
uint32 new_parent,
uint32 old_parent,
uint32 server_change_id,
uint32 client_change_id);
uint32 server_change_id);

// Invoked when a node is deleted.
OnNodeDeleted(uint32 node, uint32 server_change_id, uint32 client_change_id);
OnNodeDeleted(uint32 node, uint32 server_change_id);


// Invoked when the view associated with a node is replaced by another view.
// 0 is used to identify a null view.
OnNodeViewReplaced(uint32 node,
uint32 new_view_id,
uint32 old_view_id,
uint32 client_change_id);
OnNodeViewReplaced(uint32 node, uint32 new_view_id, uint32 old_view_id);

// Invoked when a view is deleted.
OnViewDeleted(uint32 view, uint32 server_change_id, uint32 client_change_id);
OnViewDeleted(uint32 view);
};

}
Loading

0 comments on commit 94e20d4

Please sign in to comment.