flypig.co.uk

Yesterday turned out to be an unexpectedly fruitful day. I was able to determine the point at which the ESR 78 and ESR 91 execution flows diverged when the user presses the selection widget. ESR 78 goes one way while ESR 91 goes the other. The former works and the latter crashes.

Things often aren't this clear cut, but I'm very much hoping that in this particular case it'll be as simple as this. If this turns out to be true we'll be able to trace the difference back to a change in the code on ESR 91 that should be more like the code on ESR 78.

As we left things yesterday, I'd just stepped through nsComboboxControlFrame::ShowDropDown() and determined that the value for mDelayedShowDropDown is being set to true because the following condition is resolving to false:

(!fm || fm->GetFocusedElement() == GetContent())

So what happens when we step through nsComboboxControlFrame::ShowDropDown() on ESR 78? The answer is: "we don't!" Because the method is never called on ESR 78. So we're going to have to go a bit further down this rabbit hole. Here's the backtrace for the call on ESR 91:

Thread 10 "GeckoWorkerThre" hit Breakpoint 3, nsComboboxControlFrame::
    ShowDropDown (this=0x7fb9103c28, aDoDropDown=aDoDropDown@entry=true)
    at layout/forms/nsComboboxControlFrame.cpp:890
890     void nsComboboxControlFrame::ShowDropDown(bool aDoDropDown) {
(gdb) bt
#0  nsComboboxControlFrame::ShowDropDown (this=0x7fb9103c28, 
    aDoDropDown=aDoDropDown@entry=true)
    at layout/forms/nsComboboxControlFrame.cpp:890
#1  0x0000007ff41a3d94 in nsListControlFrame::MouseDown (this=0x7fb9103d88, 
    aMouseEvent=aMouseEvent@entry=0x7fb91ec9c0)
    at layout/forms/nsListControlFrame.cpp:1718
#2  0x0000007ff41a53bc in nsListEventListener::HandleEvent (this=0x7fb90ffb50, 
    aEvent=0x7fb91ec9c0)
    at layout/forms/nsListControlFrame.cpp:2359
#3  0x0000007ff33d7330 in mozilla::EventListenerManager::HandleEventSubType (
    this=this@entry=0x7fb912ea40, aListener=<optimized out>,
    aListener@entry=0x7fb912eb08, aDOMEvent=0x7fb91ec9c0, 
    aCurrentTarget=<optimized out>, aCurrentTarget@entry=0x7fb90ec570)
    at dom/events/EventListenerManager.cpp:1118
#4  0x0000007ff33da7f8 in mozilla::EventListenerManager::HandleEventInternal (
    this=0x7fb912ea40, aPresContext=0x7fb8fe4a70, aEvent=0x7fde7d4d88,
    aDOMEvent=aDOMEvent@entry=0x7fde7d4810, aCurrentTarget=<optimized out>, 
    aEventStatus=aEventStatus@entry=0x7fde7d4818,
    aItemInShadowTree=<optimized out>)
    at dom/events/EventListenerManager.cpp:1309
#5  0x0000007ff33daf0c in mozilla::EventListenerManager::HandleEvent (
    aItemInShadowTree=<optimized out>, aEventStatus=0x7fde7d4818,
    aCurrentTarget=<optimized out>, aDOMEvent=0x7fde7d4810, aEvent=<optimized 
    out>, aPresContext=<optimized out>, this=<optimized out>)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/
    EventListenerManager.h:390
#6  mozilla::EventTargetChainItem::HandleEvent (this=this@entry=0x7fb8c9a088, 
    aVisitor=..., aCd=...)
    at dom/events/EventDispatcher.cpp:348
#7  0x0000007ff33db5b0 in mozilla::EventTargetChainItem::HandleEventTargetChain 
    (aChain=..., aVisitor=..., aCallback=aCallback@entry=0x7fde7d4aa8,
    aCd=...)
    at dom/events/EventDispatcher.cpp:550
#8  0x0000007ff33db954 in mozilla::EventTargetChainItem::HandleEventTargetChain 
    (aChain=..., aVisitor=..., aCallback=aCallback@entry=0x7fde7d4aa8,
    aCd=...)
    at dom/events/EventDispatcher.cpp:630
