flypig.co.uk

When I woke up this morning my build had failed. The changes I made to the preferences yesterday introduced an error into the C++ code:

224:55.80 mobile/sailfishos
224:57.07 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:19:10: fatal error:
          mozilla/StaticPrefs_embedlite.h: No such file or directory
224:57.07  #include "mozilla/StaticPrefs_embedlite.h" // for StaticPrefs::embedlite_*()
224:57.07           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
224:57.07 compilation terminated.

So the StaticPrefs_embedlite.h header file, which I'd assumed would be generated by the build process, can't be found. Maybe it wasn't generated after all?

Although my assumptions were wrong, they were only partially wrong, because there is a file called StaticPrefList_embedlite.h that's been generated and it does contain the preferences I added to StaticPrefList.yaml. Here's a snippet:

// This file was generated by generate_static_pref_list.py 
// from modules/libpref/init/StaticPrefList.yaml. DO NOT EDIT.

ALWAYS_PREF(
  "embedlite.azpc.handle.viewport",
   embedlite_azpc_handle_viewport,
   embedlite_azpc_handle_viewport,
  bool, true
)
[...]

And there is also a file called StaticPrefs_embedlite.h which also contains what we'd expect based on what's there for the other groups:

// This file was generated by generate_static_pref_list.py from
// modules/libpref/init/StaticPrefList.yaml. DO NOT EDIT.
// Include it to gain access to StaticPrefs::embedlite_*.

#ifndef mozilla_StaticPrefs_embedlite_h
#define mozilla_StaticPrefs_embedlite_h

#include "mozilla/StaticPrefListBegin.h"
#include "mozilla/StaticPrefList_embedlite.h"
#include "mozilla/StaticPrefListEnd.h"

#endif  // mozilla_StaticPrefs_embedlite_h

So we're not missing the StaticPrefs_embedlite.h file, it's being correctly generated by generate_static_pref_list.py, it's just not being picked up by our #include directive.

I guess my task for today will be to figure out why not.

Doing a quick grep on the source tree shows that other examples of these headers are appearing in more of the generated output than the embedlite version is appearing in. For example in obj-build-mer-qt-xr/modules/libpref/backend.mk and in obj-build-mer-qt-xr/generated-sources.json. That must be a warning sign.

A bit more searching turns up the fact that the various groups are all listed in the pref_groups variable of the gecko-dev/modules/libpref/moz.build file. This turns out to be confirmed by the documentation at the head of the StaticPrefList.yaml file:

# Please follow the existing prefs naming convention when considering adding a
# new pref, and don't create a new pref group unless it's appropriate and there
# are likely to be multiple prefs within that group. (If you do, you'll need to
# update the `pref_groups` variable in modules/libpref/moz.build.)

It feels like there's a moral to be had about reading documentation here, but I can't quite put my finger on it. The good news is that there's nothing else in the documentation about other places the group needs to be added, so with any luck making this one change should be enough to do the trick.

I've added embedlite to the list of groups in the moz.build file.

