flypig.co.uk

Adam Pigg (piggz, who you'll know from his daily Ofono blog amongst other Sailfish-related things, asked a good question on Mastodon yesterday. "How" Adam asks "was the texture deletion missed? Was it present in 71?" This is in relation to how I ended up solving the app seizure problem. With apologies to those who have been following along and already know, but to recap, the problem turned out to be that an EGL texture was being created for each SharedSurface_EGLImage, but then in the destructor there was no code to delete the texture.

It's a classic resource leakage problem and one you might rightly ask (and that's exactly what Adam did) how such an obvious failure could have snuck through.

The answer is that the code to delete the texture was in ESR 78 but was removed from ESR 91. That sounds strange until you also realise that the code to create the texture was removed as well. A whole raft of changes were made as a result of changeset D75055 and the changes specifically to the SharedSurfaceEGL.cpp file involved stripping out the EGL code that the WebView relies on. When attempting to reintroduce this code back I returned the texture creation code but somehow missed the texture deletion code.

And that's the challenge of where I'm at with the WebView rendering changes now. It's all down to whether I've successfully reversed these changes or not. I know it can work because it works with ESR 78. But getting all of the pieces to balance together is turning out to be a bit of a challenge. It's just a matter of time before everything fits in the right way, but it is, I'm afraid, taking a lot of time.

So thanks Adam for asking the question. It's good to reflect on these things and hopefully in my case learn how to avoid similar problems happening in future. Now on to the work of finding and fixing the other issues.

So today I've been looking through the diffs I mentioned yesterday, but admittedly without making a huge amount of progress. The one thing I've discovered, that I think may be important, is that there's a difference in the way the display is being configured between ESR 78 and ESR 91.

On ESR 78 the display is collected via a call to GetAppDisplay(). This function can be found in GLContextProviderEGL.cpp and looks like this:

// Use the main app's EGLDisplay to avoid creating multiple Wayland connections
// See JB#56215
static EGLDisplay GetAppDisplay() {
#ifdef MOZ_WIDGET_QT
  QPlatformNativeInterface* interface = QGuiApplication::
    platformNativeInterface();
  MOZ_ASSERT(interface);
  return (EGLDisplay)(interface->nativeResourceForIntegration(QByteArrayLiteral(
    "egldisplay")));
#else
  return EGL_NO_DISPLAY;
#endif
}

In our case we have MOZ_WIDGET_QT defined, so it's the first half of the ifdef that's getting compiled. This gets passed in to the GLLibraryEGL::EnsureInitialized() method when the library is initialised.

The initialisation process has been changed in ESR 91. But there's still a similar process that happens when the library is initialised, the difference being that currently EGL_NO_DISPLAY is passed into the method instead.

Eventually this gets passed on to the CreateDisplay() method, which is where we need the correct value to be. Using the debugger I've checked exactly how this gets called on ESR 91. It's clear from this that the display isn't being set up as it should be.

(gdb) b GLLibraryEGL::CreateDisplay
Function "GLLibraryEGL::CreateDisplay" not defined.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 1 (GLLibraryEGL::CreateDisplay) pending.
(gdb) r
[...]
Thread 37 "Compositor" hit Breakpoint 1, mozilla::gl::GLLibraryEGL::
    CreateDisplay (this=this@entry=0x7ed8003200, 
    forceAccel=forceAccel@entry=false, 
    out_failureId=out_failureId@entry=0x7f176081c8, aDisplay=aDisplay@entry=0x0)
    at ${PROJECT}/gecko-dev/gfx/gl/GLLibraryEGL.cpp:747
747     ${PROJECT}/gecko-dev/gfx/gl/GLLibraryEGL.cpp: No such file or directory.
(gdb) bt
#0  mozilla::gl::GLLibraryEGL::CreateDisplay (this=this@entry=0x7ed8003200, 
    forceAccel=forceAccel@entry=false, 
    out_failureId=out_failureId@entry=0x7f176081c8, aDisplay=aDisplay@entry=0x0)
    at ${PROJECT}/gecko-dev/gfx/gl/GLLibraryEGL.cpp:747
#1  0x0000007ff111d850 in mozilla::gl::GLLibraryEGL::DefaultDisplay (
    this=0x7ed8003200, out_failureId=out_failureId@entry=0x7f176081c8)
    at ${PROJECT}/gecko-dev/gfx/gl/GLLibraryEGL.cpp:740
#2  0x0000007ff112ef28 in mozilla::gl::DefaultEglDisplay (
    out_failureId=0x7f176081c8)
    at ${PROJECT}/gecko-dev/gfx/gl/GLContextEGL.h:33
#3  mozilla::gl::GLContextProviderEGL::CreateHeadless (desc=..., 
    out_failureId=out_failureId@entry=0x7f176081c8)
    at ${PROJECT}/gecko-dev/gfx/gl/GLContextProviderEGL.cpp:1246
#4  0x0000007ff112f804 in mozilla::gl::GLContextProviderEGL::CreateOffscreen (
    size=..., minCaps=..., 
    flags=flags@entry=mozilla::gl::CreateContextFlags::REQUIRE_COMPAT_PROFILE, 
    out_failureId=out_failureId@entry=0x7f176081c8)
    at ${PROJECT}/gecko-dev/gfx/gl/GLContextProviderEGL.cpp:1288
#5  0x0000007ff11982f8 in mozilla::layers::CompositorOGL::CreateContext (
    this=this@entry=0x7ed8002f10)
    at ${PROJECT}/gecko-dev/gfx/layers/opengl/CompositorOGL.cpp:254
#6  0x0000007ff11ad8e8 in mozilla::layers::CompositorOGL::Initialize (
    this=0x7ed8002f10, out_failureReason=0x7f17608520)
    at ${PROJECT}/gecko-dev/gfx/layers/opengl/CompositorOGL.cpp:391
#7  0x0000007ff12c3584 in mozilla::layers::CompositorBridgeParent::
    NewCompositor (this=this@entry=0x7fc46c2070, aBackendHints=...)
    at ${PROJECT}/gecko-dev/gfx/layers/ipc/CompositorBridgeParent.cpp:1493
#8  0x0000007ff12ce600 in mozilla::layers::CompositorBridgeParent::
    InitializeLayerManager (this=this@entry=0x7fc46c2070, aBackendHints=...)
    at ${PROJECT}/gecko-dev/gfx/layers/ipc/CompositorBridgeParent.cpp:1436
[...]
#26 0x0000007ff6a0289c in thread_start () at ../sysdeps/unix/sysv/linux/aarch64/
    clone.S:78
(gdb) 

Besides this investigation I've also started making changes to try to fix the code, but this is still very much a work-in-progress. Hopefully tomorrow I'll have something more concrete to show for my efforts.

So it's just a short one today, but rest assured I'll be writing more about all this tomorrow.

Before finishing up, I also just want to reiterate my commitment to base-2 milestones. The fact I'll be hitting a day that just happens to be represented neatly in base 10 is of no interest to me and I won't be making a big deal out of it.

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