#9  0x0000007ff33dc93c in mozilla::EventDispatcher::Dispatch (
    aTarget=<optimized out>, aPresContext=aPresContext@entry=0x7fb8fe4a70,
    aEvent=aEvent@entry=0x7fde7d4d88, aDOMEvent=aDOMEvent@entry=0x0, 
    aEventStatus=aEventStatus@entry=0x7fde7d4d6c,
    aCallback=aCallback@entry=0x7fde7d4aa8, aTargets=aTargets@entry=0x0)
    at dom/events/EventDispatcher.cpp:1082
#10 0x0000007ff401dccc in mozilla::PresShell::EventHandler::DispatchEventToDOM (
    this=this@entry=0x7fde7d4bd8, aEvent=aEvent@entry=0x7fde7d4d88,
    aEventStatus=aEventStatus@entry=0x7fde7d4d6c, 
    aEventCB=aEventCB@entry=0x7fde7d4aa8)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsCOMPtr.h:859
#11 0x0000007ff401e7a0 in mozilla::PresShell::EventHandler::DispatchEvent (
    this=this@entry=0x7fde7d4bd8,
    aEventStateManager=aEventStateManager@entry=0x7fb8fe4d60, 
    aEvent=aEvent@entry=0x7fde7d4d88, aTouchIsNew=false,
    aEventStatus=aEventStatus@entry=0x7fde7d4d6c, 
    aOverrideClickTarget=aOverrideClickTarget@entry=0x0)
    at layout/base/PresShell.cpp:8245
#12 0x0000007ff401f588 in mozilla::PresShell::EventHandler::
    HandleEventWithCurrentEventInfo (this=this@entry=0x7fde7d4bd8,
    aEvent=aEvent@entry=0x7fde7d4d88, 
    aEventStatus=aEventStatus@entry=0x7fde7d4d6c, 
    aIsHandlingNativeEvent=aIsHandlingNativeEvent@entry=true,
    aOverrideClickTarget=0x0)
    at layout/base/PresShell.cpp:8177
#13 0x0000007ff4023dbc in mozilla::PresShell::EventHandler::
    HandleEventUsingCoordinates (this=this@entry=0x7fde7d4ca8,
    aFrameForPresShell=aFrameForPresShell@entry=0x7fb9102980, 
    aGUIEvent=aGUIEvent@entry=0x7fde7d4d88, 
    aEventStatus=aEventStatus@entry=0x7fde7d4d6c,
    aDontRetargetEvents=aDontRetargetEvents@entry=false)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsCOMPtr.h:859
#14 0x0000007ff4023fa0 in mozilla::PresShell::EventHandler::HandleEvent (
    this=this@entry=0x7fde7d4ca8,
    aFrameForPresShell=aFrameForPresShell@entry=0x7fb9102980, 
    aGUIEvent=aGUIEvent@entry=0x7fde7d4d88,
    aDontRetargetEvents=aDontRetargetEvents@entry=false, 
    aEventStatus=aEventStatus@entry=0x7fde7d4d6c)
    at layout/base/PresShell.cpp:6898
#15 0x0000007ff40240ec in mozilla::PresShell::HandleEvent (this=0x7fb90d7250, 
    aFrameForPresShell=0x7fb9102980, aGUIEvent=aGUIEvent@entry=0x7fde7d4d88,
    aDontRetargetEvents=aDontRetargetEvents@entry=false, 
    aEventStatus=aEventStatus@entry=0x7fde7d4d6c)
    at layout/base/PresShell.cpp:6841
#16 0x0000007ff270373c in nsContentUtils::SendMouseEvent (
    aPresShell=aPresShell@entry=0x7fb90d7250, aType=..., aX=aX@entry=128.851852,
    aY=aY@entry=98.9074097, aButton=aButton@entry=0, 
    aButtons=aButtons@entry=-1, aClickCount=aClickCount@entry=1, 
    aModifiers=aModifiers@entry=0,
    aIgnoreRootScrollFrame=aIgnoreRootScrollFrame@entry=false, 
    aPressure=aPressure@entry=0, aInputSourceArg=aInputSourceArg@entry=5,
    aIdentifier=aIdentifier@entry=0, aToWindow=aToWindow@entry=true, 
    aPreventDefault=aPreventDefault@entry=0x7fde7d4f57,
    aIsDOMEventSynthesized=<optimized out>, aIsDOMEventSynthesized@entry=true, 
    aIsWidgetEventSynthesized=aIsWidgetEventSynthesized@entry=false)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsView.h:268
