flypig.co.uk

I expected this crash that's happening when the user pressed on a Selection list would be an easy one to fix. It's turning out to be far harder than I'd anticipated. The problem is simply that the mLayerManager member of the top level nsIWidget, which is of type EmbedLitePuppetWidget, is set to null. On ESR 78 it's correctly set to a value, but on ESR 91 that value is missing.

I've put together a sequence of steps for getting to the correct point in the code. It's a little intricate, but it does the trick. The steps are:

1. Add breakpoints to PresShell::Paint() and PuppetWidgetBase::GetLayerManager().
2. Disable the breakpoints.
3. Run the browser and open the test page.
4. Drop to the debugger and enable the Paint() breakpoint.
5. Continue execution.
6. Press on the problem Select widget on the web page.
7. Wait for the breakpoint to hit.
8. Disable the Paint() breakpoint and enable the GetLayerManager() breakpoint.
9. Continue execution and wait for the breakpoint to hit.

At this point the debugger will be at the start of the method where the error is about to hit. Here's a run through of me performing these steps in practice (with some annotations added):

(gdb) delete break
Delete all breakpoints? (y or n) y
(gdb) break PresShell::Paint
Breakpoint 2 at 0x7ff40086f8: file layout/base/PresShell.cpp, line 6229.
(gdb) break PuppetWidgetBase::GetLayerManager
Breakpoint 3 at 0x7ff4c58000: file mobile/sailfishos/embedshared/
    PuppetWidgetBase.cpp, line 419.
(gdb) disable break
(gdb) c
Continuing.
[...]
# Wait for page to load: https://browser.sailfishos.org/tests/testselect.html
^C
Thread 1 "sailfish-browse" received signal SIGINT, Interrupt.
0x0000007fef53e740 in poll () from /lib64/libc.so.6
(gdb) enable break 2
(gdb) c
Continuing.

# Press the selection list button

Thread 10 "GeckoWorkerThre" hit Breakpoint 2, mozilla::PresShell::
    Paint (this=this@entry=0x7fb90f3970, 
    aViewToPaint=aViewToPaint@entry=0x7fb912ae80, 
    aDirtyRegion=..., aFlags=aFlags@entry=mozilla::PaintFlags::PaintLayers)
    at layout/base/PresShell.cpp:6229
6229                          PaintFlags aFlags) {
(gdb) disable break 2
(gdb) enable break 3
(gdb) c
Continuing.

Thread 10 "GeckoWorkerThre" hit Breakpoint 3, mozilla::embedlite::
    PuppetWidgetBase::GetLayerManager (this=this@entry=0x7fb8c99570, 
    aShadowManager=aShadowManager@entry=0x0, 
    aBackendHint=aBackendHint@entry=mozilla::layers::LayersBackend::
    LAYERS_NONE, 
    aPersistence=aPersistence@entry=nsIWidget::LAYER_MANAGER_CURRENT)
    at mobile/sailfishos/embedshared/PuppetWidgetBase.cpp:419
419     {
(gdb) 

Using these steps we can now more easily see the difference between ESR 78 and ESR 91. On ESR 78 we have this:

Thread 10 "GeckoWorkerThre" hit Breakpoint 12, mozilla::embedlite::
    PuppetWidgetBase::GetLayerManager (this=this@entry=0x7f81216bb0, 
    aShadowManager=aShadowManager@entry=0x0, 
    aBackendHint=aBackendHint@entry=mozilla::layers::LayersBackend::
    LAYERS_NONE, 
    aPersistence=aPersistence@entry=nsIWidget::LAYER_MANAGER_CURRENT)
    at mobile/sailfishos/embedshared/PuppetWidgetBase.cpp:420
420       if (Destroyed()) {
(gdb) bt 9
#0  mozilla::embedlite::PuppetWidgetBase::GetLayerManager (
    this=this@entry=0x7f81216bb0, aShadowManager=aShadowManager@entry=0x0, 
    aBackendHint=aBackendHint@entry=mozilla::layers::LayersBackend::
    LAYERS_NONE, aPersistence=aPersistence@entry=nsIWidget::
    LAYER_MANAGER_CURRENT)
    at mobile/sailfishos/embedshared/PuppetWidgetBase.cpp:424
#1  0x0000007fbca92868 in mozilla::embedlite::EmbedLitePuppetWidget::
    GetLayerManager (this=0x7f81216bb0, aShadowManager=0x0, 
    aBackendHint=mozilla::layers::LayersBackend::LAYERS_NONE, 
    aPersistence=nsIWidget::LAYER_MANAGER_CURRENT)
    at mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:381
#2  0x0000007fbbf3ce10 in nsIWidget::GetLayerManager (this=<optimized out>)
    at obj-build-mer-qt-xr/dist/include/nsIWidget.h:1266
#3  mozilla::PresShell::Paint (this=this@entry=0x7f81266db0, 
    aViewToPaint=aViewToPaint@entry=0x7f80ecb310, aDirtyRegion=..., 
    aFlags=aFlags@entry=mozilla::PaintFlags::PaintLayers)
    at layout/base/PresShell.cpp:6134
#4  0x0000007fbbdb40ac in nsViewManager::ProcessPendingUpdatesPaint (
    this=this@entry=0x7f80714980, aWidget=aWidget@entry=0x7f81216bb0)
    at obj-build-mer-qt-xr/dist/include/nsTArray.h:554
