flypig.co.uk

Yesterday we started out working on getting QtMozEmbed to build. I made all the changes to QtMozEmbed that looked sensible and also added a new file — EmbedInitGlue.cpp to the Gecko code. The build of this was set to run overnight.

This morning I've unfortunately found the build failed. I actually spent a fair bit of last night getting the sfdk build target into a usable state. There were lots of problems getting the dependencies installed. So my initial response was exasperation: is this some intermittent sfdk Heisenbug? Thankfully not. On closer inspection it turns out to be an genuine error in the new EmbedInitGlue.cpp file that I added. Phew. That's likely to be much easier to track down and fix than an SDK issue.

219:03.60 mozglue/sailfishos
219:05.45 ${PROJECT}/gecko-dev/mozglue/sailfishos/EmbedInitGlue.cpp: In function
          ‘bool LoadEmbedLite(int, char**)’:
219:05.45 ${PROJECT}/gecko-dev/mozglue/sailfishos/EmbedInitGlue.cpp:89:55: error:
          no match for ‘operator=’ (operand types are ‘mozilla::Bootstrap::UniquePtr’
          {aka ‘mozilla::UniquePtr’}
          and ‘mozilla::BootstrapResult’ {aka
          ‘mozilla::Result, mozilla::Variant > > >’})
219:05.45    gBootstrap = mozilla::GetBootstrap(xpcomPath.c_str());
219:05.45                                                        ^

It looks like the problem here is the return value that's intended to end up stored in gBootstrap:

using BootstrapResult = ::mozilla::Result;

inline BootstrapResult GetBootstrap(const char* aXPCOMFile = nullptr) {

Upstream diff D104263 changed the return value from Bootstrap::UniquePtr to BootstrapResult with the definition above; so we'll need to update the EmbedInitGlue.cpp code to match.

That means we should also update the error condition from this ("check for null pointer"):

  gBootstrap = mozilla::GetBootstrap(xpcomPath.c_str());
  if (!gBootstrap) {
    printf("Couldn't load XPCOM from %s\n", xpcomPath.c_str());
    return false;
  }

To this ("check for explicit error return value"):

  BootstrapResult bootstrapResult;
  bootstrapResult = mozilla::GetBootstrap(xpcomPath.c_str());
  if (bootstrapResult.isErr()) {
    printf("Couldn't load XPCOM from %s\n", xpcomPath.c_str());
    return false;
  }
  gBootstrap = bootstrapResult.unwrap();

This change looks to me like it's coming from Rust, which has these nice Result enums which act similarly to this and force explicit error checks. It's very easy to forget to include a null-pointer check.

That's the only error coming out right now, so worth triggering another rebuild of Gecko.

Before I do that I want to run another build of QtMozEmbed to see whether the changes I've made have had a positive effect.

Obviously the EmbedInitGlue.h header will still be missing, but maybe the other changes I've made will throw up something new.

And they do! The following new errors now appear.

SailfishOS-devel-aarch64.temp/usr/include/xulrunner-qt5-91.9.1/
  nsReadableUtils.h: In function ‘bool EnsureUTF16Validity(nsAString&)’:
SailfishOS-devel-aarch64.temp/usr/include/xulrunner-qt5-91.9.1/
  nsReadableUtils.h:414:26: error: ‘Utf16ValidUpTo’ is not a member of ‘mozilla’
   size_t upTo = mozilla::Utf16ValidUpTo(aString);
                          ^~~~~~~~~~~~~~
SailfishOS-devel-aarch64.temp/usr/include/xulrunner-qt5-91.9.1/
  nsReadableUtils.h:425:12: error: ‘EnsureUtf16ValiditySpan’ is not a member of
  ‘mozilla’
   mozilla::EnsureUtf16ValiditySpan(span.From(upTo + 1));
            ^~~~~~~~~~~~~~~~~~~~~~~

Investigating these turns out to be a bit of a rabbit hole. The error is coming from the nsReadableUtils.h file. The functions the compiler claims are missing already exist in Gecko's TextUtils.h file. But nsReadableUtils.h does include TextUtils.h, so why isn't it picking it up?

My suspicion is that it relates to the MOZ_HAS_JSRUST preprocessor define which wraps the two functions. If this define is set to 1 the functions will be included, but if it's set to 0 they won't be.

Where is this set? It looks like it's in the MOZ_HAS_JSRUST file, which has this tangle of defines:

#if (defined(MOZ_HAS_MOZGLUE) || defined(MOZILLA_INTERNAL_API)) && \
    !defined(MOZ_PRETEND_NO_JSRUST)
#  define MOZ_HAS_JSRUST() 1
#else
#  define MOZ_HAS_JSRUST() 0
#endif

The mozilla/JsRust.h file is included in mozilla/Latin1.h, which is included in Textutils.h. So it's definitely getting in to the build.

It seems the problem here is that MOZILLA_INTERNAL_API isn't defined. Digging further into the logs I see this error being output:

SailfishOS-devel-aarch64.temp/usr/include/xulrunner-qt5-91.9.1/
  nsTSubstring.h:29:4: error: #error "Using XPCOM strings is limited to code
  linked into libxul."
 #  error "Using XPCOM strings is limited to code linked into libxul."
    ^~~~~

This error is also due to MOZILLA_INTERNAL_API not being defined.

I did also check that none of these files have changed since ESR 78. They haven't. So there must be something else happening with these defines.

To see whether it really is a lack of this define being set, I've forcefully set it in qopenglwebpage.cpp, which is the root of where all the header files cascade in from. Add this define doesn't help, so maybe I have this wrong.

At this point I notice that the reason the header file is being included at all is because of the changes I made yesterday. Adding in nsFocusManager::GenerateFocusActionId() forced me to also include the nsFocusManager.h header, which has triggered all of these problems.

So maybe I should row back on that addition? After all, we don't want to be accessing an internal API, so probably we shouldn't be using GenerateFocusActionId().

One potential way to tackle this is to push the code that generates the aActionId parameter inside the Gecko library, rather than trying to expose it for the browser wrapper code to deal with. That way the code can call GenerateFocusActionId() without having to worry about these errors (because MOZILLA_INTERNAL_API will be set). Since I'm still holding back a Gecko build, now would be a good time to make this change.

There's also this error to tackle in the list:

quickmozview.cpp: In destructor ‘virtual QuickMozView::~QuickMozView()’:
quickmozview.cpp:104:36: error: no matching function for call to
  ‘mozilla::embedlite::EmbedLiteView::SetIsActive(bool)’
         d->mView->SetIsActive(false);
                                    ^

This is another instance of SetIsActive() that needs the additional aActionID parameter. There's a similar error further down in the same file. The changes I've made to Gecko to remove this parameter should also result in these errors being resolved automatically, or so I believe.

That's the last of the errors that doesn't seem related to the missing EmbedInitGlue files.

So, time to run the Gecko build overnight so I can retry the QtMozEmbed build again in the morning.

If you'd like to read more about all this gecko stuff, do take a look at my full Gecko Dev Diary.