Supporting shrinking/enlarging for notifications

ui/message_center/views/toast_contents_view.cc

Index: ui/message_center/views/toast_contents_view.cc
diff --git a/ui/message_center/views/toast_contents_view.cc b/ui/message_center/views/toast_contents_view.cc
index 0852aca404c43b04e7551063b59681fbf242ec4a..07960e54ad5300c6d43497d7b7a1466948d342f0 100644
--- a/ui/message_center/views/toast_contents_view.cc
+++ b/ui/message_center/views/toast_contents_view.cc
@@ -78,8 +78,7 @@ void ToastContentsView::SetContents(MessageView* view,
   bool already_has_contents = child_count() > 0;
   RemoveAllChildViews(true);
   AddChildView(view);
-  preferred_size_ = GetToastSizeForView(view);
-  Layout();
+  UpdatePreferredSize();
 
   // If it has the contents already, this invocation means an update of the
   // popup toast, and the new contents should be read through a11y feature.
@@ -95,10 +94,7 @@ void ToastContentsView::UpdateContents(const Notification& notification,
   MessageView* message_view = static_cast<MessageView*>(child_at(0));
   message_view->UpdateWithNotification(notification);
   gfx::Size new_size = GetToastSizeForView(message_view);

[Jun Mukai, 2016/05/04 17:00:09]

You can remove this line

[stevenjb, 2016/05/04 23:39:02]

More than just "can" - this will fail to compile with an "unused variable" error
on some platforms.

[dyaroshev, 2016/05/05 14:13:19]

Done.

-  if (preferred_size_ != new_size) {
-    preferred_size_ = new_size;
-    Layout();
-  }
+  UpdatePreferredSize();
   if (a11y_feedback_for_updates)
     NotifyAccessibilityEvent(ui::AX_EVENT_ALERT, false);
 }
@@ -127,23 +123,32 @@ void ToastContentsView::CloseWithAnimation() {
 }
 
 void ToastContentsView::SetBoundsInstantly(gfx::Rect new_bounds) {
-  if (new_bounds == bounds())
-    return;
+  DCHECK(new_bounds.size().width() <= preferred_size_.width() &&
+         new_bounds.size().height() <= preferred_size_.height())
+      << "we can not display widget bigger than notification";
 
-  origin_ = new_bounds.origin();
   if (!GetWidget())
     return;
+
+  if (new_bounds == GetWidget()->GetWindowBoundsInScreen())
+    return;
+
+  origin_ = new_bounds.origin();
   GetWidget()->SetBounds(new_bounds);
 }
 
 void ToastContentsView::SetBoundsWithAnimation(gfx::Rect new_bounds) {
-  if (new_bounds == bounds())
-    return;
+  DCHECK(new_bounds.size().width() <= preferred_size_.width() &&
+         new_bounds.size().height() <= preferred_size_.height())
+      << "we can not display widget bigger than notification";
 
-  origin_ = new_bounds.origin();
   if (!GetWidget())
     return;
 
+  if (new_bounds == GetWidget()->GetWindowBoundsInScreen())

[Jun Mukai, 2016/05/04 17:00:09]

This (and line 133) could be incorrect if this happens during the animation --
you should probably check with animated_bounds_end_?
Or, since now everything is Aura, you can check
GetWidget()->GetNativeWindow()->GetTargetBounds().

[dyaroshev, 2016/05/05 14:13:19]

1) It seems like I can check animated_bounds_end_ here. I cannot do so in
...Instantly, cause we might get a text update, while animation isn't finished,
so the glitch will still be seen. And it's not "instantly". Don't you agree?
2) Does this method mean, that widget knows about his animation? Maybe, we can
get rid of animation_bounds_start_/end_? How I can pull that off?

[Jun Mukai, 2016/05/05 18:01:51]

Sorry, I was misunderstanding the (current) code a bit. This uses
gfx::Animation, compute intermediate size and set it in 
AnimationProgressed(), thus these start_/end_ are actually required.
My point was wrong.

Having said that, I think the former part is still true. Let's think
that notification size change happens very quickly and animation is
still ongoing.
And if the new_bounds happens to be same as the current window
bounds, that means it doesn't stop the animation and goes to the
bounds_end_, which might not be same as new_bounds and result in
unexpected widget size.

Therefore, you will have to compare with bounds_end_ and re-schedule
the animation if new_bounds is not same as bounds_end_.

+    return;
+
+  origin_ = new_bounds.origin();
   // This picks up the current bounds, so if there was a previous animation
   // half-done, the next one will pick up from the current location.
   // This is the only place that should query current location of the Widget
@@ -289,6 +294,16 @@ gfx::Size ToastContentsView::GetPreferredSize() const {
   return child_count() ? GetToastSizeForView(child_at(0)) : gfx::Size();
 }
 
+void ToastContentsView::UpdatePreferredSize() {
+  DCHECK_GT(child_count(), 0);
+  gfx::Size new_size = GetToastSizeForView(child_at(0));
+  if (preferred_size_ == new_size)
+    return;
+  preferred_size_ = new_size;
+  Layout();
+  SetBoundsInstantly(bounds());
+}
+
 void ToastContentsView::GetAccessibleState(ui::AXViewState* state) {
   if (child_count() > 0)
     child_at(0)->GetAccessibleState(state);