#5  0x0000007fbbdb43c0 in nsViewManager::ProcessPendingUpdatesForView (
    this=this@entry=0x7f80714980, aView=<optimized out>, 
    aFlushDirtyRegion=aFlushDirtyRegion@entry=true) at view/nsViewManager.cpp:
    395
#6  0x0000007fbbdb4b50 in nsViewManager::ProcessPendingUpdates (
    this=0x7f80714980)
    at view/nsViewManager.cpp:1018
#7  nsViewManager::ProcessPendingUpdates (this=this@entry=0x7f80714980)
    at view/nsViewManager.cpp:1004
#8  0x0000007fbbf13e4c in nsRefreshDriver::Tick (this=0x7f812166c0, aId=..., 
    aId@entry=..., aNowTime=aNowTime@entry=...)
    at layout/base/nsRefreshDriver.cpp:2201
(More stack frames follow...)
(gdb) n
530       bool Destroyed() const { return mOnDestroyCalled; }
(gdb) n
424       if (mLayerManager) {
(gdb) p mLayerManager
$22 = {mRawPtr = 0x7f80b6c6a0}

I've included the backtrace in the output here for both versions just to check that we really are in the same place. Here's the same on ESR 91:

Thread 10 "GeckoWorkerThre" hit Breakpoint 3, mozilla::embedlite::
    PuppetWidgetBase::GetLayerManager (this=this@entry=0x7fb8c99570, 
    aShadowManager=aShadowManager@entry=0x0, 
    aBackendHint=aBackendHint@entry=mozilla::layers::LayersBackend::
    LAYERS_NONE, 
    aPersistence=aPersistence@entry=nsIWidget::LAYER_MANAGER_CURRENT)
    at mobile/sailfishos/embedshared/PuppetWidgetBase.cpp:419
419     {
(gdb) bt 8
#0  mozilla::embedlite::PuppetWidgetBase::GetLayerManager (
    this=this@entry=0x7fb8c99570, aShadowManager=aShadowManager@entry=0x0, 
    aBackendHint=aBackendHint@entry=mozilla::layers::LayersBackend::
    LAYERS_NONE, aPersistence=aPersistence@entry=nsIWidget::
    LAYER_MANAGER_CURRENT)
    at mobile/sailfishos/embedshared/PuppetWidgetBase.cpp:424
#1  0x0000007ff4c52d40 in mozilla::embedlite::EmbedLitePuppetWidget::
    GetLayerManager (this=0x7fb8c99570, aShadowManager=0x0, 
    aBackendHint=mozilla::layers::LayersBackend::LAYERS_NONE, 
    aPersistence=nsIWidget::LAYER_MANAGER_CURRENT)
    at mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:379
#2  0x0000007ff4008a24 in nsIWidget::GetLayerManager (this=<optimized out>)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsIWidget.h:1303
#3  mozilla::PresShell::Paint (this=this@entry=0x7fb90f3970, 
    aViewToPaint=aViewToPaint@entry=0x7fb912ae80, aDirtyRegion=..., 
    aFlags=aFlags@entry=mozilla::PaintFlags::PaintLayers)
    at layout/base/PresShell.cpp:6274
