flypig.co.uk

This morning I woke up early unable to sleep. Last night I set a build running and although it's not nervousness about the build that's preventing me from sleeping, it does nevertheless provide a good opportunity for me to check the progress of the build.

Unfortunately the build that was running overnight has failed. There are errors resulting from the changes I made to nsPresContext.cpp, the first of which looks like this:

205:02.94 In file included from Unified_cpp_layout_base2.cpp:20:
205:02.94 ${PROJECT}/gecko-dev/layout/base/nsPresContext.cpp: In member 
    function ‘void nsPresContext::DetachPresShell()’:
205:02.94 ${PROJECT}/gecko-dev/layout/base/nsPresContext.cpp:839:3: error: 
    ‘thisRoot’ was not declared in this scope
205:02.94    thisRoot->CancelAllDidPaintTimers();
205:02.94    ^~~~~~~~
205:02.95 ${PROJECT}/gecko-dev/layout/base/nsPresContext.cpp:839:3: note: 
    suggested alternative: ‘IsRoot’
205:02.95    thisRoot->CancelAllDidPaintTimers();
205:02.95    ^~~~~~~~
205:02.95    IsRoot

The problem here is an attempt to use the thisRoot variable in the nsPresContext::DetachPresShell() method without it having been declared. Here's the specific line that's failing from the ESR 91 code:

  // The did-paint timer also depends on a non-null pres shell.
  thisRoot->CancelAllDidPaintTimers();

In comparison, here's what the equivalent code looks like in ESR 78:

  if (IsRoot()) {
    nsRootPresContext* thisRoot = static_cast<nsRootPresContext*>(this);

    // Have to cancel our plugin geometry timer, because the
    // callback for that depends on a non-null presshell.
    thisRoot->CancelApplyPluginGeometryTimer();

    // The did-paint timer also depends on a non-null pres shell.
    thisRoot->CancelAllDidPaintTimers();
  }

Note that both of these blocks of code are post-patch. That is, they're the code as it looks after patch 0044 (or my manual changes in the case of ESR 91) have been applied. Looking at the ESR 78 code gives an immediate idea about what the code ought to look like in ESR 91. I've made changes in line with this so that now the ESR 91 code looks like this:

  if (IsRoot()) {
    nsRootPresContext* thisRoot = static_cast<nsRootPresContext*>(this);

    // The did-paint timer also depends on a non-null pres shell.
    thisRoot->CancelAllDidPaintTimers();
  }

That should do the trick. There's another error in the build output though, which is the following:

205:03.09 ${PROJECT}/gecko-dev/layout/base/nsPresContext.cpp: At global scope:
205:03.09 ${PROJECT}/gecko-dev/layout/base/nsPresContext.cpp:2677:1: error: 
    ‘mOutstandingTransactionIdvoid’ does not name a type
205:03.09  mOutstandingTransactionIdvoid
205:03.09  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

To understand this better it helps once again to compare the ESR 78 and ESR 91 code. But this time I'm going to switch it around so that we see the ESR 78 code first. Here's what it looks like there:

void
nsRootPresContext::EnsureEventualDidPaintEvent(TransactionId aTransactionId)
{

And here's what it looks like in ESR 91:

mOutstandingTransactionIdvoid
nsRootPresContext::EnsureEventualDidPaintEvent(TransactionId aTransactionId)
{

Notice the error? Honestly I don't know what's going on here but I suspect I accidentally hit Ctrl-V at some point when the cursor was in the wrong place and pasted the variable in the buffer in front of the return type.

Removing the extraneous mOutstandingTransactionIdvoid so it's restored back to the original code seems to be the correct fix here.

These are the only two errors thrown up by the compiler, so after fixing them both I've set the build running again.

Unfortunately the next attempt to build also fails. This time the compilation stages all pass without incident, it gets right up to the linking stage before throwing an undefined reference error. Apparently I've forgotten to define nsRootPresContext::Detach():

216:06.48 toolkit/library/build/libxul.so
219:52.11 opt/cross/bin/aarch64-meego-linux-gnu-ld: 
    ../../../layout/base/Unified_cpp_layout_base2.o:(
    .data.rel.ro.local._ZTV17nsRootPresContext[_ZTV17nsRootPresContext]+0x38): 
    undefined reference to `nsRootPresContext::Detach()'
219:52.11 opt/cross/bin/aarch64-meego-linux-gnu-ld: 
    libxul.so: hidden symbol `_ZN17nsRootPresContext6DetachEv' isn't defined
219:52.11 opt/cross/bin/aarch64-meego-linux-gnu-ld: 
    final link failed: bad value
219:52.11 collect2: error: ld returned 1 exit status

Sure enough the nsRootPresContext::Detach() method is defined in the class header:

class nsRootPresContext final : public nsPresContext {
 public:
  nsRootPresContext(mozilla::dom::Document* aDocument, nsPresContextType aType);
  virtual ~nsRootPresContext();
  virtual bool IsRoot() override { return true; }

  virtual void Detach() override;
[...]

But there's no code to define the body of the method in the cpp file. Hence the error. The correct method body that should have been included can be seen in the 0044 patch file:

+/* virtual */
+void nsRootPresContext::Detach() {
+  CancelAllDidPaintTimers();
+  nsPresContext::Detach();
+}
+

For some reason this never made it in to my manually patched ESR 91 code. I must have missed it.

Both of the methods called in this method — nsRootPresContext::CancelAllDidPaintTimers() and nsPresContext::Detach() — are defined and accessible, so adding the missing code shouldn't break compilation. I've added it in and kicked the build off once again. The fact it got all the way through to linking on the previous attempt will hopefully mean that there are fewer source files to be recompiled this time around and the build will complete relatively swiftly.

Having said that, for the rest of the day I'll be travelling. That means a day of switching between various buses and trains. Not ideal when it comes to a lengthy build since, while I can allow the build to continue when I'm on the train, my laptop will have to be suspended and resumed as I move from one leg of the journey to the next.

Still, the build continues, on and off, throughout the day.

[...]

It's evening now and as I sit here at my destination after a day spent travelling, the build continues to run, which means I'll have to leave it running overnight. Let's hope I wake up to a complete build and an opportunity to test the result in the morning.

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