flypig.co.uk

This morning the build has finished. I'm always happy to see that! I've transferred the packages over to my development phone and installed them.

You'll recall that yesterday and the day before we were working to remove the Services.appinfo errors from the log output while running the browser. The attempted fix was to add some annotations to our @mozilla.org/xre/app-info;1 interface specified in embedding/embedlite/components/components.conf that overrides the standard interface from toolkit/xre/components.conf.

I didn't see any easy way to test it without a rebuild, so has it done the trick?

Let's give it a whirl.

$ EMBED_CONSOLE=1 sailfish-browser
[D] unknown:0 - Using Wayland-EGL
library "libGLESv2_adreno.so" not found
library "eglSubDriverAndroid.so" not found
greHome from GRE_HOME:/usr/bin
libxul.so is not found, in /usr/bin/libxul.so
Created LOG for EmbedLiteTrace
[W] unknown:0 - Unable to open bookmarks
    "/home/defaultuser/.local/share/org.sailfishos/browser/bookmarks.json"
[D] onCompleted:105 - ViewPlaceholder requires a SilicaFlickable parent
Created LOG for EmbedLite
JSComp: EmbedLiteConsoleListener.js loaded
JSComp: ContentPermissionManager.js loaded
JSComp: EmbedLiteChromeManager.js loaded
JSComp: EmbedLiteErrorPageHandler.js loaded
JSComp: EmbedLiteFaviconService.js loaded
JavaScript error:
    file:///usr/lib64/mozembedlite/components/EmbedLiteGlobalHelper.js, line 108:
    TypeError: XPCOMUtils.generateNSGetFactory is not a function
JSComp: EmbedLiteOrientationChangeHandler.js loaded
JSComp: EmbedLiteSearchEngine.js loaded
JSComp: EmbedLiteSyncService.js loaded
EmbedLiteSyncService app-startup
JSComp: EmbedLiteWebrtcUI.js: loaded
JSComp: EmbedLiteWebrtcUI.js: got app-startup
JSComp: EmbedPrefService.js loaded
EmbedPrefService app-startup
JSComp: EmbedliteDownloadManager.js loaded
JSComp: LoginsHelper.js loaded
JSComp: PrivateDataManager.js loaded
JSComp: UserAgentOverrideHelper.js loaded
UserAgentOverrideHelper app-startup
CONSOLE message:
[JavaScript Error: "TypeError: XPCOMUtils.generateNSGetFactory is not a function"
    {file: "file:///usr/lib64/mozembedlite/components/EmbedLiteGlobalHelper.js"
    line: 108}]
@file:///usr/lib64/mozembedlite/components/EmbedLiteGlobalHelper.js:108:32
[...]

There's still plenty of output but the Services.appinfo errors are now all gone. Great!

Once I've pushed these changes I'll close the associated issue ticket.

It would be good to clean out all of the really obvious errors from the log. The next one seems to be this:

JavaScript error:
    file:///usr/lib64/mozembedlite/components/EmbedLiteGlobalHelper.js, line 108:
    TypeError: XPCOMUtils.generateNSGetFactory is not a function

There are a bunch of other errors related to mozembedlite/components components too. During the Sailfish OS Community Meeting this week Raine offered to help with the embedlite-components errors, so I'll need to be a little careful about which ones I pick to work on, to avoid us working on the same thing.

So my next task will be to pick out all of the obvious errors I can see and list them in the issue tracker, so that we can assign them and avoid any conflicts.

I also really need to move my changes over to the official repository. It's definitely reached that point now, I just really don't want to have to turn everything into patches as it's going to be a lot of work. But it needs to be done; maybe I'll have time over the weekend.

It's the end of the morning now, time for work!

[...]

The evening! Time top sort out some of these errors. Here's the list that I can see just at the moment. Raine already submitted issue tickets for several of them and I've created some additional tickets to cover the rest:

1. #1042 : JavaScript error: chrome://embedlite/content/embedhelper.js, line 259: TypeError: sessionHistory is null
2. #1044: JavaScript Error: "TypeError: XPCOMUtils.generateNSGetFactory is not a function" {file: "file:///usr/lib64/mozembedlite/components/EmbedLiteGlobalHelper.js" line: 108}
3. #1045 : JavaScript Error: "Unexpected event profile-after-change" {file: "resource://gre/modules/URLQueryStrippingListService.jsm" line: 224}
4. #1041: JavaScript Error: "NS_ERROR_FILE_NOT_FOUND: " {file: "file:///usr/lib64/mozembedlite/components/EmbedLiteChromeManager.js" line: 213}
5. #1046: JavaScript Error: "TypeError: Services.search.addEngine is not a function" {file: "file:///usr/lib64/mozembedlite/components/EmbedLiteSearchEngine.js" line: 53}

