Bug 1102039, Ensure that the popup's widget visibility is updated after rollup, allows popups to disappear at the start of the minimize animation rather than after, r=tn
authorNeil Deakin <neil@mozilla.com>
Wed, 07 Jan 2015 20:52:20 -0500
changeset 235609 bb5356f6a251c4493ebd985cc9dfd815fd689871
parent 235608 e38e801acf7b8772d9bb11f33654e5ce83fd0c3d
child 235610 83f3a7efed212ef4e7d1acf58cccce2e91f44880
push id366
push usercmanchester@mozilla.com
push dateThu, 08 Jan 2015 16:40:24 +0000
reviewerstn
bugs1102039
milestone37.0a1
Bug 1102039, Ensure that the popup's widget visibility is updated after rollup, allows popups to disappear at the start of the minimize animation rather than after, r=tn
layout/forms/nsComboboxControlFrame.cpp
layout/forms/nsComboboxControlFrame.h
layout/xul/nsXULPopupManager.cpp
layout/xul/nsXULPopupManager.h
widget/cocoa/nsAppShell.mm
widget/cocoa/nsChildView.mm
widget/cocoa/nsCocoaWindow.mm
widget/cocoa/nsMenuX.mm
widget/cocoa/nsToolkit.mm
widget/gtk/nsWindow.cpp
widget/nsIRollupListener.h
widget/qt/nsWindow.cpp
widget/windows/nsWindow.cpp
--- a/layout/forms/nsComboboxControlFrame.cpp
+++ b/layout/forms/nsComboboxControlFrame.cpp
@@ -1387,17 +1387,18 @@ nsComboboxControlFrame::SetInitialChildL
     nsBlockFrame::SetInitialChildList(aListID, aChildList);
   }
 }
 
 //----------------------------------------------------------------------
   //nsIRollupListener
 //----------------------------------------------------------------------
 bool
