flypig.co.uk

I'm continuing to investigate the best way to re-enable the OnFirstPaint() call. I've now been through the code — and the patch — several times to see if I can find a way to avoid manually applying the patch in full.

The actual purpose of the patch appears to be to set up some timers so that the NotifyDidPaintForSubtree() method is triggered after the screen paint has completed. Here's where the timer is created and kicked off:

  nsCOMPtr<nsITimer> timer;
  RefPtr<nsRootPresContext> self = this;
  nsresult rv = NS_NewTimerWithCallback(
    getter_AddRefs(timer),
    NewNamedTimerCallback([self, aTransactionId](){
      nsAutoScriptBlocker blockScripts;
      self->NotifyDidPaintForSubtree(aTransactionId);
     }, &quot;NotifyDidPaintForSubtree&quot;), 100, nsITimer::TYPE_ONE_SHOT,
    Document()->EventTargetFor(TaskCategory::Other));

This piece of code would be added to the nsPresContext.cpp source file by the patch. As you can see it creates a one-shot timer that runs for 100 milliseconds (one tench of a second) before calling the NotifyDidPaintForSubtree() method.

A lot of the surrounding code is all related to ensuring the timer does — or doesn't — run under various scenarios, such as after the window has been destroyed.

Looking carefully through the patch, applying it to the most recent ESR 91 code doesn't seem to be too awkward. However the code after applying the patch seems to use mPendingTransaction and mCompletedTransaction in contrast to the original code that uses mNextTransactionId, mOutstandingTransactionId and mCompletedTransaction.

In the new ESR 91 code these have been switched out for an array called mPendingTransactions. In order to help get things straight in my head I'm going to try to summarise what each of these is used for.

In the non-patched ESR 78 code, all of these are of type TransactionId, which is a class, but essentially one that's a wrapper for a monotonically increasing uint64_t used as an identifier. Here are the three instances and what they're for in ESR 78 (before patch 0044 is applied):

• mNextTransactionId: The most recently allocated transaction id.
• mOutstandingTransactionId: Captures mCompletedTransaction + (pending transaction count).
• mCompletedTransaction: The most recently completed transaction id.

Based on this we always have mNextTransactionId >= mOutstandingTransactionId >= mCompletedTransaction. The number of transactions not yet completed is given by (mOutstandingTransactionId - mCompletedTransaction).

Because the ids are monotonically increasing, all three of these variables are needed to keep track of things. After applying the ESR 78 patch the arrangement is rejigged so that only two variables are needed:

• mPendingTransaction: The most recently allocated transaction id.
• mCompletedTransaction: The most recently completed transaction id.

Here mPendingTransaction is replacing mNextTransactionId. Both increment when a new transaction is created. However when a transaction is revoked mPendingTransaction in the post-patch version of the code is decreased, whereas in the pre-patch version it was mOutstandingTransactionId that decreased.

Consequently if we want to find out how many open transactions there are to complete in the post-patch ESR 78 build, it now becomes (mPendingTransaction - mCompletedTransaction).

In the non-patched ESR 91 code this has all changed quite substantially. Now we have a variable to ensure ids aren't repeated, plus an array of TransactionId objects.

• mNextTransactionId: The most recently allocated transaction id.
• mPendingTransactions: An extensible array of transaction ids, representing all open transactions.

This gives a more flexible arrangement. When transactions are created mNextTransactionId is incremented and used to get the next id. This way ids are monotonically increasing. The newly minted id is added to the mPendingTransactions array when it's opened and removed from the array when it's revoked.

With this arrangement we can use mPendingTransactions.Length() to find out how many open transactions there are.

As you can see from this the ESR 91 approach supports everything that the other two approaches support. So there's no point in tearing it down and replacing it with one of the earlier approaches. Better to amend the changes to the ESR 91 codebase to suit this new approach.

What does this mean in practice? It means that the changes patching nsPresContext.cpp need to go in as before, but the changes in nsRefreshDriver.cpp can be greatly simplified. Most of the changes simply aren't needed if we're not having to change the way these ids are being managed. In fact the only change we need to make to this file is to drop the call to NotifyRevokingDidPaint():

$ git diff layout/base/nsRefreshDriver.cpp
diff --git a/layout/base/nsRefreshDriver.cpp b/layout/base/nsRefreshDriver.cpp
index 7726971e63c9..34406f9cf2fe 100644
--- a/layout/base/nsRefreshDriver.cpp
+++ b/layout/base/nsRefreshDriver.cpp
@@ -2647,12 +2647,6 @@ void nsRefreshDriver::RevokeTransactionId(
     FinishedWaitingForTransaction();
   }
 
-  // Notify the pres context so that it can deliver MozAfterPaint for this
-  // id if any caller was expecting it.
-  nsPresContext* pc = GetPresContext();
-  if (pc) {
-    pc->NotifyRevokingDidPaint(aTransactionId);
-  }
   // Remove aTransactionId from the set of outstanding transactions since we're
   // no longer waiting on it to be completed, but don't revert
   // mNextTransactionId since we can't use the id again.

I've applied all of these changes to the code — manually this time — and checked through them. This time I've tried to understand what each change is doing before applying it, so I'm much more confident of about the result this time. I've started the build off so that it's all set to run overnight. Let's see how this has gone when we return to it in the morning.

Nothing builds first time of course, but whatever errors are thrown up should hopefully be more straightforward to deal with than the errors I got with my previous attempts.

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