My plan was to look at issue #1044 and I think that's a good one to continue with:

JavaScript Error: "TypeError: XPCOMUtils.generateNSGetFactory is not a function"
    {file: "file:///usr/lib64/mozembedlite/components/EmbedLiteGlobalHelper.js"
    line: 108}

This EmbedLiteGlobalHelper.js file isn't part of the gecko-dev source tree at all but can instead be found in embedlite-components.

Here's the line that's causing the trouble:

this.NSGetFactory = XPCOMUtils.generateNSGetFactory([EmbedLiteGlobalHelper]);

To figure out what's going on I'll need to dig into this generateNSGetFactory() method. It's used quite a lot in the ESR 78 codebase (33 instances) and also in the embedlite-components code (25 times). It's also used quite liberally in the ESR 91 codebase too (29 instances). So why would it be generating an error in this particular place I wonder?

Something I notice while looking through these instances is that some look like this:

const { XPCOMUtils } = ChromeUtils.import(
  "resource://gre/modules/XPCOMUtils.jsm"
);
[...]
this.NSGetFactory = XPCOMUtils.generateNSGetFactory([EmbedLiteGlobalHelper]);

Whereas others look like this:

const { ComponentUtils } = ChromeUtils.import(
  "resource://gre/modules/ComponentUtils.jsm"
);
[...]
this.NSGetFactory = ComponentUtils.generateNSGetFactory([HandlerService]);

Could this be the reason? It seems worth a try. In order to test this out I also need to find out where the EmbedLiteGlobalHelper.js file ends up once it gets installed onto a phone. This shouldn't be too hard to figure out. There are probably clever, correct, ways to do this, but this way should also work:

$ find /usr -iname "EmbedLiteGlobalHelper.js" 2>/dev/null
/usr/lib64/mozembedlite/components/EmbedLiteGlobalHelper.js

I've edited the file on-device to make the following changes:

$ git diff
--- a/EmbedLiteGlobalHelper.js
+++ b/EmbedLiteGlobalHelper.js
@@ -6,7 +6,7 @@ const Cc = Components.classes;
 const Ci = Components.interfaces;
 const Cr = Components.results;
 
-const { XPCOMUtils } = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
+const { ComponentUtils } = ChromeUtils.import("resource://gre/modules/ComponentUtils.jsm");
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 const { LoginManagerParent } = ChromeUtils.import("resource://gre/modules/LoginManagerParent.jsm");
 const { L10nRegistry, FileSource } = ChromeUtils.import("resource://gre/modules/L10nRegistry.jsm");
@@ -105,4 +105,4 @@ EmbedLiteGlobalHelper.prototype = {
   QueryInterface: ChromeUtils.generateQI([Ci.nsIObserver, Ci.nsISupportsWeakReference, Ci.nsIFormSubmitObserver])
 };
 
-this.NSGetFactory = XPCOMUtils.generateNSGetFactory([EmbedLiteGlobalHelper]);
+this.NSGetFactory = ComponentUtils.generateNSGetFactory([EmbedLiteGlobalHelper]);

Happily the "XPCOMUtils.generateNSGetFactory is not a function" error no longer appears, but now I get a couple of new errors:

JavaScript error: file:///usr/lib64/mozembedlite/components/EmbedLiteGlobalHelper.js,
    line 32: TypeError: ActorManagerParent.flush is not a function
JavaScript error: file:///usr/lib64/mozembedlite/components/EmbedLiteGlobalHelper.js,
    line 34: TypeError: L10nRegistry.registerSource is not a function

These errors are a positive sign: they suggest execution is getting further into the module than it did before. To take the first error, I can see the flush() method in the ESR 78 code. Here's what it looks like:

  /**
   * Serializes the current set of registered actors into ppmm.sharedData, for
   * use by ActorManagerChild. This must be called before any frame message
   * managers have been created. It will have no effect on existing message
   * managers.
   */
  flush() {
    for (let [name, data] of this.childGroups) {
      sharedData.set(`ChildActors:${name || ""}`, data);
    }
    sharedData.set("ChildSingletonActors", this.singletons);
  },

This has been removed from the ESR 91 code though, which is why the error is triggering. Let's find out why it was removed.

