Backed out changeset 2e63fa41257b (bug 1530594) for mochitest failure use-after-poison /builds/worker/workspace/build/src/layout/xul/nsMenuPopupFrame.h:280:42 in PopupType
authorDaniel Varga <dvarga@mozilla.com>
Sat, 02 Mar 2019 06:45:31 +0200
changeset 520012 812a2e26c05ea230f0104d2a30a19ff5d8ba7529
parent 520011 571c5aa0458018f62d1f0832fc0e92fa3f23fda9
child 520013 82134a2f7b8970a282a8bd3c80fab9fd73bc6b26
child 520038 7f2bfa2b5fb9af64a1219a222a2d02fd7654e71f
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1530594
milestone67.0a1
backs out2e63fa41257b8950f5726fd88936ba3f5e2d8441
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
Backed out changeset 2e63fa41257b (bug 1530594) for mochitest failure use-after-poison /builds/worker/workspace/build/src/layout/xul/nsMenuPopupFrame.h:280:42 in PopupType
layout/xul/nsMenuFrame.cpp
layout/xul/nsMenuPopupFrame.cpp
layout/xul/nsMenuPopupFrame.h
layout/xul/nsXULPopupManager.cpp
toolkit/content/tests/chrome/test_menulist_keynav.xul
--- a/layout/xul/nsMenuFrame.cpp
+++ b/layout/xul/nsMenuFrame.cpp
@@ -1263,23 +1263,16 @@ nsMenuFrame::GetActiveChild(dom::Element
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsMenuFrame::SetActiveChild(dom::Element* aChild) {
   nsMenuPopupFrame* popupFrame = GetPopup();
   if (!popupFrame) return NS_ERROR_FAILURE;
 
-  // Force the child frames within the popup to be generated.
-  AutoWeakFrame weakFrame(popupFrame);
-  popupFrame->GenerateFrames();
-  if (!weakFrame.IsAlive()) {
-    return NS_OK;
-  }
-
   if (!aChild) {
     // Remove the current selection
     popupFrame->ChangeMenuItem(nullptr, false, false);
     return NS_OK;
   }
 
   nsMenuFrame* menu = do_QueryFrame(aChild->GetPrimaryFrame());
   if (menu) popupFrame->ChangeMenuItem(menu, false, false);
--- a/layout/xul/nsMenuPopupFrame.cpp
+++ b/layout/xul/nsMenuPopupFrame.cpp
@@ -1607,28 +1607,16 @@ nsresult nsMenuPopupFrame::SetPopupPosit
     if (aNotify) {
       nsXULPopupPositionedEvent::DispatchIfNeeded(mContent, false, false);
     }
   }
 
   return NS_OK;
 }
 
