Use WidgetGtk as viewport of ScrollView to make scrolling renderers smoother.

 Changed the layout of buttons on notification.
 This fits nicer because a notification can be shorter and menu requires more hight.
Enable double buffer where necessary to avoid flicker.
Fixed a condition to close ControlViewHost.
  It should be closed when renderer is deleted.
Fixed notification_browsertest so that it passes on chromeos device.

Note: changes to native_view_host_gtk and widget_gtk are being done separately in

http://codereview.chromium.org/1725005

so you do not need to review them. I won't check this in unless that one is approved.

BUG=23445,33306
TEST=no functional change. all tests must still pass.

Review URL: http://codereview.chromium.org/1754003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@45395 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 144 insertions, 40 deletions

diff --git a/chrome/browser/chromeos/notifications/balloon_view.cc b/chrome/browser/chromeos/notifications/balloon_view.cc
index a59219f..0a4a93b 100644
--- a/chrome/browser/chromeos/notifications/balloon_view.cc
+++ b/chrome/browser/chromeos/notifications/balloon_view.cc
@@ -85,13 +85,14 @@ class NotificationControlView : public views::View,
     AddChildView(options_menu_button_);
 
     // The control view will never be resized, so just layout once.
-    gfx::Size button_size = close_button_->GetPreferredSize();
-    close_button_->SetBounds(
-        0, 0, button_size.width(), button_size.height());
     gfx::Size options_size = options_menu_button_->GetPreferredSize();
     options_menu_button_->SetBounds(
-        0, button_size.height() + kControlButtonsMargin,
-        options_size.width(), options_size.height());
+        0, 0, options_size.width(), options_size.height());
+
+    gfx::Size button_size = close_button_->GetPreferredSize();
+    close_button_->SetBounds(
+        options_size.width() + kControlButtonsMargin, 0,
+        button_size.width(), button_size.height());
 
     SizeToPreferredSize();
   }
@@ -254,12 +255,13 @@ void BalloonViewImpl::ViewHierarchyChanged(
   if (is_add && GetWidget() && !control_view_host_.get() && controls_) {
     control_view_host_.reset(
         new views::WidgetGtk(views::WidgetGtk::TYPE_CHILD));
+    control_view_host_->EnableDoubleBuffer(true);
     control_view_host_->Init(GetParentNativeView(), gfx::Rect());
     NotificationControlView* control = new NotificationControlView(this);
     control_view_host_->set_delete_on_destroy(false);
     control_view_host_->SetContentsView(control);
   }
-  if (!is_add && GetWidget() && control_view_host_.get() && controls_) {
+  if (!is_add && this == child && control_view_host_.get() && controls_) {
     control_view_host_.release()->CloseNow();
   }
 }
diff --git a/chrome/browser/chromeos/notifications/notification_browsertest.cc b/chrome/browser/chromeos/notifications/notification_browsertest.cc
index a86a720..5301431 100644
--- a/chrome/browser/chromeos/notifications/notification_browsertest.cc
+++ b/chrome/browser/chromeos/notifications/notification_browsertest.cc
@@ -95,7 +95,7 @@ class NotificationTest : public InProcessBrowserTest,
                          PanelController::State state) {
     if (under_chromeos_ && state != state_) {
       expected_ = state;
-      ui_test_utils::RunMessageLoop();
+      ui_test_utils::RunAllPendingInMessageLoop();
     }
   }
 