#4  0x0000007ff3e41068 in nsViewManager::ProcessPendingUpdatesPaint (
    this=this@entry=0x7fb90cdd70, aWidget=aWidget@entry=0x7fb8c99570)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/gfx/RectAbsolute.h:43
#5  0x0000007ff3e4141c in nsViewManager::ProcessPendingUpdatesForView (
    this=this@entry=0x7fb90cdd70, aView=<optimized out>, 
    aFlushDirtyRegion=aFlushDirtyRegion@entry=true)
    at view/nsViewManager.cpp:394
#6  0x0000007ff3e41a0c in nsViewManager::ProcessPendingUpdates (
    this=this@entry=0x7fb90cdd70)
    at view/nsViewManager.cpp:972
#7  0x0000007ff3fe94bc in nsRefreshDriver::Tick (this=0x7fb90cd7a0, aId=..., 
    aId@entry=..., aNowTime=aNowTime@entry=..., 
    aIsExtraTick=aIsExtraTick@entry=nsRefreshDriver::IsExtraTick::No)
    at layout/base/nsRefreshDriver.cpp:2477
(More stack frames follow...)
(gdb) n
564       bool Destroyed() const { return mOnDestroyCalled; }
(gdb) n
424       if (mLayerManager) {
(gdb) p mLayerManager
$1 = {mRawPtr = 0x0}
(gdb) 

As we can see from this, in the ESR 78 case we have a valid value for mLayerManager whereas on ESR 91 it's set to null. This leads to a different flow coming out of PuppetWidgetBase::GetLayerManager(). First on ESR 78:

(gdb) disable break
(gdb) finish
Run till exit from #0  mozilla::embedlite::PuppetWidgetBase::GetLayerManager (
    this=this@entry=0x7f81216bb0, aShadowManager=aShadowManager@entry=0x0, 
    aBackendHint=aBackendHint@entry=mozilla::layers::LayersBackend::
    LAYERS_NONE, aPersistence=aPersistence@entry=nsIWidget::
    LAYER_MANAGER_CURRENT)
    at mobile/sailfishos/embedshared/PuppetWidgetBase.cpp:424
mozilla::embedlite::EmbedLitePuppetWidget::GetLayerManager (this=0x7f81216bb0, 
    aShadowManager=0x0, 
    aBackendHint=mozilla::layers::LayersBackend::LAYERS_NONE, 
    aPersistence=nsIWidget::LAYER_MANAGER_CURRENT)
    at mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:382
382       if (lm) {
Value returned is $23 = (mozilla::layers::LayerManager *) 0x7f80b6c6a0
(gdb) p lm
$24 = (nsIWidget::LayerManager *) 0x7f80b6c6a0

This is as things should be. But on ESR 91 the null value for the layer manager results in additional code being executed. Because this is the top level widget no attempt can be made to extract the layer manager from a parent widget and so the method returns null:

(gdb) disable break
(gdb) finish
Run till exit from #0  mozilla::embedlite::PuppetWidgetBase::GetLayerManager (
    this=this@entry=0x7fb8c99570, aShadowManager=aShadowManager@entry=0x0, 
    aBackendHint=aBackendHint@entry=mozilla::layers::LayersBackend::
    LAYERS_NONE, aPersistence=aPersistence@entry=nsIWidget::
    LAYER_MANAGER_CURRENT)
    at mobile/sailfishos/embedshared/PuppetWidgetBase.cpp:424
[New LWP 22078]
0x0000007ff4c52d40 in mozilla::embedlite::EmbedLitePuppetWidget::
    GetLayerManager (this=0x7fb8c99570, aShadowManager=0x0, 
    aBackendHint=mozilla::layers::LayersBackend::LAYERS_NONE, 
    aPersistence=nsIWidget::LAYER_MANAGER_CURRENT)
    at mobile/sailfishos/embedshared/EmbedLitePuppetWidget.cpp:379
379       LayerManager *lm = PuppetWidgetBase::GetLayerManager(aShadowManager, 
    aBackendHint, aPersistence);
