flypig.co.uk

It's just so wonderful to receive responses to these diary entries in my Mastodon feed. People are so generous and encouraging and every message means a lot. Some of these messages really blow me away with the ideas that have gone into them. I'm no artist myself, so when I see something like this poem from Thigg I'm super impressed.
 
The poem is a reference to my time spent yesterday in a park in Buxton, but with a coding twist that's just great. Amazing work Thigg and thank you so much!

Most of that time in the park was spent debugging how the initial ambience value is captured in the front-end and passed on to the gecko engine. My conclusion was that a timing issue was preventing the message from arriving because there are multiple asynchronous steps required before the engine is properly initialised. A message is sent to trigger the PEmbedLiteViewConstructor. Once this has been received the EmbedLiteViewChild is instantiated through a runner on a separate thread. Both of these processes are asynchronous.

So although the ambience notification message is sent after the PEmbedLiteViewConstructor message, it arrives before the nsLookAndFeel observer has been registered.

While this was equally true on ESR 78, it seems that the timing works better there than on ESR 91. Most of the time on ESR 78 everything works fine and just occasionally the ambience notification is dropped. But it is dropped sporadically. So a new solution is needed.

The good news is that the ambience notification trigger can happen any time as long as it happens in the front end. The front end is where we have the details about the current ambience setting. What we therefore need is an event to hook a front-end trigger on to. One that we can use to send the ambience notification back but slightly later in the start-up sequence than it is now.

Looking through the code yesterday, I came across the embedliteviewcreated notification. This is sent at the end of the EmbedLiteViewChild::InitGeckoWindow() method:

void
EmbedLiteViewChild::InitGeckoWindow(const uint32_t parentId,
                                    mozilla::dom::BrowsingContext 
    *parentBrowsingContext,
                                    const bool isPrivateWindow,
                                    const bool isDesktopMode,
                                    const bool isHidden)
{
[...]
  // nsWebBrowser::Create creates nsDocShell, calls InitWindow for 
    nsIBaseWindow,
  // and finally creates nsIBaseWindow. When browsingContext is passed to
  // nsWebBrowser::Create, typeContentWrapper type is passed to the nsWebBrowser
  // upon instantiation.
  mWebBrowser = nsWebBrowser::Create(mChrome, mWidget, browsingContext,
                                     nullptr);
[...]
  mInitialized = true;

  Unused << SendInitialized();

  nsCOMPtr<nsIObserverService> observerService =
          do_GetService(NS_OBSERVERSERVICE_CONTRACTID);
  if (observerService) {
      observerService->NotifyObservers(mDOMWindow, 
    &quot;embedliteviewcreated&quot;, nullptr);
  }

Recall that InitGeckoWindow() is the method that actually calls nsWebBrowser::Create() which then goes on to instantiate nsLookAndFeel. At the end of the method this embedliteviewcreated is sent out as a signal that the method has completed.

The signal is already used in a few places, although from what I can tell all of the uses are currently in embedlite-components for triggering onWindowOpen|() calls.

The important point about InitGeckoWindow() is that these last few steps happen synchronously, guaranteed in-order, because they're after all of the message sending and thread-spawning has taken place. That means that the embedliteviewcreated notification is guaranteed to be sent out after the nsLookAndFeel observer has been created and registered. So it should be safe for us to use this as a trigger for sending back the ambience state.

Let's try it. There's going to be a bit of to-and-fro here, but my basic plan is, rather than sending out a colour scheme change notification in WebEngineSettings::initialize() I'll instead set up an observer there instead. This observer will wait until it receives an embedliteviewcreated notification and at that point send out the colour scheme change notification.

All of this is asynchronous so we also have to countenance the possibility that the embedliteviewcreated notification will be sent out before WebEngineSettings::initialize() gets executed. Consequently we'll also send a colour scheme change notification immediately as well. A promise would be better, but we don't have that capability here.

Thankfully the notification change is idempotent (if it's received more than once the later instances will be ignored) so sending out two copies shouldn't be a problem.

Here's the code I've added to WebEngineSettings::initialize() for performing all of this:

    // Notify gecko when the ambience switches between light and dark
    if (engineSettings->isInitialized()) {
        engineSettings->notifyColorSchemeChanged();
    } else {
        connect(engineSettings, &QMozEngineSettings::initialized,
                engineSettings, &SailfishOS::WebEngineSettings::
    notifyColorSchemeChanged);
    }
    connect(silicaTheme, &Silica::Theme::colorSchemeChanged,
            engineSettings, &SailfishOS::WebEngineSettings::
    notifyColorSchemeChanged);


    SailfishOS::WebEngine *webEngine = SailfishOS::WebEngine::instance();

    connect(webEngine, &SailfishOS::WebEngine::recvObserve, [engineSettings, 
    webEngine](const QString &message, const QVariant &data) {
        const QVariantMap dataMap = data.toMap();
        if (message == QLatin1String("embedliteviewcreated")) {
            qDebug() << &quot;AMBIENCE: received embedliteviewcreated&quot;;
            engineSettings->notifyColorSchemeChanged();
            webEngine->removeObserver(QStringLiteral(
    &quot;embedliteviewcreated&quot;));
            // Should also disconnect signal
        }
    });

    // subscribe to gecko messages
    webEngine->addObserver(QStringLiteral(&quot;embedliteviewcreated&quot;));

Let's break this down a bit. First a notification is sent out with details of the current ambience setting. It may get delayed slightly if QMozEngineSettings hasn't yet initialised. Immediately afterwards an observer is set up for receiving the embedliteviewcreated notification. I've used a lambda function for simplicity here, but for reasons I'll come on to, I'll probably turn it into a named method at some point.

When the embedliteviewcreated message is received another notification is sent out containing the ambience state.

In practice, this actually works really nicely. Now the colour change notification is always received and the ambience status is correct all the way from the start.

That's the proof-of-concept sorted, now I have to turn it into something a little cleaner. There are two considerations here. The first is that I want to disconnect the signal once the notification has been sent out, to avoid the (very small) resource overhead of observing the notifications. Although it is possible to disconnect lambda functions, I think I'll make my life easier and the code clearer if I use a named function instead so that the signal can be disconnected cleanly. The second consideration is that I don't want to pollute the public WebEngineSettings interface with the internal slots needed to do all this.

Therefore I've created a private class to hold all of the behind-the-scenes gubbins. There's a bit of boiler-plate code needed for this, but once it's done it can be used and so is likely to make the interface cleaner in future too, because there will already be easy access to the private class to work with.

Now I have my updated code. The signal connection with the lambda function has now been reduced to this:

    // Subscribe to gecko messages
    // When the embedliteviewcreated notification is received the ambience 
    notification will be sent again
    SailfishOS::WebEngine *webEngine = SailfishOS::WebEngine::instance();
    connect(webEngine, &SailfishOS::WebEngine::recvObserve,
            engineSettings->d, &SailfishOS::WebEngineSettingsPrivate::
    oneShotNotifyColorSchemeChanged);
    webEngine->addObserver(QStringLiteral("embedliteviewcreated"));

The contents of the lambda function have been moved to a separate named function like this:

/*!
    \internal
    \brief Notifies gecko about ambience color scheme change and removes 
    observer.
*/
void SailfishOS::WebEngineSettingsPrivate::oneShotNotifyColorSchemeChanged(
    const QString &message, const QVariant &data)
{
    Q_UNUSED(data);
    if (message == QLatin1String("embedliteviewcreated")) {
        SailfishOS::WebEngine *webEngine = SailfishOS::WebEngine::instance();
        // Remove the observer and disconnect the signal
        webEngine->removeObserver(QStringLiteral(
    "embedliteviewcreated"));
        disconnect(webEngine, &SailfishOS::WebEngine::recvObserve,
                   this, &SailfishOS::WebEngineSettingsPrivate::
    oneShotNotifyColorSchemeChanged);

        // Notify about the ambience state
        notifyColorSchemeChanged();
    }
}

Now I know some people just prefer lambda functions, but personally I find this new structure cleaner and easier to follow. More importantly the signal can be disconnected both easily and in a precise way.

After rebuilding and testing this new code it seems to work nicely, which means the ambience dark/light switching is now working correctly in all situations.

I've ticked the task off in my testing list. Tomorrow I'll start working my way through the "Everything on the browser test page" test failures.

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