-void nsMenuPopupFrame::GenerateFrames()
-{
-  const bool generateFrames = IsLeaf();
-  MOZ_ASSERT_IF(generateFrames, !mGeneratedChildren);
-  mGeneratedChildren = true;
-  if (generateFrames) {
-    MOZ_ASSERT(PrincipalChildList().IsEmpty());
-    nsCOMPtr<nsIPresShell> presShell = PresContext()->PresShell();
-    presShell->FrameConstructor()->GenerateChildFrames(this);
-  }
-}
-
 /* virtual */
 nsMenuFrame* nsMenuPopupFrame::GetCurrentMenuItem() { return mCurrentMenu; }
 
 LayoutDeviceIntRect nsMenuPopupFrame::GetConstraintRect(
     const LayoutDeviceIntRect& aAnchorRect,
     const LayoutDeviceIntRect& aRootScreenRect, nsPopupLevel aPopupLevel) {
   LayoutDeviceIntRect screenRectPixels;
 
--- a/layout/xul/nsMenuPopupFrame.h
+++ b/layout/xul/nsMenuPopupFrame.h
@@ -261,18 +261,18 @@ class nsMenuPopupFrame final : public ns
   // (or the frame for mAnchorContent if aAnchorFrame is null), anchored at a
   // rectangle, or at a specific point if a screen position is set. The popup
   // will be adjusted so that it is on screen. If aIsMove is true, then the
   // popup is being moved, and should not be flipped. If aNotify is true, then
   // a popuppositioned event is sent.
   nsresult SetPopupPosition(nsIFrame* aAnchorFrame, bool aIsMove,
                             bool aSizedToPopup, bool aNotify);
 
-  // Force the children to be generated if they have not already been generated.
-  void GenerateFrames();
+  bool HasGeneratedChildren() { return mGeneratedChildren; }
+  void SetGeneratedChildren() { mGeneratedChildren = true; }
 
   // called when the Enter key is pressed while the popup is open. This will
   // just pass the call down to the current menu, if any. If a current menu
   // should be opened as a result, this method should return the frame for
   // that menu, or null if no menu should be opened. Also, calling Enter will
   // reset the current incremental search string, calculated in
   // FindMenuWithShortcut.
   nsMenuFrame* Enter(mozilla::WidgetGUIEvent* aEvent);
--- a/layout/xul/nsXULPopupManager.cpp
+++ b/layout/xul/nsXULPopupManager.cpp
@@ -1274,25 +1274,34 @@ void nsXULPopupManager::FirePopupShowing
                                               bool aIsContextMenu,
                                               bool aSelectFirstItem,
                                               Event* aTriggerEvent) {
   nsCOMPtr<nsIContent> popup = aPopup;  // keep a strong reference to the popup
 
   nsMenuPopupFrame* popupFrame = do_QueryFrame(aPopup->GetPrimaryFrame());
   if (!popupFrame) return;
 
-  popupFrame->GenerateFrames();
+  nsPresContext* presContext = popupFrame->PresContext();
+  nsCOMPtr<nsIPresShell> presShell = presContext->PresShell();
+  nsPopupType popupType = popupFrame->PopupType();
+
+  // generate the child frames if they have not already been generated
+  const bool generateFrames = popupFrame->IsLeaf();
+  MOZ_ASSERT_IF(generateFrames, !popupFrame->HasGeneratedChildren());
+  popupFrame->SetGeneratedChildren();
+  if (generateFrames) {
+    MOZ_ASSERT(popupFrame->PrincipalChildList().IsEmpty());
+    presShell->FrameConstructor()->GenerateChildFrames(popupFrame);
+  }
 
   // get the frame again
   nsIFrame* frame = aPopup->GetPrimaryFrame();
   if (!frame) return;
 
-  nsPresContext* presContext = popupFrame->PresContext();
-  nsCOMPtr<nsIPresShell> presShell = presContext->PresShell();
-  presShell->FrameNeedsReflow(popupFrame, nsIPresShell::eTreeChange,
+  presShell->FrameNeedsReflow(frame, nsIPresShell::eTreeChange,
                               NS_FRAME_HAS_DIRTY_CHILDREN);
 
   // cache the popup so that document.popupNode can retrieve the trigger node
   // during the popupshowing event. It will be cleared below after the event
   // has fired.
   mOpeningPopup = aPopup;
 
   nsEventStatus status = nsEventStatus_eIgnore;
@@ -1325,17 +1334,16 @@ void nsXULPopupManager::FirePopupShowing
   mOpeningPopup = nullptr;
 
   mCachedModifiers = 0;
 
   // if a panel, blur whatever has focus so that the panel can take the focus.
   // This is done after the popupshowing event in case that event is cancelled.
   // Using noautofocus="true" will disable this behaviour, which is needed for
   // the autocomplete widget as it manages focus itself.
-  nsPopupType popupType = popupFrame->PopupType();
   if (popupType == ePopupTypePanel &&
       !popup->AsElement()->AttrValueIs(kNameSpaceID_None,
                                        nsGkAtoms::noautofocus, nsGkAtoms::_true,
                                        eCaseMatters)) {
     nsFocusManager* fm = nsFocusManager::GetFocusManager();
     if (fm) {
       Document* doc = popup->GetUncomposedDoc();
 
--- a/toolkit/content/tests/chrome/test_menulist_keynav.xul
+++ b/toolkit/content/tests/chrome/test_menulist_keynav.xul
@@ -22,24 +22,16 @@
 <menulist id="list2">
   <menupopup id="popup" onpopupshown="checkCursorNavigation();">
     <menuitem id="b1" label="One"/>
     <menuitem id="b2" label="Two" selected="true"/>
     <menuitem id="b3" label="Three"/>
     <menuitem id="b4" label="Four"/>
   </menupopup>
 </menulist>
-<menulist id="list3" sizetopopup="none">
-  <menupopup>
-    <menuitem id="s1" label="One"/>
-    <menuitem id="s2" label="Two"/>
-    <menuitem id="s3" label="Three"/>
-    <menuitem id="s4" label="Four"/>
-  </menupopup>
-</menulist>
 
 <script class="testbody" type="application/javascript">
 <![CDATA[
 
 SimpleTest.waitForExplicitFinish();
 
 var gShowPopup = false;
 var gModifiers = 0;
@@ -278,26 +270,16 @@ function checkCursorNavigation()
 
   synthesizeKey("KEY_ArrowUp", {altKey: true});
   is(list.open, ismac, "alt+up closes popup");
 
   if (ismac) {
     list.open = false;
   }
 
-  // Finally, test a menulist with sizetopopup="none" to ensure keyboard navigation
-  // still works when the popup has not been opened.
-  if (!ismac) {
-    let unsizedMenulist = document.getElementById("list3");
-    unsizedMenulist.focus();
-    synthesizeKey("KEY_ArrowDown");
-    is(unsizedMenulist.selectedIndex, 1, "correct menulist index on keydown");
-    is(unsizedMenulist.label, "Two", "correct menulist label on keydown");
-  }
-
   SimpleTest.finish();
 }
 
 SimpleTest.waitForFocus(runTests);
 
 ]]>
 </script>