$ git log -S "flush" -1 ./toolkit/modules/ActorManagerParent.jsm
commit c5eff6620590dbf2c5eceb62a1c42fe8e71c8f48
Author: Neil Deakin 
Date:   Fri Nov 6 15:46:11 2020 +0000

    Bug 1649843, remove now unused legacy actor code, r=kmag
    
    Differential Revision: https://phabricator.services.mozilla.com/D95206

What I'm seeing in this diff is that all of the calls to ActorManagerParent.flush() have been removed. It's not being immediately replaced by something in this diff, but that doesn't mean that there wasn't something else added earlier (or later). In many places I'm seeing calls like this, which don't seem to appear in the EmbedLite code:

    ActorManagerParent.addJSProcessActors(JSPROCESSACTORS);
    ActorManagerParent.addJSWindowActors(JSWINDOWACTORS);

These all seem to relate to JSActors which have some associated documentation. But that's a rabbit hole that's going to be too complex to go down today. So today I'm just going to remove the call to flush() to match what's happening in the diff. Maybe we'll have to return to this later.

Now let's try to tackle the second error related to L10nRegistry.registerSource(). Checking the ESR 78 and ESR 91 code against each other, it looks like L10nRegistry.registerSources() (plural) is now used instead. Let's check this.

First from ESR 78:

  /**
   * Adds a new resource source to the L10nRegistry.
   *
   * @param {FileSource} source
   */
  registerSource(source) {
    if (this.hasSource(source.name)) {
      throw new Error(`Source with name "${source.name}" already registered.`);
    }
    this.sources.set(source.name, source);

    if (isParentProcess) {
      this._synchronizeSharedData();
      Services.locale.availableLocales = this.getAvailableLocales();
    }
  }

And then from ESR 91:

  /**
   * Adds new resource source(s) to the L10nRegistry.
   *
   * Notice: Each invocation of this method flushes any changes out to extant
   * content processes, which is expensive. Please coalesce multiple
   * registrations into a single sources array and then call this method once.
   *
   * @param {Array} sources
   */
  registerSources(sources) {
    for (const source of sources) {
      if (this.hasSource(source.name)) {
        throw new Error(`Source with name "${source.name}" already registered.`);
      }
      this.sources.set(source.name, source);
    }
    if (isParentProcess && sources.length > 0) {
      this._synchronizeSharedData();
      Services.locale.availableLocales = this.getAvailableLocales();
    }
  }

So they are different, but in practice it just means that we need to pass in an array of resources, rather than just a single resource. Checking with git blame also takes us back to the upstream diff and associated Bugzilla bug:

$ git log -1 391ecb85154ba
commit 391ecb85154bab7416ab01f3de35f506327704c8
Author: Aaron Klotz 
Date:   Fri Jun 26 21:04:21 2020 +0000

    Bug 1648631: Part 1 - Make L10nRegistryService._synchronizeSharedData() explicitly flush and convert source registration to accept arrays; r=zbraniecki
    
    Setting a key/value pair on the parent process message manager's `sharedData`
    initiates a pending runnable that runs at **idle priority**. If the current
    thread never gets a chance to idle, then those registry changes will never be
    synchronized.
    
    This patch adds an explicit flush so that `_synchronizeSharedData` does indeed
    actually synchronize.
    
    We replace the scalar `registerSource`, `updateSource`, and `removeSource` with
    variants that accept arrays instead. This allows us to process multiple
    registration changes while deferring the synchronization until the entire list
    has been processed. The scalar variants were removed to future-proof against
    perf issues.
    
    Differential Revision: https://phabricator.services.mozilla.com/D81243

That looks like pretty good corroboration.

Here are the remaining changes I've made to address these two errors:

 function EmbedLiteGlobalHelper()
 {
-  ActorManagerParent.flush();
-
-  L10nRegistry.registerSource(new FileSource(
-                                  "0-mozembedlite",
-                                  ["en-US", "fi", "ru"],
-                                  "chrome://browser/content/localization/{locale}/"))
+  L10nRegistry.registerSources([new FileSource(
+                                   "0-mozembedlite",
+                                   ["en-US", "fi", "ru"],
+                                   "chrome://browser/content/localization/{locale}/")])
 
   Logger.debug("JSComp: EmbedLiteGlobalHelper.js loaded");
 }

I've edited these changes directly on my device and now when I run the browser I just see the following coming up in relation to EmbedLiteGlobalHelper.js:

JSComp: EmbedLiteGlobalHelper.js loaded
EmbedLiteGlobalHelper app-startup

Much better! As I mentioned before, these changes aren't part of the gecko-dev codebase but rather make their way into the embedlite-components repository instead.

That'll have to be all for today. More tomorrow!

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