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

@@ -1268,13 +1268,6 @@ 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);

layout/xul/nsMenuPopupFrame.cpp

@@ -1612,18 +1612,6 @@ nsresult nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame,
   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; }
 

layout/xul/nsMenuPopupFrame.h

@@ -266,8 +266,8 @@ class nsMenuPopupFrame final : public nsBoxFrame,
   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

layout/xul/nsXULPopupManager.cpp

@@ -1279,15 +1279,24 @@ void nsXULPopupManager::FirePopupShowingEvent(nsIContent* aPopup,
   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
@@ -1330,7 +1339,6 @@ void nsXULPopupManager::FirePopupShowingEvent(nsIContent* aPopup,
   // 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,

toolkit/content/tests/chrome/test_menulist_keynav.xul

@@ -27,14 +27,6 @@
     <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[
@@ -283,16 +275,6 @@ function checkCursorNavigation()
     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();
 }