Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1277)

Unified Diff: mojo/services/public/cpp/view_manager/lib/view_tree_node.cc

Issue 296133012: Some security checks around destruction/setting bounds. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 6 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: mojo/services/public/cpp/view_manager/lib/view_tree_node.cc
diff --git a/mojo/services/public/cpp/view_manager/lib/view_tree_node.cc b/mojo/services/public/cpp/view_manager/lib/view_tree_node.cc
index 66331a72c39ff33bf4d7c1c0a339fe8383e42678..715d799415761709accc66d3c6a5033503deb311 100644
--- a/mojo/services/public/cpp/view_manager/lib/view_tree_node.cc
+++ b/mojo/services/public/cpp/view_manager/lib/view_tree_node.cc
@@ -180,6 +180,12 @@ class ScopedDestructionNotifier {
DISALLOW_COPY_AND_ASSIGN(ScopedDestructionNotifier);
};
+// Some operations are only permitted in the connection that created the node.
+bool CanMutateNode(ViewManager* manager, ViewTreeNode* node) {
sky 2014/05/23 13:16:29 Maybe this should be named OwnedNode since some mu
+ return !manager ||
+ ViewManagerPrivate(manager).synchronizer()->OwnsNode(node->id());
+}
+
} // namespace
////////////////////////////////////////////////////////////////////////////////
@@ -193,7 +199,9 @@ ViewTreeNode* ViewTreeNode::Create(ViewManager* view_manager) {
}
void ViewTreeNode::Destroy() {
- // TODO(beng): only proceed if |manager_| OwnsNode(this).
+ if (!CanMutateNode(manager_, this))
+ return;
+
if (manager_)
ViewManagerPrivate(manager_).synchronizer()->DestroyViewTreeNode(id_);
while (!children_.empty())
@@ -202,7 +210,9 @@ void ViewTreeNode::Destroy() {
}
void ViewTreeNode::SetBounds(const gfx::Rect& bounds) {
- // TODO(beng): only proceed if |manager_| OwnsNode(this).
+ if (!CanMutateNode(manager_, this))
+ return;
+
if (manager_)
ViewManagerPrivate(manager_).synchronizer()->SetBounds(id_, bounds);
LocalSetBounds(bounds_, bounds);
@@ -217,6 +227,8 @@ void ViewTreeNode::RemoveObserver(ViewTreeNodeObserver* observer) {
}
void ViewTreeNode::AddChild(ViewTreeNode* child) {
+ // TODO(beng): not necessarily valid to all connections, but possibly to the
+ // embeddee in an embedder-embeddee relationship.
if (manager_)
CHECK_EQ(ViewTreeNodePrivate(child).view_manager(), manager_);
LocalAddChild(child);
@@ -225,6 +237,8 @@ void ViewTreeNode::AddChild(ViewTreeNode* child) {
}
void ViewTreeNode::RemoveChild(ViewTreeNode* child) {
+ // TODO(beng): not necessarily valid to all connections, but possibly to the
+ // embeddee in an embedder-embeddee relationship.
if (manager_)
CHECK_EQ(ViewTreeNodePrivate(child).view_manager(), manager_);
LocalRemoveChild(child);
@@ -256,6 +270,8 @@ ViewTreeNode* ViewTreeNode::GetChildById(TransportNodeId id) {
}
void ViewTreeNode::SetActiveView(View* view) {
+ // TODO(beng): not necessarily valid to all connections, but possibly to the
+ // embeddee in an embedder-embeddee relationship.
if (manager_)
CHECK_EQ(ViewPrivate(view).view_manager(), manager_);
LocalSetActiveView(view);

Powered by Google App Engine
This is Rietveld 408576698