-nsComboboxControlFrame::Rollup(uint32_t aCount, const nsIntPoint* pos, nsIContent** aLastRolledUp)
+nsComboboxControlFrame::Rollup(uint32_t aCount, bool aFlush,
+                               const nsIntPoint* pos, nsIContent** aLastRolledUp)
 {
   if (!mDroppedDown) {
     return false;
   }
 
   bool consume = true;
 #ifdef XP_WIN
   consume = false;
@@ -1406,16 +1407,24 @@ nsComboboxControlFrame::Rollup(uint32_t 
   mListControlFrame->AboutToRollup(); // might destroy us
   if (!weakFrame.IsAlive()) {
     return consume;
   }
   ShowDropDown(false); // might destroy us
   if (weakFrame.IsAlive()) {
     mListControlFrame->CaptureMouseEvents(false);
   }
+
+  if (aFlush && weakFrame.IsAlive()) {
+    // The popup's visibility doesn't update until the minimize animation has
+    // finished, so call UpdateWidgetGeometry to update it right away.
+    nsViewManager* viewManager = mDropdownFrame->GetView()->GetViewManager();
+    viewManager->UpdateWidgetGeometry();
+  }
+
   return consume;
 }
 
 nsIWidget*
 nsComboboxControlFrame::GetRollupWidget()
 {
   nsView* view = mDropdownFrame->GetView();
   MOZ_ASSERT(view);
--- a/layout/forms/nsComboboxControlFrame.h
+++ b/layout/forms/nsComboboxControlFrame.h
@@ -161,17 +161,18 @@ public:
   NS_IMETHOD OnOptionSelected(int32_t aIndex, bool aSelected) MOZ_OVERRIDE;
   NS_IMETHOD OnSetSelectedIndex(int32_t aOldIndex, int32_t aNewIndex) MOZ_OVERRIDE;
 
   //nsIRollupListener
   /**
    * Hide the dropdown menu and stop capturing mouse events.
    * @note This method might destroy |this|.
    */
-  virtual bool Rollup(uint32_t aCount, const nsIntPoint* pos, nsIContent** aLastRolledUp) MOZ_OVERRIDE;
+  virtual bool Rollup(uint32_t aCount, bool aFlush,
+                      const nsIntPoint* pos, nsIContent** aLastRolledUp) MOZ_OVERRIDE;
   virtual void NotifyGeometryChange() MOZ_OVERRIDE;
 
   /**
    * A combobox should roll up if a mousewheel event happens outside of
    * the popup area.
    */
   virtual bool ShouldRollupOnMouseWheelEvent() MOZ_OVERRIDE
     { return true; }
--- a/layout/xul/nsXULPopupManager.cpp
+++ b/layout/xul/nsXULPopupManager.cpp
@@ -175,17 +175,18 @@ nsXULPopupManager::Observe(nsISupports *
 nsXULPopupManager*
 nsXULPopupManager::GetInstance()
 {
   MOZ_ASSERT(sInstance);
   return sInstance;
 }
 
 bool
-nsXULPopupManager::Rollup(uint32_t aCount, const nsIntPoint* pos, nsIContent** aLastRolledUp)
+nsXULPopupManager::Rollup(uint32_t aCount, bool aFlush,
+                          const nsIntPoint* pos, nsIContent** aLastRolledUp)
 {
   bool consume = false;
 
   nsMenuChainItem* item = GetTopVisibleMenu();
   if (item) {
     if (aLastRolledUp) {
       // we need to get the popup that will be closed last, so that
       // widget can keep track of it so it doesn't reopen if a mouse
@@ -243,17 +244,26 @@ nsXULPopupManager::Rollup(uint32_t aCoun
       while (--aCount && last->GetParent()) {
         last = last->GetParent();
       }
       if (last) {
         lastPopup = last->Content();
       }
     }
 
+    nsPresContext* presContext = item->Frame()->PresContext();
+    nsRefPtr<nsViewManager> viewManager = presContext->PresShell()->GetViewManager();
+
     HidePopup(item->Content(), true, true, false, true, lastPopup);
+
+    if (aFlush) {
+      // The popup's visibility doesn't update until the minimize animation has
+      // finished, so call UpdateWidgetGeometry to update it right away.
+      viewManager->UpdateWidgetGeometry();
+    }
   }
 
   return consume;
 }
 
 ////////////////////////////////////////////////////////////////////////
 bool nsXULPopupManager::ShouldRollupOnMouseWheelEvent()
 {
@@ -2155,17 +2165,17 @@ nsXULPopupManager::HandleKeyboardEventWi
       break;
 
     case nsIDOMKeyEvent::DOM_VK_TAB:
 #ifndef XP_MACOSX
     case nsIDOMKeyEvent::DOM_VK_F10:
 #endif
       // close popups or deactivate menubar when Tab or F10 are pressed
       if (aTopVisibleMenuItem) {
-        Rollup(0, nullptr, nullptr);
+        Rollup(0, false, nullptr, nullptr);
       } else if (mActiveMenuBar) {
         mActiveMenuBar->MenuClosed();
       }
       break;
 
     case nsIDOMKeyEvent::DOM_VK_RETURN: {
       // If there is a popup open, check if the current item needs to be opened.
       // Otherwise, tell the active menubar, if any, to activate the menu. The
@@ -2433,17 +2443,17 @@ nsXULPopupManager::KeyDown(nsIDOMKeyEven
         aKeyEvent->GetShiftKey(&shift);
       bool meta=false;
       if (menuAccessKey != nsIDOMKeyEvent::DOM_VK_META)
         aKeyEvent->GetMetaKey(&meta);
       if (!(ctrl || alt || shift || meta)) {
         // The access key just went down and no other
         // modifiers are already down.
         if (mPopups)
-          Rollup(0, nullptr, nullptr);
+          Rollup(0, false, nullptr, nullptr);
         else if (mActiveMenuBar)
           mActiveMenuBar->MenuClosed();
       }
       aKeyEvent->PreventDefault();
     }
   }
 
   // Since a menu was open, stop propagation of the event to keep other event
--- a/layout/xul/nsXULPopupManager.h
+++ b/layout/xul/nsXULPopupManager.h
@@ -283,17 +283,18 @@ public:
   friend class TransitionEnder;
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSIOBSERVER
   NS_DECL_NSITIMERCALLBACK
   NS_DECL_NSIDOMEVENTLISTENER
 
   // nsIRollupListener
-  virtual bool Rollup(uint32_t aCount, const nsIntPoint* pos, nsIContent** aLastRolledUp) MOZ_OVERRIDE;
+  virtual bool Rollup(uint32_t aCount, bool aFlush,
+                      const nsIntPoint* pos, nsIContent** aLastRolledUp) MOZ_OVERRIDE;
   virtual bool ShouldRollupOnMouseWheelEvent() MOZ_OVERRIDE;
   virtual bool ShouldConsumeOnMouseWheelEvent() MOZ_OVERRIDE;
   virtual bool ShouldRollupOnMouseActivate() MOZ_OVERRIDE;
   virtual uint32_t GetSubmenuWidgetChain(nsTArray<nsIWidget*> *aWidgetChain) MOZ_OVERRIDE;
   virtual void NotifyGeometryChange() MOZ_OVERRIDE {}
   virtual nsIWidget* GetRollupWidget() MOZ_OVERRIDE;
 
   static nsXULPopupManager* sInstance;
--- a/widget/cocoa/nsAppShell.mm
+++ b/widget/cocoa/nsAppShell.mm
@@ -845,17 +845,17 @@ nsAppShell::AfterProcessNextEvent(nsIThr
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
   NSString *sender = [aNotification object];
   if (!sender || ![sender isEqualToString:@"org.mozilla.gecko.PopupWindow"]) {
     nsIRollupListener* rollupListener = nsBaseWidget::GetActiveRollupListener();
     nsCOMPtr<nsIWidget> rollupWidget = rollupListener->GetRollupWidget();
     if (rollupWidget)
-      rollupListener->Rollup(0, nullptr, nullptr);
+      rollupListener->Rollup(0, true, nullptr, nullptr);
   }
 
   NS_OBJC_END_TRY_ABORT_BLOCK;
 }
 
 @end
 
 // We hook terminate: in order to make OS-initiated termination work nicely
--- a/widget/cocoa/nsChildView.mm
+++ b/widget/cocoa/nsChildView.mm
@@ -3772,20 +3772,20 @@ NSEvent* gLastDragMouseDownEvent = nil;
         }
       }
 
       if (shouldRollup) {
         if ([theEvent type] == NSLeftMouseDown) {
           NSPoint point = [NSEvent mouseLocation];
           FlipCocoaScreenCoordinate(point);
           nsIntPoint pos(point.x, point.y);
-          consumeEvent = (BOOL)rollupListener->Rollup(popupsToRollup, &pos, nullptr);
+          consumeEvent = (BOOL)rollupListener->Rollup(popupsToRollup, true, &pos, nullptr);
         }
         else {
-          consumeEvent = (BOOL)rollupListener->Rollup(popupsToRollup, nullptr, nullptr);
+          consumeEvent = (BOOL)rollupListener->Rollup(popupsToRollup, true, nullptr, nullptr);
         }
       }
     }
   }
 
   return consumeEvent;
 
   NS_OBJC_END_TRY_ABORT_BLOCK_RETURN(NO);
--- a/widget/cocoa/nsCocoaWindow.mm
+++ b/widget/cocoa/nsCocoaWindow.mm
@@ -87,17 +87,17 @@ NS_IMPL_ISUPPORTS_INHERITED(nsCocoaWindo
 
 static void RollUpPopups()
 {
   nsIRollupListener* rollupListener = nsBaseWidget::GetActiveRollupListener();
   NS_ENSURE_TRUE_VOID(rollupListener);
   nsCOMPtr<nsIWidget> rollupWidget = rollupListener->GetRollupWidget();
   if (!rollupWidget)
     return;
-  rollupListener->Rollup(0, nullptr, nullptr);
+  rollupListener->Rollup(0, true, nullptr, nullptr);
 }
 
 nsCocoaWindow::nsCocoaWindow()
 : mParent(nullptr)
 , mWindow(nil)
 , mDelegate(nil)
 , mSheetWindowParent(nil)
 , mPopupContentView(nil)
--- a/widget/cocoa/nsMenuX.mm
+++ b/widget/cocoa/nsMenuX.mm
@@ -821,17 +821,17 @@ nsresult nsMenuX::SetupIcon()
   // menus, but it also resolves many other problems.
   if (nsMenuX::sIndexingMenuLevel > 0)
     return;
 
   nsIRollupListener* rollupListener = nsBaseWidget::GetActiveRollupListener();
   if (rollupListener) {
     nsCOMPtr<nsIWidget> rollupWidget = rollupListener->GetRollupWidget();
     if (rollupWidget) {
-      rollupListener->Rollup(0, nullptr, nullptr);
+      rollupListener->Rollup(0, true, nullptr, nullptr);
       [menu cancelTracking];
       return;
     }
   }
   mGeckoMenu->MenuOpened();
 }
 
 - (void)menuDidClose:(NSMenu *)menu
--- a/widget/cocoa/nsToolkit.mm
+++ b/widget/cocoa/nsToolkit.mm
@@ -179,17 +179,17 @@ static CGEventRef EventTapCallback(CGEve
   NSWindow *ctxMenuWindow = (NSWindow*) rollupWidget->GetNativeData(NS_NATIVE_WINDOW);
   if (!ctxMenuWindow)
     return event;
   NSPoint screenLocation = ConvertCGGlobalToCocoaScreen(CGEventGetLocation(event));
   // Don't roll up the rollup widget if our mouseDown happens over it (doing
   // so would break the corresponding context menu).
   if (NSPointInRect(screenLocation, [ctxMenuWindow frame]))
     return event;
-  rollupListener->Rollup(0, nullptr, nullptr);
+  rollupListener->Rollup(0, false, nullptr, nullptr);
   return event;
 
   NS_OBJC_END_TRY_ABORT_BLOCK_RETURN(NULL);
 }
 
 // Cocoa Firefox's use of custom context menus requires that we explicitly
 // handle mouse events from other processes that the OS handles
 // "automatically" for native context menus -- mouseMoved events so that
--- a/widget/gtk/nsWindow.cpp
+++ b/widget/gtk/nsWindow.cpp
@@ -629,17 +629,17 @@ nsWindow::Destroy(void)
     g_signal_handlers_disconnect_by_func(gtk_settings_get_default(),
                                          FuncToGpointer(theme_changed_cb),
                                          this);
 
     nsIRollupListener* rollupListener = nsBaseWidget::GetActiveRollupListener();
     if (rollupListener) {
         nsCOMPtr<nsIWidget> rollupWidget = rollupListener->GetRollupWidget();
         if (static_cast<nsIWidget *>(this) == rollupWidget) {
-            rollupListener->Rollup(0, nullptr, nullptr);
+            rollupListener->Rollup(0, false, nullptr, nullptr);
         }
     }
 
     // dragService will be null after shutdown of the service manager.
     nsDragService *dragService = nsDragService::GetInstance();
     if (dragService && this == dragService->GetMostRecentDestWindow()) {
         dragService->ScheduleLeaveEvent();
     }
@@ -4827,17 +4827,17 @@ nsWindow::CheckForRollup(gdouble aMouseX
                   break;
                 }
             } // foreach parent menu widget
         } // if rollup listener knows about menus
 
         // if we've determined that we should still rollup, do it.
         bool usePoint = !aIsWheel && !aAlwaysRollup;
         nsIntPoint point(aMouseX, aMouseY);