@@ -360,6 +360,11 @@ IN_PROC_BROWSER_TEST_F(NotificationTest, TestStateTransition2) {
 }
 
 IN_PROC_BROWSER_TEST_F(NotificationTest, TestCleanupOnExit) {
+  NotificationRegistrar registrar;
+  registrar.Add(this,
+                NotificationType::PANEL_STATE_CHANGED,
+                NotificationService::AllSources());
+
   BalloonCollectionImpl* collection = GetBalloonCollectionImpl();
   NotificationPanel* panel = GetNotificationPanel();
   NotificationPanelTester* tester = panel->GetTester();
diff --git a/chrome/browser/chromeos/notifications/notification_panel.cc b/chrome/browser/chromeos/notifications/notification_panel.cc
index 858f46f..97db6ba 100644
--- a/chrome/browser/chromeos/notifications/notification_panel.cc
+++ b/chrome/browser/chromeos/notifications/notification_panel.cc
@@ -13,6 +13,7 @@
 #include "gfx/canvas.h"
 #include "grit/generated_resources.h"
 #include "views/background.h"
+#include "views/controls/native/native_view_host.h"
 #include "views/controls/scroll_view.h"
 #include "views/widget/root_view.h"
 #include "views/widget/widget_gtk.h"
@@ -60,10 +61,43 @@ chromeos::BalloonViewImpl* GetBalloonViewOf(const Balloon* balloon) {
   return static_cast<chromeos::BalloonViewImpl*>(balloon->view());
 }
 
+// A WidgetGtk to preevnt recursive calls to PaintNow, which is observed
+// with gtk 2.18.6. See http://crbug.com/42235 for more details.
 class PanelWidget : public views::WidgetGtk {
  public:
-  explicit PanelWidget(chromeos::NotificationPanel* panel)
-      : WidgetGtk(views::WidgetGtk::TYPE_WINDOW),
+  PanelWidget() : WidgetGtk(TYPE_WINDOW), painting_(false) {
+  }
+
+  virtual ~PanelWidget() {
+    // Enable double buffering because the panel has both pure views control and
+    // native controls (scroll bar).
+    EnableDoubleBuffer(true);
+  }
+
+  // views::WidgetGtk overrides.
+  virtual void PaintNow(const gfx::Rect& update_rect) {
+    if (!painting_) {
+      painting_ = true;
+      WidgetGtk::PaintNow(update_rect);
+      painting_ = false;
+    }
+  }
+
+ private:
+  // True if the painting is in progress.
+  bool painting_;
+
+  DISALLOW_COPY_AND_ASSIGN(PanelWidget);
+};
+
+// A WidgetGtk that covers entire ScrollView's viewport. Without this,
+// all renderer's native gtk widgets are moved one by one via
+// View::VisibleBoundsInRootChanged() notification, which makes
+// scrolling not smooth.
+class ViewportWidget : public views::WidgetGtk {
+ public:
+  explicit ViewportWidget(chromeos::NotificationPanel* panel)
+      : WidgetGtk(views::WidgetGtk::TYPE_CHILD),
         panel_(panel) {
   }
 
@@ -78,6 +112,13 @@ class PanelWidget : public views::WidgetGtk {
 
     int x = 0, y = 0;
     GetContainedWidgetEventCoordinates(event, &x, &y);
+
+    // The window_contents_' allocation has been moved off the top left
+    // corner, so we need to adjust it.
+    GtkAllocation alloc = widget->allocation;
+    x -= alloc.x;
+    y -= alloc.y;
+
     if (!last_point_.get()) {
       last_point_.reset(new gfx::Point(x, y));
     } else {
@@ -105,7 +146,7 @@ class PanelWidget : public views::WidgetGtk {
  private:
   chromeos::NotificationPanel* panel_;
   scoped_ptr<gfx::Point> last_point_;
-  DISALLOW_COPY_AND_ASSIGN(PanelWidget);
+  DISALLOW_COPY_AND_ASSIGN(ViewportWidget);
 };
 
 class BalloonSubContainer : public views::View {
@@ -199,7 +240,7 @@ class BalloonSubContainer : public views::View {
 
   BalloonViewImpl* FindBalloonView(const gfx::Point point) {
     gfx::Point copy(point);
-    ConvertPointToView(GetRootView(), this, &copy);
+    ConvertPointFromWidget(this, &copy);
     for (int i = GetChildViewCount() - 1; i >= 0; --i) {
       views::View* view = GetChildViewAt(i);
       if (view->bounds().Contains(copy))
@@ -228,6 +269,7 @@ class BalloonContainer : public views::View {
     AddChildView(sticky_container_);
     AddChildView(non_sticky_container_);
   }
+  virtual ~BalloonContainer() {}
 
   // views::View overrides.
   virtual void Layout() {
@@ -367,6 +409,8 @@ class BalloonContainer : public views::View {
 
 NotificationPanel::NotificationPanel()
     : balloon_container_(NULL),
+      panel_widget_(NULL),
+      container_host_(NULL),
       state_(CLOSED),
       task_factory_(this),
       min_bounds_(0, 0, kBalloonMinWidth, kBalloonMinHeight),
@@ -384,18 +428,32 @@ NotificationPanel::~NotificationPanel() {
 // NottificationPanel public.
 
 void NotificationPanel::Show() {
-  if (!panel_widget_.get()) {
+  if (!panel_widget_) {
     // TODO(oshima): Using window because Popup widget behaves weird
     // when resizing. This needs to be investigated.
-    panel_widget_.reset(new PanelWidget(this));
+    panel_widget_ = new PanelWidget();
     gfx::Rect bounds = GetPreferredBounds();
     bounds = bounds.Union(min_bounds_);
     panel_widget_->Init(NULL, bounds);
-    // TODO(oshima): I needed the following code in order to get sizing
-    // reliably. Investigate and fix it in WidgetGtk.
+    // Set minimum bounds so that it can grow freely.
     gtk_widget_set_size_request(GTK_WIDGET(panel_widget_->GetNativeView()),
-                                bounds.width(), bounds.height());
+                                min_bounds_.width(), min_bounds_.height());
+
+    views::NativeViewHost* native = new views::NativeViewHost();
+    scroll_view_->SetContents(native);
+
     panel_widget_->SetContentsView(scroll_view_.get());
+
+    // Add the view port after scroll_view is attached to the panel widget.
+    ViewportWidget* widget = new ViewportWidget(this);
+    container_host_ = widget;
+    container_host_->Init(NULL, gfx::Rect());
+    container_host_->SetContentsView(balloon_container_.get());
+    // The window_contents_ is onwed by the WidgetGtk. Increase ref count
+    // so that window_contents does not get deleted when detached.
+    g_object_ref(widget->window_contents());
+    native->Attach(widget->window_contents());
+
     UnregisterNotification();
     panel_controller_.reset(
         new PanelController(this,
@@ -408,14 +466,25 @@ void NotificationPanel::Show() {
 }
 
 void NotificationPanel::Hide() {
-  if (panel_widget_.get()) {
+  if (panel_widget_) {
+    container_host_->GetRootView()->RemoveChildView(balloon_container_.get());
+
+    views::NativeViewHost* native =
+        static_cast<views::NativeViewHost*>(scroll_view_->GetContents());
+    native->Detach();
+    scroll_view_->SetContents(NULL);
+    container_host_->Hide();
+    container_host_->CloseNow();
+    container_host_ = NULL;
+
     UnregisterNotification();
     panel_controller_.release()->Close();
     // We need to remove & detach the scroll view from hierarchy to
     // avoid GTK deleting child.
     // TODO(oshima): handle this details in WidgetGtk.
     panel_widget_->GetRootView()->RemoveChildView(scroll_view_.get());
-    panel_widget_.release()->Close();
+    panel_widget_->Close();
+    panel_widget_ = NULL;
   }
 }
 
@@ -465,8 +534,7 @@ void NotificationPanel::Remove(Balloon* balloon) {
   // no change to the state
   if (state_ == KEEP_SIZE) {
     // Just update the content.
-    balloon_container_->UpdateBounds();
-    scroll_view_->Layout();
+    UpdateContainerBounds();
   } else {
     if (state_ != CLOSED &&
         balloon_container_->GetStickyNewNotificationCount() == 0)
@@ -577,14 +645,14 @@ NotificationPanelTester* NotificationPanel::GetTester() {
 // NotificationPanel private.
 
 void NotificationPanel::Init() {
-  DCHECK(!panel_widget_.get());
-  balloon_container_ = new BalloonContainer(1);
+  DCHECK(!panel_widget_);
+  balloon_container_.reset(new BalloonContainer(1));
+  balloon_container_->set_parent_owned(false);
   balloon_container_->set_background(
       views::Background::CreateSolidBackground(ResourceBundle::frame_color));
 
   scroll_view_.reset(new views::ScrollView());
   scroll_view_->set_parent_owned(false);
-  scroll_view_->SetContents(balloon_container_);
   scroll_view_->set_background(
       views::Background::CreateSolidBackground(SK_ColorWHITE));
 }
@@ -598,15 +666,19 @@ void NotificationPanel::UnregisterNotification() {
 void NotificationPanel::ScrollBalloonToVisible(Balloon* balloon) {
   BalloonViewImpl* view = GetBalloonViewOf(balloon);
   if (!view->closed()) {
-    view->ScrollRectToVisible(gfx::Rect(0, 0, view->width(), view->height()));
+    // We can't use View::ScrollRectToVisible because the viewport is not
+    // ancestor of the BalloonViewImpl.
+    // Use Widget's coordinate which is same as viewport's coordinates.
+    gfx::Point p(0, 0);
+    views::View::ConvertPointToWidget(view, &p);
+    gfx::Rect visible_rect(p.x(), p.y(), view->width(), view->height());
+    scroll_view_->ScrollContentsRegionToBeVisible(visible_rect);
   }
 }
 
-void NotificationPanel::UpdatePanel(bool update_panel_size) {
-  if (update_panel_size) {
-    balloon_container_->UpdateBounds();
-    scroll_view_->Layout();
-  }
+void NotificationPanel::UpdatePanel(bool update_container_size) {
+  if (update_container_size)
+    UpdateContainerBounds();
   switch(state_) {
     case KEEP_SIZE: {
       gfx::Rect min_bounds = GetPreferredBounds();
@@ -614,6 +686,14 @@ void NotificationPanel::UpdatePanel(bool update_panel_size) {
       panel_widget_->GetBounds(&panel_bounds, true);
       if (min_bounds.height() < panel_bounds.height())
         panel_widget_->SetBounds(min_bounds);
+      else if (min_bounds.height() > panel_bounds.height()) {
+        // need scroll bar
+        int width = balloon_container_->width() +
+            scroll_view_->GetScrollBarWidth();
+        panel_bounds.set_width(width);
+        panel_widget_->SetBounds(panel_bounds);
+      }
+
       // no change.
       break;
     }
@@ -627,13 +707,13 @@ void NotificationPanel::UpdatePanel(bool update_panel_size) {
         panel_controller_->SetState(PanelController::MINIMIZED);
       break;
     case FULL:
-      if (panel_widget_.get()) {
+      if (panel_widget_) {
         panel_widget_->SetBounds(GetPreferredBounds());
         panel_controller_->SetState(PanelController::EXPANDED);
       }
       break;
     case STICKY_AND_NEW:
-      if (panel_widget_.get()) {
+      if (panel_widget_) {
         panel_widget_->SetBounds(GetStickyNewBounds());
         panel_controller_->SetState(PanelController::EXPANDED);
       }
@@ -641,9 +721,17 @@ void NotificationPanel::UpdatePanel(bool update_panel_size) {
   }
 }
 
+void NotificationPanel::UpdateContainerBounds() {
+  balloon_container_->UpdateBounds();
+  views::NativeViewHost* native =
+      static_cast<views::NativeViewHost*>(scroll_view_->GetContents());
+  native->SetBounds(balloon_container_->bounds());
+  scroll_view_->Layout();
+}
+
 void NotificationPanel::UpdateControl() {
-  if (panel_widget_.get())
-    static_cast<PanelWidget*>(panel_widget_.get())->UpdateControl();
+  if (container_host_)
+    static_cast<ViewportWidget*>(container_host_)->UpdateControl();
 }
 
 gfx::Rect NotificationPanel::GetPreferredBounds() {
@@ -651,8 +739,9 @@ gfx::Rect NotificationPanel::GetPreferredBounds() {
   int new_height = std::min(pref_size.height(), kMaxPanelHeight);
   int new_width = pref_size.width();
   // Adjust the width to avoid showing a horizontal scroll bar.
-  if (new_height != pref_size.height())
+  if (new_height != pref_size.height()) {
     new_width += scroll_view_->GetScrollBarWidth();
+  }
   return gfx::Rect(0, 0, new_width, new_height).Union(min_bounds_);
 }
 
@@ -745,7 +834,8 @@ BalloonViewImpl* NotificationPanelTester::GetBalloonView(
 bool NotificationPanelTester::IsVisible(const BalloonViewImpl* view) const {
   gfx::Rect rect = panel_->scroll_view_->GetVisibleRect();
   gfx::Point origin(0, 0);
-  views::View::ConvertPointToView(view, panel_->balloon_container_, &origin);
+  views::View::ConvertPointToView(view, panel_->balloon_container_.get(),
+                                  &origin);
   return rect.Contains(gfx::Rect(origin, view->bounds().size()));
 }
 
diff --git a/chrome/browser/chromeos/notifications/notification_panel.h b/chrome/browser/chromeos/notifications/notification_panel.h
index 3f86eec..6a4f512 100644
--- a/chrome/browser/chromeos/notifications/notification_panel.h
+++ b/chrome/browser/chromeos/notifications/notification_panel.h
@@ -125,6 +125,9 @@ class NotificationPanel : public PanelController::Delegate,
   // Scroll the panel so that the |balloon| is visible.
   void ScrollBalloonToVisible(Balloon* balloon);
 
+  // Update the container's bounds so that it can show all notifications.
+  void UpdateContainerBounds();
+
   // Update the notification's control view state.
   void UpdateControl();
 
@@ -153,18 +156,22 @@ class NotificationPanel : public PanelController::Delegate,
     return state_ != CLOSED && state_ != MINIMIZED;
   }
 
-  // Contains all notifications.
-  BalloonContainer* balloon_container_;
+  // Contains all notifications. This is owned by the panel so that we can
+  // re-attach to the widget when closing and opening the panel.
+  scoped_ptr<BalloonContainer> balloon_container_;
+
+  // The notification panel's widget.
+  views::Widget* panel_widget_;
 
   // The notification panel's widget.
-  scoped_ptr<views::Widget> panel_widget_;
+  views::Widget* container_host_;
 
   // Panel controller for the notification panel.
+  // This is owned by the panel to compute the panel size before
+  // actually opening the panel.
   scoped_ptr<PanelController> panel_controller_;
 
   // A scrollable parent of the BalloonContainer.
-  // This is owned by the panel so that we can re-attache to the widget
-  // when closing and opening the panel.
   scoped_ptr<views::ScrollView> scroll_view_;
 
   // Panel's state.