pref_groups = [
    "accessibility",
    "alerts",
[...]
    "editor",
    "embedlite",
    "extensions",
    "findbar",
[...]

I'll need to do another full rebuild to see whether this has fixed anything.

Before I do that and call it a day, I want to try to pick off another of the JavaScript errors that are being output on startup. We have this issue #1041 about the EmbedLiteChromeManager component not being able to find a file:

JavaScript error: file:///usr/lib64/mozembedlite/components/
    EmbedLiteChromeManager.js, line 213: NS_ERROR_FILE_NOT_FOUND: 

Maybe, just maybe, this will be an easy one to track down and fix.

This file is part of the embedlite-components repository. Line 213, the place that's generating the error, doesn't look too auspicious:

      AboutCertViewerHandler.init();

Here's that line with a bit more context:

  observe(aSubject, aTopic, aData) {
    let self = this;
    switch (aTopic) {
[...]
    case "browser-delayed-startup-finished":
      AboutCertViewerHandler.init();
      Services.obs.removeObserver(this, "browser-delayed-startup-finished");
      break;
[...]

This AboutCertViewerHandler is defined lazily, so it could well be that the problem is a missing AboutCertViewerHandler.jsm source file:

XPCOMUtils.defineLazyModuleGetters(this, {
  AboutCertViewerHandler: "resource://gre/modules/AboutCertViewerHandler.jsm",
  ContentLinkHandler: "chrome://embedlite/content/ContentLinkHandler.jsm",
  Feeds: "chrome://embedlite/content/Feeds.jsm"
});

In that list, both ContentLinkHandler.jsm and Feeds.jsm can be found in the embedlite-components code.

On the other hand, there is a file called AboutCertViewerHandler.jsm in the ESR 78 gecko source that's been removed from the ESR 91 source. Well, not removed, but renamed:

$ git diff 92c10e5f503ad8e~ 92c10e5f503ad8e
[...]
diff --git a/toolkit/components/certviewer/AboutCertViewerHandler.jsm
           b/toolkit/components/certviewer/AboutCertViewerParent.jsm
similarity index 50%
rename from toolkit/components/certviewer/AboutCertViewerHandler.jsm
rename to toolkit/components/certviewer/AboutCertViewerParent.jsm
index 390fa3113836..16cedeafee9d 100644
--- a/toolkit/components/certviewer/AboutCertViewerHandler.jsm
+++ b/toolkit/components/certviewer/AboutCertViewerParent.jsm

What was AboutCertViewerHandler.jsm is apparently now AboutCertViewerParent.jsm. For reference, here's the commit message, although I'm not sure this on its own is super-enlightening.

$ git log -1 92c10e5f503ad8e16424e554f8fb6393ff77152f
commit 92c10e5f503ad8e16424e554f8fb6393ff77152f
Author: Neil Deakin <neil@mozilla.com>
Date:   Thu Jun 25 01:13:05 2020 +0000

    Bug 1646197, convert about:certificate to JSWindowActor instead of old RPM,
    r=johannh
    
    Differential Revision: https://phabricator.services.mozilla.com/D80017

The actual changeset D80017 associated with this is more helpful though. It makes clear that both the init() and uninit() have both been removed and don't need to be called any more. So we can remove them from our embedlite-components code too. We do need to check whether we're using this certificate code anywhere though.

A swift grep of the code shows it is being used, but only in the existing gecko code (none of the EmbedLite additions) and only in EmbedLiteChromeManager.js. In the latter it's only being used for init() and uninit(), so hopefully that means there are no other changes to make apart from removing these references.

The truth of whether it's needed in some other way may only become apparent later if there's some other broken functionality, but my gut tells me this won't happen.

These two calls that we've removed seem to be the only reason why we're interested in the "browser-delayed-startup-finished" and "xpcom-shutdown" event messages as well, so I've also removed the observers related to these:

$ git diff
diff --git a/jscomps/EmbedLiteChromeManager.js b/jscomps/EmbedLiteChromeManager.js
index da6151b..60e2aef 100644
--- a/jscomps/EmbedLiteChromeManager.js
+++ b/jscomps/EmbedLiteChromeManager.js
@@ -17,7 +17,6 @@ const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 const { NetErrorHelper } = ChromeUtils.import("chrome://embedlite/content/NetErrorHelper.jsm")
 
 XPCOMUtils.defineLazyModuleGetters(this, {
-  AboutCertViewerHandler: "resource://gre/modules/AboutCertViewerHandler.jsm",
   ContentLinkHandler: "chrome://embedlite/content/ContentLinkHandler.jsm",
   Feeds: "chrome://embedlite/content/Feeds.jsm"
 });
@@ -137,8 +136,6 @@ EmbedLiteChromeManager.prototype = {
     Services.obs.addObserver(this, "embed-network-link-status", true)
     Services.obs.addObserver(this, "domwindowclosed", true);
     Services.obs.addObserver(this, "keyword-uri-fixup", true);
-    Services.obs.addObserver(this, "browser-delayed-startup-finished");
-    Services.obs.addObserver(this, "xpcom-shutdown");
   },
 
   onWindowOpen(aWindow) {
@@ -209,13 +206,6 @@ EmbedLiteChromeManager.prototype = {
       Services.io.offline = network.offline;
       Services.obs.notifyObservers(null, "network:link-status-changed",
                                    network.offline ? "down" : "up");
-    case "browser-delayed-startup-finished":
-      AboutCertViewerHandler.init();
-      Services.obs.removeObserver(this, "browser-delayed-startup-finished");
-      break;
-    case "xpcom-shutdown":
-      AboutCertViewerHandler.uninit();
-      break;
     default:
       Logger.debug("EmbedLiteChromeManager subject", aSubject, "topic:", aTopic);
     }

Removing code from our downstream changes always fills me with a warm and fuzzy feeling. Less code to maintain in future.

It's worth noting that this change is essentially reversing commit b1510a7a that added the initialisation and deinitialisation calls in to the EmbedLite code.

The commit message doesn't provide a huge amount of context, but it doesn't appear to be a smaller part of a larger changeset, so hopefully this is an atomic change without other consequences elsewhere.

$ git log -1 b1510a7a
commit b1510a7a5771906e44b4247bd75271c1bb5c54f6 (upstream/jb56094, andrew-korolev-omp/jb56094)
Author: Andrew den Exter <andrew.den.exter@jolla.com>
Date:   Mon Nov 15 06:46:16 2021 +0000

    [embedlite-components] Initialize the certificate viewer handler.
    JB#56094 OMP#JOLLA-492

Okay, I've made and committed these changes; I've built and installed the resulting embedlite-components packages. The errors seen in the console output are now gone and I don't see any new issues arising.

I think that's enough for today. I'm going to hit the main gecko-dev build off and see what happens with it tomorrow.

If you'd like to catch up on all the diary entries, they're all available on my Gecko-dev Diary page.