-        if (rollup && rollupListener->Rollup(popupsToRollup, usePoint ? &point : nullptr, nullptr)) {
+        if (rollup && rollupListener->Rollup(popupsToRollup, true, usePoint ? &point : nullptr, nullptr)) {
             retVal = true;
         }
     }
     return retVal;
 }
 
 /* static */
 bool
--- a/widget/nsIRollupListener.h
+++ b/widget/nsIRollupListener.h
@@ -15,27 +15,30 @@ struct nsIntPoint;
 
 class nsIRollupListener {
  public: 
 
   /**
    * Notifies the object to rollup, optionally returning the node that
    * was just rolled up.
    *
+   * If aFlush is true, then views should be flushed after the rollup.
+   *
    * aPoint is the mouse pointer position where the event that triggered the
    * rollup occurred, which may be nullptr.
    *
    * aCount is the number of popups in a chain to close. If this is
    * UINT32_MAX, then all popups are closed.
    * If aLastRolledUp is non-null, it will be set to the last rolled up popup,
    * if this is supported. aLastRolledUp is not addrefed.
    *
    * Returns true if the event that the caller is processing should be consumed.
    */
-  virtual bool Rollup(uint32_t aCount, const nsIntPoint* aPoint, nsIContent** aLastRolledUp) = 0;
+  virtual bool Rollup(uint32_t aCount, bool aFlush,
+                      const nsIntPoint* aPoint, nsIContent** aLastRolledUp) = 0;
 
   /**
    * Asks the RollupListener if it should rollup on mouse wheel events
    */
   virtual bool ShouldRollupOnMouseWheelEvent() = 0;
 
   /**
    * Asks the RollupListener if it should consume mouse wheel events
--- a/widget/qt/nsWindow.cpp
+++ b/widget/qt/nsWindow.cpp
@@ -264,17 +264,17 @@ nsWindow::Destroy(void)
     DestroyCompositor();
 
     ClearCachedResources();
 
     nsIRollupListener* rollupListener = nsBaseWidget::GetActiveRollupListener();
     if (rollupListener) {
         nsCOMPtr<nsIWidget> rollupWidget = rollupListener->GetRollupWidget();
         if (static_cast<nsIWidget *>(this) == rollupWidget) {
-            rollupListener->Rollup(0, nullptr, nullptr);
+            rollupListener->Rollup(0, false, nullptr, nullptr);
         }
     }
 
     Show(false);
 
     // walk the list of children and call destroy on them.  Have to be
     // careful, though -- calling destroy on a kid may actually remove
     // it from our child list, losing its sibling links.
@@ -1613,17 +1613,17 @@ nsWindow::CheckForRollup(double aMouseX,
                   break;
                 }
             } // foreach parent menu widget
         } // if rollup listener knows about menus
 
         // if we've determined that we should still rollup, do it.
         if (rollup) {
             nsIntPoint pos(aMouseX, aMouseY);
-            retVal = rollupListener->Rollup(popupsToRollup, &pos, nullptr);
+            retVal = rollupListener->Rollup(popupsToRollup, true, &pos, nullptr);
         }
     }
 
     return retVal;
 }
 
 /* static */
 bool
--- a/widget/windows/nsWindow.cpp
+++ b/widget/windows/nsWindow.cpp
@@ -6602,17 +6602,17 @@ void nsWindow::OnDestroy()
   // turn off capture.
   nsIRollupListener* rollupListener = nsBaseWidget::GetActiveRollupListener();
   nsCOMPtr<nsIWidget> rollupWidget;
   if (rollupListener) {
     rollupWidget = rollupListener->GetRollupWidget();
   }
   if (this == rollupWidget) {
     if ( rollupListener )
-      rollupListener->Rollup(0, nullptr, nullptr);
+      rollupListener->Rollup(0, false, nullptr, nullptr);
     CaptureRollupEvents(nullptr, false);
   }
 
   IMEHandler::OnDestroyWindow(this);
 
   // Turn off mouse trails if enabled.
   MouseTrailer* mtrailer = nsToolkit::gMouseTrailer;
   if (mtrailer) {
@@ -7548,21 +7548,21 @@ nsWindow::DealWithPopups(HWND aWnd, UINT
   if (nativeMessage == WM_LBUTTONDOWN) {
     POINT pt;
     pt.x = GET_X_LPARAM(aLParam);
     pt.y = GET_Y_LPARAM(aLParam);
     ::ClientToScreen(aWnd, &pt);
     nsIntPoint pos(pt.x, pt.y);
 
     consumeRollupEvent =
-      rollupListener->Rollup(popupsToRollup, &pos, &mLastRollup);
+      rollupListener->Rollup(popupsToRollup, true, &pos, &mLastRollup);
     NS_IF_ADDREF(mLastRollup);
   } else {
     consumeRollupEvent =
-      rollupListener->Rollup(popupsToRollup, nullptr, nullptr);
+      rollupListener->Rollup(popupsToRollup, true, nullptr, nullptr);
   }
 
   // Tell hook to stop processing messages
   sProcessHook = false;
   sRollupMsgId = 0;
   sRollupMsgWnd = nullptr;
 
   // If we are NOT supposed to be consuming events, let it go through