Value returned is $2 = (mozilla::layers::LayerManager *) 0x0
(gdb) n
380       if (lm) {
(gdb) p lm
$3 = (nsIWidget::LayerManager *) 0x0
(gdb) n
385       if (EmbedLiteApp::GetInstance()->GetType() == EmbedLiteApp::
    EMBED_INVALID) {
(gdb) n
396       nsIWidget* topWidget = GetTopLevelWidget();
(gdb) n
397       if (topWidget && topWidget != this) {
(gdb) p topWidget
$4 = (nsIWidget *) 0x7fb8c99570
(gdb) p this
$5 = (mozilla::embedlite::EmbedLitePuppetWidget * const) 0x7fb8c99570
(gdb) 

The obvious questions arises: "where is the layer manager actually created?". At this point I'm not really sure, but most of the opportunities to actually create the layer manager appear to happen inside nsBaseWidget. By placing breakpoints on all of the locations that trigger the creation of a layer manager, we should be able to find out which is the one creating it and where it's happening.

So here's the outcome of this test on ESR 78:

Thread 10 "GeckoWorkerThre" hit Breakpoint 14, nsBaseWidget::
    CreateCompositor (this=0x7f80b6b4b0, aWidth=1080, aHeight=2520)
    at widget/nsBaseWidget.cpp:1260
1260    void nsBaseWidget::CreateCompositor(int aWidth, int aHeight) {
(gdb) bt 10
#0  nsBaseWidget::CreateCompositor (this=0x7f80b6b4b0, aWidth=1080, 
    aHeight=2520)
    at widget/nsBaseWidget.cpp:1260
#1  0x0000007fbca9a1fc in mozilla::embedlite::nsWindow::GetLayerManager (
    this=0x7f80b6b4b0, aShadowManager=0x0, aBackendHint=<optimized out>, 
    aPersistence=nsIWidget::LAYER_MANAGER_CURRENT)
    at mobile/sailfishos/embedshared/nsWindow.cpp:225
#2  0x0000007fbca99318 in nsIWidget::GetLayerManager (this=0x7f80b6b4b0)
    at widget/nsIWidget.h:1266
#3  mozilla::embedlite::PuppetWidgetBase::Invalidate (aRect=..., 
    this=0x7f80b6b4b0)
    at mobile/sailfishos/embedshared/PuppetWidgetBase.cpp:261
#4  mozilla::embedlite::PuppetWidgetBase::Invalidate (this=0x7f80b6b4b0, 
    aRect=...)
    at mobile/sailfishos/embedshared/PuppetWidgetBase.cpp:253
#5  0x0000007fbca9c1b4 in mozilla::embedlite::PuppetWidgetBase::UpdateBounds (
    this=0x7f80b6b4b0, aRepaint=<optimized out>)
    at mobile/sailfishos/embedshared/PuppetWidgetBase.cpp:395
#6  0x0000007fbcaa5950 in mozilla::embedlite::EmbedLiteWindowChild::
    CreateWidget (this=0x7f80b73670)
    at mobile/sailfishos/embedshared/EmbedLiteWindowChild.cpp:198
#7  0x0000007fbca95474 in mozilla::detail::RunnableMethodArguments<>::
    applyImpl<mozilla::embedlite::EmbedLiteWindowChild, void (mozilla::
    embedlite::EmbedLiteWindowChild::*)()>(mozilla::embedlite::
    EmbedLiteWindowChild*, void (mozilla::embedlite::EmbedLiteWindowChild::*)(
    ), mozilla::Tuple<>&, std::integer_sequence<unsigned long>) (args=..., 
    m=<optimized out>, o=<optimized out>)
    at xpcom/threads/nsThreadUtils.h:1188
#8  mozilla::detail::RunnableMethodArguments<>::apply<mozilla::embedlite::
    EmbedLiteWindowChild, void (mozilla::embedlite::EmbedLiteWindowChild::*)()> 
    (
    this=<optimized out>, m=<optimized out>, o=<optimized out>)
    at xpcom/threads/nsThreadUtils.h:1191
#9  mozilla::detail::RunnableMethodImpl<mozilla::embedlite::
    EmbedLiteWindowChild*, void (mozilla::embedlite::EmbedLiteWindowChild::*)(
    ), true, (mozilla::RunnableKind)1>::Run (this=<optimized out>) at xpcom/
    threads/nsThreadUtils.h:1237
(More stack frames follow...)

As we can see, the creation happens in nsBaseWidget::CreateCompositor(). Interestingly on ESR 91 it's also being created in roughly the same place:

