Changes view manager to report visibility and drawn state

Visibility tracks whether the view itself is visible and drawn whether
all the views ancestors are visible and connected to the root.

BUG=none
TEST=none
R=ben@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#293949}

mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.cc

@@ -365,6 +365,16 @@ void ViewManagerClientImpl::OnViewDeleted(Id view_id) {
     ViewPrivate(view).LocalDestroy();
 }
 
+void ViewManagerClientImpl::OnViewVisibilityChanged(Id view_id, bool visible) {
+  // TODO(sky): implement me.
+  NOTIMPLEMENTED();
+}
+
+void ViewManagerClientImpl::OnViewDrawnStateChanged(Id view_id, bool drawn) {
+  // TODO(sky): implement me.
+  NOTIMPLEMENTED();
+}
+
 void ViewManagerClientImpl::OnViewInputEvent(
     Id view_id,
     EventPtr event,

mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.h

@@ -105,7 +105,9 @@ class ViewManagerClientImpl : public ViewManager,
                                Id relative_view_id,
                                OrderDirection direction) OVERRIDE;
   virtual void OnViewDeleted(Id view_id) OVERRIDE;
-  virtual void OnViewInputEvent(Id view,
+  virtual void OnViewVisibilityChanged(Id view_id, bool visible) OVERRIDE;
+  virtual void OnViewDrawnStateChanged(Id view_id, bool drawn) OVERRIDE;
+  virtual void OnViewInputEvent(Id view_id,
                                 EventPtr event,
                                 const Callback<void()>& callback) OVERRIDE;
   virtual void Embed(

mojo/services/public/interfaces/view_manager/view_manager.mojom

@@ -13,7 +13,12 @@ struct ViewData {
   uint32 parent_id;
   uint32 view_id;
   mojo.Rect bounds;
-  // TODO(sky): add visible.
+  // True if this view is visible. The view may not be drawn on screen (see
+  // drawn for specifics).
+  bool visible;
+  // True if this view is drawn on screen. A view is drawn if attached to the
+  // root and all ancestors (including this view) are visible.
+  bool drawn;
 };
 
 enum ErrorCode {
@@ -176,6 +181,21 @@ interface ViewManagerClient {
   // Invoked when a view is deleted.
   OnViewDeleted(uint32 view);
 
+  // Invoked when the visibility of the specified view changes.
+  OnViewVisibilityChanged(uint32 view, bool visible);
+
+  // Invoked when a change to the visibility of |view| or one if it's ancestors
+  // is done such that the drawn state changes. This is only invoked for the
+  // top most view of a particular connection. For example, if you have the
+  // hierarchy: A -> B1 -> B2 (B2 is a child of B1 and B1 a child of A), B1/B2
+  // are from connection 2 and A from connection 1 with all views visible and
+  // drawn and the visiblity of A changes to false, then connection 2 is told
+  // the drawn state of B1 has changed (to false), but is not told anything
+  // about B2 as it's drawn state can be calculated from that of B1.
+  //
+  // NOTE: This is not invoked if OnViewVisibilityChanged() is invoked.
+  OnViewDrawnStateChanged(uint32 view, bool drawn);
+
   // Invoked when an event is targeted at the specified view.
   OnViewInputEvent(uint32 view, mojo.Event event) => ();
 

mojo/services/view_manager/connection_manager.cc

@@ -161,6 +161,18 @@ void ConnectionManager::ProcessViewBoundsChanged(const ServerView* view,
   }
 }
 
+void ConnectionManager::ProcessWillChangeViewHierarchy(
+    const ServerView* view,
+    const ServerView* new_parent,
+    const ServerView* old_parent) {
+  for (ConnectionMap::iterator i = connection_map_.begin();
+       i != connection_map_.end();
+       ++i) {
+    i->second->ProcessWillChangeViewHierarchy(
+        view, new_parent, old_parent, IsChangeSource(i->first));
+  }
+}
+
 void ConnectionManager::ProcessViewHierarchyChanged(
     const ServerView* view,
     const ServerView* new_parent,
@@ -238,6 +250,14 @@ void ConnectionManager::OnViewDestroyed(const ServerView* view) {
   ProcessViewDeleted(view->id());
 }
 
+void ConnectionManager::OnWillChangeViewHierarchy(
+    const ServerView* view,
+    const ServerView* new_parent,
+    const ServerView* old_parent) {
+  if (!display_manager_.in_setup())
+    ProcessWillChangeViewHierarchy(view, new_parent, old_parent);
+}
+
 void ConnectionManager::OnViewHierarchyChanged(const ServerView* view,
                                                const ServerView* new_parent,
                                                const ServerView* old_parent) {
@@ -261,5 +281,13 @@ void ConnectionManager::OnViewBitmapChanged(const ServerView* view) {
   display_manager_.SchedulePaint(view, gfx::Rect(view->bounds().size()));
 }
 
+void ConnectionManager::OnWillChangeViewVisibility(const ServerView* view) {
+  for (ConnectionMap::iterator i = connection_map_.begin();
+       i != connection_map_.end();
+       ++i) {
+    i->second->ProcessWillChangeViewVisibility(view, IsChangeSource(i->first));
+  }
+}
+
 }  // namespace service
 }  // namespace mojo

mojo/services/view_manager/connection_manager.h

@@ -125,6 +125,9 @@ class MOJO_VIEW_MANAGER_EXPORT ConnectionManager : public ServerViewDelegate {
   void ProcessViewBoundsChanged(const ServerView* view,
                                 const gfx::Rect& old_bounds,
                                 const gfx::Rect& new_bounds);
+  void ProcessWillChangeViewHierarchy(const ServerView* view,
+                                      const ServerView* new_parent,
+                                      const ServerView* old_parent);
   void ProcessViewHierarchyChanged(const ServerView* view,
                                    const ServerView* new_parent,
                                    const ServerView* old_parent);
@@ -167,6 +170,9 @@ class MOJO_VIEW_MANAGER_EXPORT ConnectionManager : public ServerViewDelegate {
 
   // Overridden from ServerViewDelegate:
   virtual void OnViewDestroyed(const ServerView* view) OVERRIDE;
+  virtual void OnWillChangeViewHierarchy(const ServerView* view,
+                                         const ServerView* new_parent,
+                                         const ServerView* old_parent) OVERRIDE;
   virtual void OnViewHierarchyChanged(const ServerView* view,
                                       const ServerView* new_parent,
                                       const ServerView* old_parent) OVERRIDE;
@@ -174,6 +180,7 @@ class MOJO_VIEW_MANAGER_EXPORT ConnectionManager : public ServerViewDelegate {
                                    const gfx::Rect& old_bounds,
                                    const gfx::Rect& new_bounds) OVERRIDE;
   virtual void OnViewBitmapChanged(const ServerView* view) OVERRIDE;
+  virtual void OnWillChangeViewVisibility(const ServerView* view) OVERRIDE;
 
   Context context_;
 

mojo/services/view_manager/server_view.cc

@@ -37,6 +37,7 @@ void ServerView::Add(ServerView* child) {
   }
 
   const ServerView* old_parent = child->parent();
+  child->delegate_->OnWillChangeViewHierarchy(child, this, old_parent);
   if (child->parent())
     child->parent()->RemoveImpl(child);
 
@@ -51,6 +52,7 @@ void ServerView::Remove(ServerView* child) {
   DCHECK(child != this);
   DCHECK(child->parent() == this);
 
+  child->delegate_->OnWillChangeViewHierarchy(child, NULL, this);
   RemoveImpl(child);
   child->delegate_->OnViewHierarchyChanged(child, NULL, this);
 }
@@ -114,8 +116,17 @@ void ServerView::SetVisible(bool value) {
   if (visible_ == value)
     return;
 
+  delegate_->OnWillChangeViewVisibility(this);
   visible_ = value;
-  // TODO(sky): notification, including repaint.
+}
+
+bool ServerView::IsDrawn(const ServerView* root) const {
+  if (!root->visible_)
+    return false;
+  const ServerView* view = this;
+  while (view && view != root && view->visible_)
+    view = view->parent_;
+  return view == root;
 }
 
 void ServerView::SetBitmap(const SkBitmap& bitmap) {

mojo/services/view_manager/server_view.h

@@ -53,6 +53,7 @@ class MOJO_VIEW_MANAGER_EXPORT ServerView {
   std::vector<const ServerView*> GetChildren() const;
   std::vector<ServerView*> GetChildren();
 
+  // Returns true if this contains |view| or is |view|.
   bool Contains(const ServerView* view) const;
 
   // Returns true if the window is visible. This does not consider visibility
@@ -60,6 +61,10 @@ class MOJO_VIEW_MANAGER_EXPORT ServerView {
   bool visible() const { return visible_; }
   void SetVisible(bool value);
 
+  // Returns true if this view is attached to |root| and all ancestors are
+  // visible.
+  bool IsDrawn(const ServerView* root) const;
+
   void SetBitmap(const SkBitmap& contents);
   const SkBitmap& bitmap() const { return bitmap_; }
 

mojo/services/view_manager/server_view_delegate.h

@@ -22,7 +22,10 @@ class MOJO_VIEW_MANAGER_EXPORT ServerViewDelegate {
   // the hierarchy).
   virtual void OnViewDestroyed(const ServerView* view) = 0;
 
-  // Invoked when the hierarchy has changed.
+  virtual void OnWillChangeViewHierarchy(const ServerView* view,
+                                         const ServerView* new_parent,
+                                         const ServerView* old_parent) = 0;
+
   virtual void OnViewHierarchyChanged(const ServerView* view,
                                       const ServerView* new_parent,
                                       const ServerView* old_parent) = 0;
@@ -33,6 +36,8 @@ class MOJO_VIEW_MANAGER_EXPORT ServerViewDelegate {
 
   virtual void OnViewBitmapChanged(const ServerView* view) = 0;
 
+  virtual void OnWillChangeViewVisibility(const ServerView* view) = 0;
+
  protected:
   virtual ~ServerViewDelegate() {}
 };

mojo/services/view_manager/test_change_tracker.cc

@@ -59,10 +59,21 @@ std::string ChangeToDescription1(const Change& change) {
       return base::StringPrintf("ViewDeleted view=%s",
                                 ViewIdToString(change.view_id).c_str());
 
+    case CHANGE_TYPE_NODE_VISIBILITY_CHANGED:
+      return base::StringPrintf("VisibilityChanged view=%s visible=%s",
+                                ViewIdToString(change.view_id).c_str(),
+                                change.bool_value ? "true" : "false");
+
+    case CHANGE_TYPE_NODE_DRAWN_STATE_CHANGED:
+      return base::StringPrintf("DrawnStateChanged view=%s drawn=%s",
+                                ViewIdToString(change.view_id).c_str(),
+                                change.bool_value ? "true" : "false");
+
     case CHANGE_TYPE_INPUT_EVENT:
       return base::StringPrintf("InputEvent view=%s event_action=%d",
                                 ViewIdToString(change.view_id).c_str(),
                                 change.event_action);
+
     case CHANGE_TYPE_DELEGATE_EMBED:
       return base::StringPrintf("DelegateEmbed url=%s",
                                 change.embed_url.data());
@@ -93,6 +104,8 @@ TestView ViewDataToTestView(const ViewDataPtr& data) {
   TestView view;
   view.parent_id = data->parent_id;
   view.view_id = data->view_id;
+  view.visible = data->visible;
+  view.drawn = data->drawn;
   return view;
 }
 
@@ -109,7 +122,8 @@ Change::Change()
       view_id2(0),
       view_id3(0),
       event_action(0),
-      direction(ORDER_DIRECTION_ABOVE) {
+      direction(ORDER_DIRECTION_ABOVE),
+      bool_value(false) {
 }
 
 Change::~Change() {
@@ -175,6 +189,22 @@ void TestChangeTracker::OnViewDeleted(Id view_id) {
   AddChange(change);
 }
 
+void TestChangeTracker::OnViewVisibilityChanged(Id view_id, bool visible) {
+  Change change;
+  change.type = CHANGE_TYPE_NODE_VISIBILITY_CHANGED;
+  change.view_id = view_id;
+  change.bool_value = visible;
+  AddChange(change);
+}
+
+void TestChangeTracker::OnViewDrawnStateChanged(Id view_id, bool drawn) {
+  Change change;
+  change.type = CHANGE_TYPE_NODE_DRAWN_STATE_CHANGED;
+  change.view_id = view_id;
+  change.bool_value = drawn;
+  AddChange(change);
+}
+
 void TestChangeTracker::OnViewInputEvent(Id view_id, EventPtr event) {
   Change change;
   change.type = CHANGE_TYPE_INPUT_EVENT;
@@ -202,5 +232,13 @@ std::string TestView::ToString() const {
                             ViewIdToString(parent_id).c_str());
 }
 
+std::string TestView::ToString2() const {
+  return base::StringPrintf("view=%s parent=%s visible=%s drawn=%s",
+                            ViewIdToString(view_id).c_str(),
+                            ViewIdToString(parent_id).c_str(),
+                            visible ? "true" : "false",
+                            drawn ? "true" : "false");
+}
+
 }  // namespace service
 }  // namespace mojo

mojo/services/view_manager/test_change_tracker.h

@@ -18,9 +18,12 @@ namespace service {
 
 enum ChangeType {
   CHANGE_TYPE_EMBED,
+  // TODO(sky): NODE->VIEW.
   CHANGE_TYPE_NODE_BOUNDS_CHANGED,
   CHANGE_TYPE_NODE_HIERARCHY_CHANGED,
   CHANGE_TYPE_NODE_REORDERED,
+  CHANGE_TYPE_NODE_VISIBILITY_CHANGED,
+  CHANGE_TYPE_NODE_DRAWN_STATE_CHANGED,
   CHANGE_TYPE_NODE_DELETED,
   CHANGE_TYPE_INPUT_EVENT,
   CHANGE_TYPE_DELEGATE_EMBED,
@@ -31,8 +34,13 @@ struct TestView {
   // Returns a string description of this.
   std::string ToString() const;
 
+  // Returns a string description that includes visible and drawn.
+  std::string ToString2() const;
+
   Id parent_id;
   Id view_id;
+  bool visible;
+  bool drawn;
 };
 
 // Tracks a call to ViewManagerClient. See the individual functions for the
@@ -53,6 +61,7 @@ struct Change {
   String creator_url;
   String embed_url;
   OrderDirection direction;
+  bool bool_value;
 };
 
 // Converts Changes to string descriptions.
@@ -104,6 +113,8 @@ class TestChangeTracker {
                        Id relative_view_id,
                        OrderDirection direction);
   void OnViewDeleted(Id view_id);
+  void OnViewVisibilityChanged(Id view_id, bool visible);
+  void OnViewDrawnStateChanged(Id view_id, bool drawn);
   void OnViewInputEvent(Id view_id, EventPtr event);
   void DelegateEmbed(const String& url);
 

mojo/services/view_manager/view_manager_service_impl.cc

@@ -81,6 +81,23 @@ void ViewManagerServiceImpl::ProcessViewBoundsChanged(
                                 Rect::From(new_bounds));
 }
 
+void ViewManagerServiceImpl::ProcessWillChangeViewHierarchy(
+    const ServerView* view,
+    const ServerView* new_parent,
+    const ServerView* old_parent,
+    bool originated_change) {
+  if (originated_change)
+    return;
+
+  const bool old_drawn = view->IsDrawn(connection_manager_->root());
+  const bool new_drawn = view->visible() && new_parent &&
+      new_parent->IsDrawn(connection_manager_->root());
+  if (old_drawn == new_drawn)
+    return;
+
+  NotifyDrawnStateChanged(view, new_drawn);
+}
+
 void ViewManagerServiceImpl::ProcessViewHierarchyChanged(
     const ServerView* view,
     const ServerView* new_parent,
@@ -142,6 +159,31 @@ void ViewManagerServiceImpl::ProcessViewDeleted(const ViewId& view,
   }
 }
 
+void ViewManagerServiceImpl::ProcessWillChangeViewVisibility(
+    const ServerView* view,
+    bool originated_change) {
+  if (originated_change)
+    return;
+
+  if (IsViewKnown(view)) {
+    client()->OnViewVisibilityChanged(ViewIdToTransportId(view->id()),
+                                      !view->visible());
+    return;
+  }
+
+  bool view_target_drawn_state;
+  if (view->visible()) {
+    // View is being hidden, won't be drawn.
+    view_target_drawn_state = false;
+  } else {
+    // View is being shown. View will be drawn if its parent is drawn.
+    view_target_drawn_state =
+        view->parent() && view->parent()->IsDrawn(connection_manager_->root());
+  }
+
+  NotifyDrawnStateChanged(view, view_target_drawn_state);
+}
+
 void ViewManagerServiceImpl::OnConnectionError() {
   if (delete_on_connection_error_)
     delete this;
@@ -264,6 +306,8 @@ ViewDataPtr ViewManagerServiceImpl::ViewToViewData(const ServerView* view) {
   view_data->parent_id = ViewIdToTransportId(parent ? parent->id() : ViewId());
   view_data->view_id = ViewIdToTransportId(view->id());
   view_data->bounds = Rect::From(view->bounds());
+  view_data->visible = view->visible();
+  view_data->drawn = view->IsDrawn(connection_manager_->root());
   return view_data.Pass();
 }
 
@@ -285,6 +329,21 @@ void ViewManagerServiceImpl::GetViewTreeImpl(
     GetViewTreeImpl(children[i], views);
 }
 
+void ViewManagerServiceImpl::NotifyDrawnStateChanged(const ServerView* view,
+                                                     bool new_drawn_value) {
+  // Even though we don't know about view, it may be an ancestor of one of our
+  // roots, in which case the change may effect our roots drawn state.
+  for (ViewIdSet::iterator i = roots_.begin(); i != roots_.end(); ++i) {
+    const ServerView* root = GetView(ViewIdFromTransportId(*i));
+    DCHECK(root);
+    if (view->Contains(root) &&
+        (new_drawn_value != root->IsDrawn(connection_manager_->root()))) {
+      client()->OnViewDrawnStateChanged(ViewIdToTransportId(root->id()),
+                                        new_drawn_value);
+    }
+  }
+}
+
 void ViewManagerServiceImpl::CreateView(
     Id transport_view_id,
     const Callback<void(ErrorCode)>& callback) {
@@ -414,14 +473,16 @@ void ViewManagerServiceImpl::SetViewVisibility(
     bool visible,
     const Callback<void(bool)>& callback) {
   ServerView* view = GetView(ViewIdFromTransportId(transport_view_id));
-  const bool success = view && view->visible() != visible &&
-                       access_policy_->CanChangeViewVisibility(view);
-  if (success) {
-    DCHECK(view);
+  if (!view || view->visible() == visible ||
+      !access_policy_->CanChangeViewVisibility(view)) {
+    callback.Run(false);
+    return;
+  }
+  {
+    ConnectionManager::ScopedChange change(this, connection_manager_, false);
     view->SetVisible(visible);
   }
-  // TODO(sky): need to notify of visibility changes.
-  callback.Run(success);
+  callback.Run(true);
 }
 
 void ViewManagerServiceImpl::Embed(

mojo/services/view_manager/view_manager_service_impl.h

@@ -77,6 +77,10 @@ class MOJO_VIEW_MANAGER_EXPORT ViewManagerServiceImpl
                                 const gfx::Rect& old_bounds,
                                 const gfx::Rect& new_bounds,
                                 bool originated_change);
+  void ProcessWillChangeViewHierarchy(const ServerView* view,
+                                      const ServerView* new_parent,
+                                      const ServerView* old_parent,
+                                      bool originated_change);
   void ProcessViewHierarchyChanged(const ServerView* view,
                                    const ServerView* new_parent,
                                    const ServerView* old_parent,
@@ -86,6 +90,8 @@ class MOJO_VIEW_MANAGER_EXPORT ViewManagerServiceImpl
                           OrderDirection direction,
                           bool originated_change);
   void ProcessViewDeleted(const ViewId& view, bool originated_change);
+  void ProcessWillChangeViewVisibility(const ServerView* view,
+                                       bool originated_change);
 
   // TODO(sky): move this to private section (currently can't because of
   // bindings).
@@ -136,6 +142,10 @@ class MOJO_VIEW_MANAGER_EXPORT ViewManagerServiceImpl
   void GetViewTreeImpl(const ServerView* view,
                        std::vector<const ServerView*>* views) const;
 
+  // Notify the client if the drawn state of any of the roots changes.
+  // |view| is the view that is changing to the drawn state |new_drawn_value|.
+  void NotifyDrawnStateChanged(const ServerView* view, bool new_drawn_value);
+
   // ViewManagerService:
   virtual void CreateView(Id transport_view_id,
                           const Callback<void(ErrorCode)>& callback) OVERRIDE;

mojo/services/view_manager/view_manager_unittest.cc

@@ -201,6 +201,17 @@ class ViewManagerProxy : public TestChangeTracker::Delegate {
     RunMainLoop();
     return result;
   }
+  bool SetViewVisibility(Id view_id, bool visible) {
+    changes_.clear();
+    bool result = false;
+    view_manager_->SetViewVisibility(
+        view_id,
+        visible,
+        base::Bind(
+            &ViewManagerProxy::GotResult, base::Unretained(this), &result));
+    RunMainLoop();
+    return result;
+  }
 
  private:
   friend class TestViewManagerClientConnection;
@@ -329,6 +340,12 @@ class TestViewManagerClientConnection
     tracker_.OnViewReordered(view_id, relative_view_id, direction);
   }
   virtual void OnViewDeleted(Id view) OVERRIDE { tracker_.OnViewDeleted(view); }
+  virtual void OnViewVisibilityChanged(uint32_t view, bool visible) OVERRIDE {
+    tracker_.OnViewVisibilityChanged(view, visible);
+  }
+  virtual void OnViewDrawnStateChanged(uint32_t view, bool drawn) OVERRIDE {
+    tracker_.OnViewDrawnStateChanged(view, drawn);
+  }
   virtual void OnViewInputEvent(Id view_id,
                                 EventPtr event,
                                 const Callback<void()>& callback) OVERRIDE {
@@ -479,10 +496,9 @@ class ViewManagerTest : public testing::Test {
     ASSERT_EQ(1u, changes.size());
     EXPECT_EQ("OnEmbed creator=mojo:test_url",
               ChangesToDescription1(changes)[0]);
-    if (create_initial_view) {
+    if (create_initial_view)
       EXPECT_EQ("[view=1,1 parent=null]", ChangeViewDescription(changes));
   }
-  }
 
   void EstablishThirdConnection(ViewManagerProxy* owner, Id root_id) {
     ASSERT_TRUE(connection3_ == NULL);
@@ -780,22 +796,24 @@ TEST_F(ViewManagerTest, ViewHierarchyChangedViews) {
 
   // 0,1->1,1->1,2->1,11.
   {
-    // Again, client 2 should not get anything.
-    connection2_->ClearChanges();
+    // Client 2 is now connected to the root, so it should have gotten a drawn
+    // notification.
     ASSERT_TRUE(connection_->AddView(BuildViewId(0, 1), BuildViewId(1, 1)));
-    ASSERT_TRUE(connection2_->CreateView(BuildViewId(2, 101)));
-    connection2_->CopyChangesFromTracker();
-    EXPECT_TRUE(connection2_->changes().empty());
+    connection2_->DoRunLoopUntilChangesCount(1);
+    ASSERT_EQ(1u, connection2_->changes().size());
+    EXPECT_EQ("DrawnStateChanged view=1,1 drawn=true",
+              ChangesToDescription1(connection2_->changes())[0]);
   }
 
   // 1,1->1,2->1,11.
   {
-    connection2_->ClearChanges();
+    // Client 2 is no longer connected to the root, should get drawn state
+    // changed.
     ASSERT_TRUE(connection_->RemoveViewFromParent(BuildViewId(1, 1)));
-    EXPECT_TRUE(connection_->changes().empty());
-    ASSERT_TRUE(connection2_->CreateView(BuildViewId(2, 102)));
-    connection2_->CopyChangesFromTracker();
-    EXPECT_TRUE(connection2_->changes().empty());
+    connection2_->DoRunLoopUntilChangesCount(1);
+    ASSERT_EQ(1u, connection2_->changes().size());
+    EXPECT_EQ("DrawnStateChanged view=1,1 drawn=false",
+              ChangesToDescription1(connection2_->changes())[0]);
   }
 
   // 1,1->1,2->1,11->1,111.
@@ -812,9 +830,10 @@ TEST_F(ViewManagerTest, ViewHierarchyChangedViews) {
   {
     connection2_->ClearChanges();
     ASSERT_TRUE(connection_->AddView(BuildViewId(0, 1), BuildViewId(1, 1)));
-    ASSERT_TRUE(connection2_->CreateView(BuildViewId(2, 104)));
-    connection2_->CopyChangesFromTracker();
-    EXPECT_TRUE(connection2_->changes().empty());
+    connection2_->DoRunLoopUntilChangesCount(1);
+    ASSERT_EQ(1u, connection2_->changes().size());
+    EXPECT_EQ("DrawnStateChanged view=1,1 drawn=true",
+              ChangesToDescription1(connection2_->changes())[0]);
   }
 }
 
@@ -1278,6 +1297,141 @@ TEST_F(ViewManagerTest, EmbedWithSameViewId2) {
   }
 }
 
+// Assertions for SetViewVisibility.
+TEST_F(ViewManagerTest, SetViewVisibility) {
+  // Create 1 and 2 in the first connection and parent both to the root.
+  ASSERT_TRUE(connection_->CreateView(BuildViewId(1, 1)));
+  ASSERT_TRUE(connection_->CreateView(BuildViewId(1, 2)));
+
+  ASSERT_TRUE(connection_->AddView(BuildViewId(0, 1), BuildViewId(1, 1)));
+  {
+    std::vector<TestView> views;
+    connection_->GetViewTree(BuildViewId(0, 1), &views);
+    ASSERT_EQ(2u, views.size());
+    EXPECT_EQ("view=0,1 parent=null visible=true drawn=true",
+              views[0].ToString2());
+    EXPECT_EQ("view=1,1 parent=0,1 visible=true drawn=true",
+              views[1].ToString2());
+  }
+
+  // Hide 1.
+  ASSERT_TRUE(connection_->SetViewVisibility(BuildViewId(1, 1), false));
+  {
+    std::vector<TestView> views;
+    connection_->GetViewTree(BuildViewId(1, 1), &views);
+    ASSERT_EQ(1u, views.size());
+    EXPECT_EQ("view=1,1 parent=0,1 visible=false drawn=false",
+              views[0].ToString2());
+  }
+
+  // Attach 2 to 1.
+  ASSERT_TRUE(connection_->AddView(BuildViewId(1, 1), BuildViewId(1, 2)));
+  {
+    std::vector<TestView> views;
+    connection_->GetViewTree(BuildViewId(1, 1), &views);
+    ASSERT_EQ(2u, views.size());
+    EXPECT_EQ("view=1,1 parent=0,1 visible=false drawn=false",
+              views[0].ToString2());
+    EXPECT_EQ("view=1,2 parent=1,1 visible=true drawn=false",
+              views[1].ToString2());
+  }
+
+  // Show 1.
+  ASSERT_TRUE(connection_->SetViewVisibility(BuildViewId(1, 1), true));
+  {
+    std::vector<TestView> views;
+    connection_->GetViewTree(BuildViewId(1, 1), &views);
+    ASSERT_EQ(2u, views.size());
+    EXPECT_EQ("view=1,1 parent=0,1 visible=true drawn=true",
+              views[0].ToString2());
+    EXPECT_EQ("view=1,2 parent=1,1 visible=true drawn=true",
+              views[1].ToString2());
+  }
+}
+
+// Assertions for SetViewVisibility sending notifications.
+TEST_F(ViewManagerTest, SetViewVisibilityNotifications) {
+  // Create 1,1 and 1,2, 1,2 and child of 1,1 and 1,1 a child of the root.
+  ASSERT_TRUE(connection_->CreateView(BuildViewId(1, 1)));
+  ASSERT_TRUE(connection_->CreateView(BuildViewId(1, 2)));
+  ASSERT_TRUE(connection_->AddView(BuildViewId(0, 1), BuildViewId(1, 1)));
+  ASSERT_TRUE(connection_->AddView(BuildViewId(1, 1), BuildViewId(1, 2)));
+
+  // Establish the second connection at 1,2.
+  ASSERT_NO_FATAL_FAILURE(
+      EstablishSecondConnectionWithRoot(BuildViewId(1, 2)));
+
+  // Add 2,3 as a child of 1,2.
+  ASSERT_TRUE(connection2_->CreateView(BuildViewId(2, 3)));
+  connection_->ClearChanges();
+  ASSERT_TRUE(connection2_->AddView(BuildViewId(1, 2), BuildViewId(2, 3)));
+  connection_->DoRunLoopUntilChangesCount(1);
+
+  // Hide 1,2 from connection 1. Connection 2 should see this.
+  ASSERT_TRUE(connection_->SetViewVisibility(BuildViewId(1, 2), false));
+  {
+    connection2_->DoRunLoopUntilChangesCount(1);
+    ASSERT_EQ(1u, connection2_->changes().size());
+    EXPECT_EQ("VisibilityChanged view=1,2 visible=false",
+              ChangesToDescription1(connection2_->changes())[0]);
+  }
+
+  // Show 1,2 from connection 2, connection 1 should be notified.
+  ASSERT_TRUE(connection2_->SetViewVisibility(BuildViewId(1, 2), true));
+  {
+    connection_->DoRunLoopUntilChangesCount(1);
+    ASSERT_EQ(1u, connection_->changes().size());
+    EXPECT_EQ("VisibilityChanged view=1,2 visible=true",
+              ChangesToDescription1(connection_->changes())[0]);
+  }
+
+  // Hide 1,1, connection 2 should be told the draw state changed.
+  ASSERT_TRUE(connection_->SetViewVisibility(BuildViewId(1, 1), false));
+  {
+    connection2_->DoRunLoopUntilChangesCount(1);
+    ASSERT_EQ(1u, connection2_->changes().size());
+    EXPECT_EQ("DrawnStateChanged view=1,2 drawn=false",
+              ChangesToDescription1(connection2_->changes())[0]);
+  }
+
+  // Show 1,1 from connection 1. Connection 2 should see this.
+  ASSERT_TRUE(connection_->SetViewVisibility(BuildViewId(1, 1), true));
+  {
+    connection2_->DoRunLoopUntilChangesCount(1);
+    ASSERT_EQ(1u, connection2_->changes().size());
+    EXPECT_EQ("DrawnStateChanged view=1,2 drawn=true",
+              ChangesToDescription1(connection2_->changes())[0]);
+  }
+
+  // Change visibility of 2,3, connection 1 should see this.
+  connection_->ClearChanges();
+  ASSERT_TRUE(connection2_->SetViewVisibility(BuildViewId(2, 3), false));
+  {
+    connection_->DoRunLoopUntilChangesCount(1);
+    ASSERT_EQ(1u, connection_->changes().size());
+    EXPECT_EQ("VisibilityChanged view=2,3 visible=false",
+              ChangesToDescription1(connection_->changes())[0]);
+  }
+
+  // Remove 1,1 from the root, connection 2 should see drawn state changed.
+  ASSERT_TRUE(connection_->RemoveViewFromParent(BuildViewId(1, 1)));
+  {
+    connection2_->DoRunLoopUntilChangesCount(1);
+    ASSERT_EQ(1u, connection2_->changes().size());
+    EXPECT_EQ("DrawnStateChanged view=1,2 drawn=false",
+              ChangesToDescription1(connection2_->changes())[0]);
+  }
+
+  // Add 1,1 back to the root, connection 2 should see drawn state changed.
+  ASSERT_TRUE(connection_->AddView(BuildViewId(0, 1), BuildViewId(1, 1)));
+  {
+    connection2_->DoRunLoopUntilChangesCount(1);
+    ASSERT_EQ(1u, connection2_->changes().size());
+    EXPECT_EQ("DrawnStateChanged view=1,2 drawn=true",
+              ChangesToDescription1(connection2_->changes())[0]);
+  }
+}
+
 // TODO(sky): add coverage of test that destroys connections and ensures other
 // connections get deletion notification.