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 248411 bb5356f6a251c4493ebd985cc9dfd815fd689871
parent 248410 e38e801acf7b8772d9bb11f33654e5ce83fd0c3d
child 248412 83f3a7efed212ef4e7d1acf58cccce2e91f44880
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1102039
milestone37.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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