#17 0x0000007ff2716980 in nsDOMWindowUtils::SendMouseEventCommon (
    this=this@entry=0x7fb84bc880, aType=..., aX=aX@entry=128.851852,
    aY=aY@entry=98.9074097, aButton=aButton@entry=0, 
    aClickCount=aClickCount@entry=1, aModifiers=aModifiers@entry=0,
    aIgnoreRootScrollFrame=aIgnoreRootScrollFrame@entry=false, 
    aPressure=aPressure@entry=0, aInputSourceArg=aInputSourceArg@entry=5,
    aPointerId=aPointerId@entry=0, aToWindow=aToWindow@entry=true, 
    aPreventDefault=aPreventDefault@entry=0x0,
    aIsDOMEventSynthesized=aIsDOMEventSynthesized@entry=true, 
    aIsWidgetEventSynthesized=aIsWidgetEventSynthesized@entry=false, 
    aButtons=aButtons@entry=-1)
    at dom/base/nsDOMWindowUtils.cpp:732
#18 0x0000007ff2716ca0 in nsDOMWindowUtils::SendMouseEventToWindow (
    this=0x7fb84bc880, aType=..., aX=128.851852, aY=98.9074097, aButton=0, 
    aClickCount=1,
    aModifiers=0, aIgnoreRootScrollFrame=false, aPressure=0, aInputSourceArg=5, 
    aIsDOMEventSynthesized=<optimized out>, aIsWidgetEventSynthesized=false,
    aButtons=0, aIdentifier=<optimized out>, aOptionalArgCount=3 '\003')
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/ProfilerLabels.h:249
[...]
#69 0x0000007fef54889c in ?? () from /lib64/libc.so.6
(gdb)

We need to know why this isn't called on ESR 78. Without having yet checked the code, it would seem that the answer must be in nsListControlFrame::MouseDown() given that this is frame #1 of the backtrace above and that it also gets called on ESR 78:

(gdb) b nsListControlFrame::MouseDown
Breakpoint 14 at 0x7fbc0b3d08: file layout/forms/nsListControlFrame.cpp, line 
    1659.
(gdb) c
Continuing.

Thread 10 "GeckoWorkerThre" hit Breakpoint 14, nsListControlFrame::
    MouseDown (this=0x7f80f671e0, aMouseEvent=aMouseEvent@entry=0x7f803cc1f0)
    at layout/forms/nsListControlFrame.cpp:1659
1659    nsresult nsListControlFrame::MouseDown(dom::Event* aMouseEvent) {
(gdb) 

Once again the code for this method appears to be identical in both versions. It's a bit longer than the previous examples, so I've removed some lines at the start that aren't so relevant. In practice though, we really do need to see all of the remaining lines to get a trip on what's happening.

nsresult nsListControlFrame::MouseDown(dom::Event* aMouseEvent) {
[...]
  if (!IsLeftButton(aMouseEvent)) {
    if (IsInDropDownMode()) {
      if (!IgnoreMouseEventForSelection(aMouseEvent)) {
        aMouseEvent->PreventDefault();
        aMouseEvent->StopPropagation();
      } else {
        return NS_OK;
      }
      return NS_ERROR_FAILURE;  // means consume event
    } else {
      return NS_OK;
    }
  }

  int32_t selectedIndex;
  if (NS_SUCCEEDED(GetIndexFromDOMEvent(aMouseEvent, selectedIndex))) {
    // Handle Like List
    mButtonDown = true;
    CaptureMouseEvents(true);
    AutoWeakFrame weakFrame(this);
    bool change =
        HandleListSelection(aMouseEvent, selectedIndex);  // might destroy us
    if (!weakFrame.IsAlive()) {
      return NS_OK;
    }
    mChangesSinceDragStart = change;
  } else {
    // NOTE: the combo box is responsible for dropping it down
    if (mComboboxFrame) {
      // Ignore the click that occurs on the option element when one is
      // selected from the parent process popup.
      if (mComboboxFrame->IsOpenInParentProcess()) {
        nsCOMPtr<nsIContent> econtent =
            do_QueryInterface(aMouseEvent->GetTarget());
        HTMLOptionElement* option = HTMLOptionElement::FromNodeOrNull(econtent);
        if (option) {
          return NS_OK;
        }
      }

      uint16_t inputSource = mouseEvent->MozInputSource();
      bool isSourceTouchEvent =
          inputSource == MouseEvent_Binding::MOZ_SOURCE_TOUCH;
      if (FireShowDropDownEvent(
              mContent, !mComboboxFrame->IsDroppedDownOrHasParentPopup(),
              isSourceTouchEvent)) {
        return NS_OK;
      }

      if (!IgnoreMouseEventForSelection(aMouseEvent)) {
        return NS_OK;
      }

      if (!nsComboboxControlFrame::ToolkitHasNativePopup()) {
        bool isDroppedDown = mComboboxFrame->IsDroppedDown();
        nsIFrame* comboFrame = do_QueryFrame(mComboboxFrame);
        AutoWeakFrame weakFrame(comboFrame);
        mComboboxFrame->ShowDropDown(!isDroppedDown);
        if (!weakFrame.IsAlive()) return NS_OK;
        if (isDroppedDown) {
          CaptureMouseEvents(false);
        }
      }
    }
  }

  return NS_OK;
}

Stepping through using the debugger, it looks like the return value for IgnoreMouseEventForSelection(aMouseEvent)) is what's causing the method to return early so that the ShowDropDown() method never gets called on ESR 78:

Thread 10 "GeckoWorkerThre" hit Breakpoint 14, nsListControlFrame::
    MouseDown (this=0x7f80f671e0, aMouseEvent=aMouseEvent@entry=0x7f803cc1f0)
    at layout/forms/nsListControlFrame.cpp:1659
1659    nsresult nsListControlFrame::MouseDown(dom::Event* aMouseEvent) {
(gdb) n
1662      MouseEvent* mouseEvent = aMouseEvent->AsMouseEvent();
1663      NS_ENSURE_TRUE(mouseEvent, NS_ERROR_FAILURE);
(gdb) n
1665      UpdateInListState(aMouseEvent);
(gdb) n
1668      if (eventStates.HasState(NS_EVENT_STATE_DISABLED)) {
(gdb) n
1675      if (!IsLeftButton(aMouseEvent)) {
(gdb) n
1690      if (NS_SUCCEEDED(GetIndexFromDOMEvent(aMouseEvent, selectedIndex))) {
(gdb) n
1703        if (mComboboxFrame) {
(gdb) n
162       bool IsOpenInParentProcess() { return mIsOpenInParentProcess; }
(gdb) n
1715          uint16_t inputSource = mouseEvent->MozInputSource();
(gdb) n
1718          if (FireShowDropDownEvent(
(gdb) n
1719                  mContent, !mComboboxFrame->IsDroppedDownOrHasParentPopup(
    ),
(gdb) n
1718          if (FireShowDropDownEvent(
(gdb) n
1724          if (!IgnoreMouseEventForSelection(aMouseEvent)) {
(gdb) n
nsListEventListener::HandleEvent (aEvent=0x7f803cc1f0, this=0x7f80f7d490)
    at layout/forms/nsListControlFrame.cpp:2352
2352      nsAutoString eventType;

If we compare this with the flow on ESR 91 we can see that there the method doesn't drop out early in this way:

Thread 10 "GeckoWorkerThre" hit Breakpoint 4, nsListControlFrame::
    MouseDown (this=0x7fb91555d8, aMouseEvent=aMouseEvent@entry=0x7fb9140940)
    at layout/forms/nsListControlFrame.cpp:1645
1645    nsresult nsListControlFrame::MouseDown(dom::Event* aMouseEvent) {
(gdb) n
1648      MouseEvent* mouseEvent = aMouseEvent->AsMouseEvent();
(gdb) n
1649      NS_ENSURE_TRUE(mouseEvent, NS_ERROR_FAILURE);
(gdb) n
1651      UpdateInListState(aMouseEvent);
(gdb) n
1653      EventStates eventStates = mContent->AsElement()->State();
(gdb) n
1661      if (!IsLeftButton(aMouseEvent)) {
(gdb) n
1676      if (NS_SUCCEEDED(GetIndexFromDOMEvent(aMouseEvent, selectedIndex))) {
(gdb) n
1689        if (mComboboxFrame) {
(gdb) n
156       bool IsOpenInParentProcess() { return mIsOpenInParentProcess; }
(gdb) n
1701          uint16_t inputSource = mouseEvent->MozInputSource();
(gdb) n
1702          bool isSourceTouchEvent =
(gdb) n
1704          if (FireShowDropDownEvent(
(gdb) n
1705                  mContent, !mComboboxFrame->IsDroppedDownOrHasParentPopup(
    ),
(gdb) n
1710          if (!IgnoreMouseEventForSelection(aMouseEvent)) {
(gdb) n
1715            bool isDroppedDown = mComboboxFrame->IsDroppedDown();
(gdb) n
1716            nsIFrame* comboFrame = do_QueryFrame(mComboboxFrame);
(gdb) n
1717            AutoWeakFrame weakFrame(comboFrame);
(gdb) n
1718            mComboboxFrame->ShowDropDown(!isDroppedDown);
(gdb) n
5610      bool IsAlive() const { return !!mFrame; }
(gdb) n
1720            if (isDroppedDown) {
(gdb) n
nsListEventListener::HandleEvent (this=0x7fb9138f20, aEvent=0x7fb9140940)
    at layout/forms/nsListControlFrame.cpp:2347
2347      nsAutoString eventType;

Different!

So it looks like IgnoreMouseEventForSelection() is returning different values depending on the version. If that really is what's happening here it would be really helpful to know why. Checking out IgnoreMouseEventForSelection(), the code is, once again, identical on both version:

bool nsListControlFrame::IgnoreMouseEventForSelection(dom::Event* aEvent) {
  if (!mComboboxFrame) return false;

  // Our DOM listener does get called when the dropdown is not
  // showing, because it listens to events on the SELECT element
  if (!mComboboxFrame->IsDroppedDown()) return true;

  return !mItemSelectionStarted;
}

To understand why this is returning different values it would be convenient if we could step through the method to find out why. Unfortunately the method has been highly optimised on ESR 78 so that it's impossible to step through. Nevertheless we can at least extract the values and figure out what the flow would be ourselves. First on ESR 78:

(gdb) p mComboboxFrame
$8 = (nsComboboxControlFrame *) 0x7f80f67078
(gdb) p mComboboxFrame->mDroppedDown
$9 = false
(gdb) p mItemSelectionStarted
$10 = true
(gdb) 

Then on ESR 91:

(gdb) p mComboboxFrame
$8 = (nsComboboxControlFrame *) 0x7fb9103b18
(gdb) p mComboboxFrame->mDroppedDown
$9 = false
(gdb) p mItemSelectionStarted
$10 = true
(gdb) 

The values are identical, which isn't what I was expecting. Same code, same state, means same return value. Since mComboboxFrame->mDroppedDown is set to false the method will return with a value of true. If we look back at the code for MouseDown() this means that the condition wrapping IgnoreMouseEventForSelection() won't cause an early return on ESR 91. But it won't cause on early return on ESR 78 either. So this was a misunderstanding on my part because of the way the code was optimised on ESR 78. That's because at the end of the method, although it doesn't return early, the code in MouseDown() actually skips straight passed the following conditional block:

      if (!nsComboboxControlFrame::ToolkitHasNativePopup()) {
        bool isDroppedDown = mComboboxFrame->IsDroppedDown();
        nsIFrame* comboFrame = do_QueryFrame(mComboboxFrame);
        AutoWeakFrame weakFrame(comboFrame);
        mComboboxFrame->ShowDropDown(!isDroppedDown);
        if (!weakFrame.IsAlive()) return NS_OK;
        if (isDroppedDown) {
          CaptureMouseEvents(false);
        }
      }

Once this has been skipped we immediately reach the end of the MouseDown() method and return. So it's not an early return at all, but just the rest of the code in the method being skipped and us reaching the end of the method.

So we can conclude from our step-throughs from earlier that in fact it's this block of code that's being skipped on ESR 78 but not in ESR 91, and that's because nsComboboxControlFrame::ToolkitHasNativePopup() returns different values. On ESR 78 it returns true whereas on ESR 91 it returns false.

I can't show you this with the debugger because the entire method is optimised down into a single value of either true or false. We can see why this optimisation happens if we look at the code. Unfortunately, looking at the code for this method alone also doesn't tell us which value it will return. Here it is:

/* static */
bool nsComboboxControlFrame::ToolkitHasNativePopup() {
#ifdef MOZ_USE_NATIVE_POPUP_WINDOWS
  return true;
#else
  return false;
#endif /* MOZ_USE_NATIVE_POPUP_WINDOWS */
}

The return value of this method, or more precisely the Boolean value that the compiler completely replaces this method with, is determined by the pre-processor depending on whether or not MOZ_USE_NATIVE_POPUP_WINDOWS has been set. The easiest way for me to check which value this represents on ESR 91 is to add some errors in to the code and attempt to compile it. Only the error inside the compiled block will be hit; the error in the other branch will be silently ignored. So here's the amendment I've made to this code:

/* static */
bool nsComboboxControlFrame::ToolkitHasNativePopup() {
#ifdef MOZ_USE_NATIVE_POPUP_WINDOWS
#error MOZ_USE_NATIVE_POPUP_WINDOWS=1
  return true;
#else
#error MOZ_USE_NATIVE_POPUP_WINDOWS=0
  return false;
#endif /* MOZ_USE_NATIVE_POPUP_WINDOWS */
}

Now when I try to compile this on ESR 91 I get the following output.

In file included from Unified_cpp_layout_forms0.cpp:29:
${PROJECT}/gecko-dev/layout/forms/nsComboboxControlFrame.cpp:1618:2: error: 
    #error MOZ_USE_NATIVE_POPUP_WINDOWS=0
 #error MOZ_USE_NATIVE_POPUP_WINDOWS=0
  ^~~~~

In other words, on ESR 91 MOZ_USE_NATIVE_POPUP_WINDOWS hasn't been defined even though it should have been. Clearly this is wrong and some brief testing shows that yes, if the return value from this method is reversed the crash goes away when pressing on a Select widget.

That means we've finally reached the answer we need. It's still not entirely clear how this should be fixed, but now I know there is a way to fix it and it's just a matter of finding the most appropriate approach.

Getting to a solution like this feels like coming up for air; coming out of the rabbit hole and seeing the sun. When you're still digging downwards trying to find out what the problem is you have no idea how hard it's going to be to find a solution, or whether you'll even be able to find a solution at all. As we saw with the rendering pipeline, finding the solution can take months. This time it took just a few days, but the relief of knowing that you can solve it... that's the real joy of this kind of development. It's a really good feeling.

But we do still need to figure out what changes to make to solve this and it's all going to come down to getting MOZ_USE_NATIVE_POPUP_WINDOWS defined as the correct value. On ESR 78, we can see that MOZ_USE_NATIVE_POPUP_WINDOWS is set in embedding/embedlite/confvars.sh:

MOZ_APP_BASENAME=xulrunner-qt5
MOZ_APP_NAME=xulrunner-qt5
MOZ_APP_DISPLAYNAME=XULRunner
MOZ_APP_ID={6105197a-d833-4e45-ac65-dfda38830696}
MOZ_UPDATER=0
MOZ_XULRUNNER=1
MOZ_CHROME_FILE_FORMAT=omni
MOZ_APP_VERSION=$MOZILLA_VERSION
MOZ_URL_CLASSIFIER=1
MOZ_SERVICES_COMMON=
MOZ_SERVICES_CRYPTO=
MOZ_SERVICES_SYNC=
MOZ_MEDIA_NAVIGATOR=1
MOZ_USE_NATIVE_POPUP_WINDOWS=1
MOZ_SERVICES_HEALTHREPORT=
MOZ_DISABLE_EXPORT_JS=1

This then gets used by old-configure so that it ends up defined for the pre-processor. The value also appears in confvars.sh in the ESR 91 code, so the obvious question is why this isn't then being set as a pre-processor define on ESR 91. The answer, it turns out, can be found in toolkit/moz.configure, where this bit of code appears in the ESR 91 version:

option(
    env="MOZ_USE_NATIVE_POPUP_WINDOWS",
    default=target_is_android,
    help="Whether to use native popup windows",
)

set_define("MOZ_USE_NATIVE_POPUP_WINDOWS", True, 
    when="MOZ_USE_NATIVE_POPUP_WINDOWS")

We don't see this on ESR 78 and using git blame it's possible to find out exactly what changed, when and why. Although it turns out to be a little more effort than expected, because there are multiple commits since then that just reformat the same code in slightly different ways, without changing their meaning. Eventually though, if we work back far enough, we get to the relevant upstream change:

$ git diff f5328d27ba2c2~ f5328d27ba2c2 -- toolkit/moz.configure
diff --git a/toolkit/moz.configure b/toolkit/moz.configure
index 2c4d8fa2445e..a30c6b227481 100644
--- a/toolkit/moz.configure
+++ b/toolkit/moz.configure
@@ -2097,3 +2097,9 @@ set_config('ANDROID_PACKAGE_NAME', android_package_name)
 option(env='MOZ_WINCONSOLE', nargs='?',
        help='Whether we can create a console window.')
 set_define('MOZ_WINCONSOLE', True, when=depends('MOZ_WINCONSOLE')(lambda x: x))
+
+option(env='MOZ_USE_NATIVE_POPUP_WINDOWS', default=target_is_android,
+       help='Whether to use native popup windows')
+
+set_define('MOZ_USE_NATIVE_POPUP_WINDOWS', True,
+           when='MOZ_USE_NATIVE_POPUP_WINDOWS')

Checking the commit log we can see how the change was described:

$ git log -1 f5328d27ba2c2
commit f5328d27ba2c2c591384aab2d6cef323fecdfceb
Author: Ricky Stewart <rstewart@mozilla.com>
Date:   Fri Aug 21 22:48:09 2020 +0000

    Bug 1659756 - Move `MOZ_USE_NATIVE_POPUP_WINDOWS` from `old-configure` to 
    Python `configure` r=geckoview-reviewers,mhentges,agi,froydnj,glandium
    
    Differential Revision: https://phabricator.services.mozilla.com/D87462

So this commit didn't just add some code in to moz.configure, it also removed some from old-configure.in:

$ git diff f5328d27ba2c2~ f5328d27ba2c2 -- old-configure.in
diff --git a/old-configure.in b/old-configure.in
index 696b31c10dff..69d558cb7304 100644
--- a/old-configure.in
+++ b/old-configure.in
@@ -1490,7 +1490,6 @@ MOZ_UNIVERSALCHARDET=1
 MOZ_XUL=1
 MOZ_ZIPWRITER=1
 MOZ_NO_SMART_CARDS=
-MOZ_USE_NATIVE_POPUP_WINDOWS=
 MOZ_EXCLUDE_HYPHENATION_DICTIONARIES=
 MOZ_SANDBOX=1
 MOZ_BINARY_EXTENSIONS=
@@ -1897,10 +1896,6 @@ for extension in $MOZ_EXTENSIONS; do
     fi
 done
 
-if test -n "$MOZ_USE_NATIVE_POPUP_WINDOWS"; then
-  AC_DEFINE(MOZ_USE_NATIVE_POPUP_WINDOWS)
-fi
-
 # Avoid defining MOZ_ENABLE_CAIRO_FT on Windows platforms because
 # &quot;cairo-ft-font.c&quot; includes <dlfcn.h>, which only exists on posix 
    platforms
 if test -n &quot;$MOZ_TREE_FREETYPE&quot; -a &quot;$OS_TARGET&quot; != WINNT; 
    then

Interpreting these changes, apparently the mechanism through which this MOZ_USE_NATIVE_POPUP_WINDOWS define is set has been updated. Previously it could be set using confvar.sh but I'm a bit confused about how it should be set now. Based on the code now found in the moz.configure file, the value is now set via an option. The documentation tells us what the various parts of this option statement mean the following:
 
From this and the fact the option has a parameter of env="MOZ_USE_NATIVE_POPUP_WINDOWS" it would seem that we need to set an environment variable. If set, the set_define() part of this code will then apply, which is explained in the documentation as follows:
 
So we need to set an environment variable, but it's not clear to me why this isn't the same as adding the a key-value pair to confvars.sh. Nevertheless I've now also added it to the mozconfig.merqtxulrunner file:

export MOZ_USE_NATIVE_POPUP_WINDOWS=1

In order to test this I'm going to have to perform a full rebuild. This changes the build values, so I really do need to build everything, which will likely take at least until the end of the day, probably even until the early hours of the morning. So that's all I can reasonably do today. Tomorrow we'll see whether this change has worked or not.

If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.