Thread 10 "GeckoWorkerThre" hit Breakpoint 2, nsBaseWidget::
    CreateCompositor (this=this@entry=0x7fb8a426a0, aWidth=aWidth@entry=1080, 
    aHeight=aHeight@entry=2520)
    at widget/nsBaseWidget.cpp:1415
1415    void nsBaseWidget::CreateCompositor(int aWidth, int aHeight) {
(gdb) bt 10
#0  nsBaseWidget::CreateCompositor (this=this@entry=0x7fb8a426a0, 
    aWidth=aWidth@entry=1080, aHeight=aHeight@entry=2520)
    at widget/nsBaseWidget.cpp:1415
#1  0x0000007ff4c58208 in mozilla::embedlite::nsWindow::CreateCompositor (
    this=0x7fb8a426a0, aWidth=1080, aHeight=2520)
    at mobile/sailfishos/embedshared/nsWindow.cpp:159
#2  0x0000007ff4c572dc in mozilla::embedlite::nsWindow::CreateCompositor (
    this=0x7fb8a426a0)
    at mobile/sailfishos/embedshared/nsWindow.cpp:152
#3  0x0000007ff4c5a100 in mozilla::embedlite::nsWindow::GetLayerManager (
    this=0x7fb8a426a0, aShadowManager=<optimized out>, aBackendHint=<optimized 
    out>, 
    aPersistence=nsIWidget::LAYER_MANAGER_CURRENT)
    at mobile/sailfishos/embedshared/nsWindow.cpp:214
#4  0x0000007ff4c57d2c in nsIWidget::GetLayerManager (this=0x7fb8a426a0)
    at widget/nsIWidget.h:1303
#5  mozilla::embedlite::PuppetWidgetBase::Invalidate (this=0x7fb8a426a0, 
    aRect=...)
    at mobile/sailfishos/embedshared/PuppetWidgetBase.cpp:261
#6  0x0000007ff4c5c650 in mozilla::embedlite::PuppetWidgetBase::UpdateBounds (
    this=0x7fb8a426a0, aRepaint=aRepaint@entry=true)
    at mobile/sailfishos/embedshared/PuppetWidgetBase.cpp:395
#7  0x0000007ff4c6583c in mozilla::embedlite::EmbedLiteWindowChild::
    CreateWidget (this=0x7fb8b9c010)
    at xpcom/base/nsCOMPtr.h:851
#8  0x0000007ff4c55da8 in mozilla::detail::RunnableMethodArguments<>::
    applyImpl<mozilla::embedlite::EmbedLiteWindowChild, void (mozilla::
    embedlite::EmbedLiteWindowChild::*)()>(mozilla::embedlite::
    EmbedLiteWindowChild*, void (mozilla::embedlite::EmbedLiteWindowChild::*)(
    ), mozilla::Tuple<>&, std::integer_sequence<unsigned long>) (args=..., 
    m=<optimized out>, o=<optimized out>)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsThreadUtils.h:1151
#9  mozilla::detail::RunnableMethodArguments<>::apply<mozilla::embedlite::
    EmbedLiteWindowChild, void (mozilla::embedlite::EmbedLiteWindowChild::*)()> 
    (
    m=<optimized out>, o=<optimized out>, this=<optimized out>)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsThreadUtils.h:1154
(More stack frames follow...)

I say "roughly" because the stack trace isn't identical. The call to nsWindow::CreateCompositor() only happens on ESR 91, which is unexpected. The actual difference is happening inside nsWindow::GetLayerManager(). The code is similar on both, and where they end up in nsBaseWidget::CreateCompositor() is the same, but for some reason the backtrace on ESR 91 is including the two hops needed to get there, whereas they're being skipped in the ESR 78 backtrace. In both cases the call in nsWindow::GetLayerManager() is to CreateCompositor():

void
nsWindow::CreateCompositor()
{
  LOGT();
  // Compositor should be created only for top level widgets, aka windows.
  MOZ_ASSERT(mWindow);
  LayoutDeviceIntRect size = mWindow->GetSize();
  CreateCompositor(size.width, size.height);
}

void
nsWindow::CreateCompositor(int aWidth, int aHeight)
{
  LOGT();
  nsBaseWidget::CreateCompositor(aWidth, aHeight);
}

I'll need to look in to this further, but it looks to me like this might just be a case of optimisation in the ESR 78 code. Unfortunately I've run out of time and energy for today, so I'll have to continue the investigation tomorrow.

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