Bug 1349187 - Ensure the Rollup implementations clear the out-pointer even upon returning false. r?enndeakin draft
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 21 Mar 2017 10:32:56 -0400
changeset 502266 ac10417bad42204a2943be4673619ba9b8244810
parent 502245 bd4f3810b402147f8656390555b29502ce5e2644
child 550112 ec366b8d25707e2bbd3666770c71c79ed5d09a0b
push id50231
push userkgupta@mozilla.com
push dateTue, 21 Mar 2017 14:33:29 +0000
reviewersenndeakin
bugs1349187
milestone55.0a1
Bug 1349187 - Ensure the Rollup implementations clear the out-pointer even upon returning false. r?enndeakin This ensures that the pointer is always either null or a valid nsIContent after the call to Rollup returns, and avoids potentially leaving it as garbage. An alternative approach would be to make the call sites responsible for ensuring it is set to nullptr if the function returns false, but this seems safer. MozReview-Commit-ID: BXxPBgs6MZL
layout/forms/nsComboboxControlFrame.cpp
layout/xul/nsXULPopupManager.cpp
--- a/layout/forms/nsComboboxControlFrame.cpp
+++ b/layout/forms/nsComboboxControlFrame.cpp
@@ -1462,16 +1462,20 @@ nsComboboxControlFrame::SetInitialChildL
 
 //----------------------------------------------------------------------
   //nsIRollupListener
 //----------------------------------------------------------------------
 bool
 nsComboboxControlFrame::Rollup(uint32_t aCount, bool aFlush,
                                const nsIntPoint* pos, nsIContent** aLastRolledUp)
 {
+  if (aLastRolledUp) {
+    *aLastRolledUp = nullptr;
+  }
+
   if (!mDroppedDown) {
     return false;
   }
 
   bool consume = !!COMBOBOX_ROLLUP_CONSUME_EVENT;
   AutoWeakFrame weakFrame(this);
   mListControlFrame->AboutToRollup(); // might destroy us
   if (!weakFrame.IsAlive()) {
--- a/layout/xul/nsXULPopupManager.cpp
+++ b/layout/xul/nsXULPopupManager.cpp
@@ -200,16 +200,20 @@ nsXULPopupManager::GetInstance()
   MOZ_ASSERT(sInstance);
   return sInstance;
 }
 
 bool
 nsXULPopupManager::Rollup(uint32_t aCount, bool aFlush,
                           const nsIntPoint* pos, nsIContent** aLastRolledUp)
 {
+  if (aLastRolledUp) {
+    *aLastRolledUp = nullptr;
+  }
+
   // We can disable the autohide behavior via a pref to ease debugging.
   if (nsXULPopupManager::sDevtoolsDisableAutoHide) {
     // Required on linux to allow events to work on other targets.
     if (mWidget) {
       mWidget->CaptureRollupEvents(nullptr, false);
     }
     return false;
   }