flypig.co.uk

List items

Items from the current list are shown below.

Blog

23 Jan 2024 : Day 147 #
Last night I woke up in a mild panic. This happens sometimes, usually when I have a lot going on and I feel like I'm in danger of dropping the ball.

This seems to be my mind's (or maybe my body's?) way of telling me that I need to get my priorities straight. That there's something that I need to get done, resolved or somehow dealt with, and I need to do it urgently or I'll continue to have sleepless nights until I do.

The reason for this particular panic was my FOSDEM preparations, combined with a build up of projects at work that are coming to a head. For FOSDEM I have two talks to prepare (one about this blog, the other related to my work), the Linux on Mobile stand to help organise, the Sailfish community dinner on the Saturday to help organise, support for the HPC, Big Data & Data Science devroom on the Saturday, and also with the Python devroom on the Sunday. It's going to be a crazy busy event. But it's actually the run-up to it and the fact I've to still write my presentations, that's losing me sleep.

It's all fine: it's under control. But in order to prevent it spiralling out of control, I'm going to be taking a break from gecko development for a couple of weeks until things have calmed down. This will slow down development, which of course saddens me because more than anything else I just want ESR 91 to end up in a good, releasable, state. But as others wiser than I am have already cautioned, this also means keeping a positive and healthy state of mind.

I'll finish my last post on Day 149 (that's this Thursday). I'll start back up again on Thursday the 8th February, assuming all goes to plan!

But for the next couple of days there's still development to be done, so let's get straight back to it.

Today I'm still attempting to fix the NS_ERROR_FILE_NOT_FOUND error coming from SessionStore.jsm which I believe may be causing the Back and Forwards buttons in the browser to fail. It's become clear that the SessionStore.jsm file itself is missing (along with a bunch of other files listed in gecko-dev/browser/components/sessionstore/moz.build) but what's not clear is whether the problem is that the files should be there, or that the calls to the methods in this file shouldn't be there.

Overnight while lying in bed I came up with some kind of plan to help move things forwards. The call to the failing method is happening in updateSessionStoreForWindow() and this is only called by the exported method UpdateSessionStoreForWindow(). As far as I can tell this isn't executed by any JavaScript code, but because it has an IPDL interface it's possible for it to be called by C++ code as well.

And sure enough, it's being called twice in WindowGlobalParent.cpp. Once at the end of the WriteFormDataAndScrollToSessionStore() method on line 1260, like this:
nsresult WindowGlobalParent::WriteFormDataAndScrollToSessionStore(
    const Maybe<FormData>& aFormData, const Maybe<nsPoint>& aScrollPosition,
    uint32_t aEpoch) {
[...]
  return funcs->UpdateSessionStoreForWindow(GetRootOwnerElement(), context, key,
                                            aEpoch, update);
}
And another time at the end of the ResetSessionStore() method on line 1310, like this:
nsresult WindowGlobalParent::ResetSessionStore(uint32_t aEpoch) {
[...]
  return funcs->UpdateSessionStoreForWindow(GetRootOwnerElement(), context, key,
                                            aEpoch, update);
}
I've placed a breakpoint on these two locations to find out whether these are actually where it's being fired from. If you're being super-observant you'll notice I've not actually placed the breakpoints where I actually want them; I've had to place them earlier in the code (but crucially, still within the same methods). That's because it's not always possible to place breakpoints on the exact line you want.

The debugger will place it on the first line it can after the point you request. Because both of the cases I'm interested in are right at the end of the methods they're called in, when I attempt to put a breakpoint on the exact line the debugger places it instead in the next method along in the source code. That isn't much use for what I needed.

Hence I've placed them a little earlier in the code instead: on the first lines where they actually stick.
bash-5.0$ EMBED_CONSOLE=1 MOZ_LOG="EmbedLite:5" gdb sailfish-browser
(gdb) b WindowGlobalParent.cpp:1260
Breakpoint 5 at 0x7fbbc8e31c: file dom/ipc/WindowGlobalParent.cpp, line 1260.
(gdb) b WindowGlobalParent.cpp:1294
Breakpoint 9 at 0x7fbbc8e688: file dom/ipc/WindowGlobalParent.cpp, line 1294.
(gdb) r
[...]
JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 541:
    NS_ERROR_FILE_NOT_FOUND: 
CONSOLE message:
[JavaScript Error: "NS_ERROR_FILE_NOT_FOUND: " {file:
    "resource:///modules/sessionstore/SessionStore.jsm" line: 541}]
@resource:///modules/sessionstore/SessionStore.jsm:541:3
SSF_updateSessionStoreForWindow@resource://gre/modules/
    SessionStoreFunctions.jsm:120:5
UpdateSessionStoreForStorage@resource://gre/modules/
    SessionStoreFunctions.jsm:54:35

Thread 10 "GeckoWorkerThre" hit Breakpoint 5, mozilla::dom::WindowGlobalParent::
    WriteFormDataAndScrollToSessionStore (this=this@entry=0x7f81164520, 
    aFormData=..., aScrollPosition=..., aEpoch=0)
    at dom/ipc/WindowGlobalParent.cpp:1260
1260      windowState.mHasChildren.Construct() = !context->Children().IsEmpty();
(gdb) n
709     ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/Span.h:
        No such file or directory.
(gdb) 
1260      windowState.mHasChildren.Construct() = !context->Children().IsEmpty();
(gdb) 
1262      JS::RootedValue update(jsapi.cx());
(gdb) 
1263      if (!ToJSValue(jsapi.cx(), windowState, &update)) {
(gdb) 
1267      JS::RootedValue key(jsapi.cx(), context->Top()->PermanentKey());
(gdb) 
1269      return funcs->UpdateSessionStoreForWindow(GetRootOwnerElement(),
          context, key,
(gdb) n
1297    ${PROJECT}/obj-build-mer-qt-xr/dist/include/js/RootingAPI.h:
        No such file or directory.
(gdb) 
JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 541:
    NS_ERROR_FILE_NOT_FOUND: 
1267      JS::RootedValue key(jsapi.cx(), context->Top()->PermanentKey());
(gdb) c
Continuing.
It's clear from the above that the WriteFormDataAndScrollToSessionStore() method is being called and is then going on to call the missing JavaScript method. We can even see the error coming from the JavaScript as we step out of the calling method.

You'll notice from the output that there's another — earlier — NS_ERROR_FILE_NOT_FOUND error before the breakpoint hits. This is coming from a different spot: line 120 of SessionStoreFunctions.jsm rather than the line 105 we were looking at here.

This new error is called from line 54 of the same file (we can see from from the backtrace in the log output) which is in the UpdateSessionStoreForStorage() method in the file. So where is this being called from?
$ grep -rIn "UpdateSessionStoreForStorage" * --include="*.js" \
    --include="*.jsm" --include="*.cpp" --exclude-dir="obj-build-mer-qt-xr"
gecko-dev/docshell/base/CanonicalBrowsingContext.cpp:2233:
    return funcs->UpdateSessionStoreForStorage(Top()->GetEmbedderElement(), this,
gecko-dev/docshell/base/CanonicalBrowsingContext.cpp:2255:
    void CanonicalBrowsingContext::UpdateSessionStoreForStorage(
gecko-dev/dom/storage/SessionStorageManager.cpp:854:
    CanonicalBrowsingContext::UpdateSessionStoreForStorage(
gecko-dev/toolkit/components/sessionstore/SessionStoreFunctions.jsm:47:
    function UpdateSessionStoreForStorage(
gecko-dev/toolkit/components/sessionstore/SessionStoreFunctions.jsm:66:
    "UpdateSessionStoreForStorage",
From this we can see it's being called in a few places, all of them from C++ code. Again, that means we can explore them with gdb using breakpoints. Let's give this a go as well.
(gdb) break CanonicalBrowsingContext.cpp:2213
Breakpoint 7 at 0x7fbc7c6abc: file docshell/base/CanonicalBrowsingContext.cpp,
    line 2213.
(gdb) r
[...]

Thread 10 "GeckoWorkerThre" hit Breakpoint 7, mozilla::dom::
    CanonicalBrowsingContext::WriteSessionStorageToSessionStore
    (this=0x7f80b6dee0, aSesssionStorage=..., aEpoch=0)
    at docshell/base/CanonicalBrowsingContext.cpp:2213
2213      AutoJSAPI jsapi;
(gdb) bt
#0  mozilla::dom::CanonicalBrowsingContext::WriteSessionStorageToSessionStore
    (this=0x7f80b6dee0, aSesssionStorage=..., aEpoch=0)
    at docshell/base/CanonicalBrowsingContext.cpp:2213
#1  0x0000007fbc7c6f54 in mozilla::dom::CanonicalBrowsingContext::
    <lambda(const mozilla::MozPromise<nsTArray<mozilla::dom::SSCacheCopy>,
    mozilla::ipc::ResponseRejectReason, true>::ResolveOrRejectValue&)>::
    operator() (valueList=..., __closure=0x7f80bd70c8)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/Variant.h:768
#2  mozilla::MozPromise<nsTArray<mozilla::dom::SSCacheCopy>, mozilla::ipc::
    ResponseRejectReason, true>::InvokeMethod<mozilla::dom::
    CanonicalBrowsingContext::UpdateSessionStoreSessionStorage(const
    std::function<void()>&)::<lambda(const mozilla::MozPromise<nsTArray
    <mozilla::dom::SSCacheCopy>, mozilla::ipc::ResponseRejectReason, true>::
    ResolveOrRejectValue&)>, void (mozilla::dom::CanonicalBrowsingContext::
    UpdateSessionStoreSessionStorage(const std::function<void()>&)::
    <lambda(const mozilla::MozPromise<nsTArray<mozilla::dom::SSCacheCopy>,
    mozilla::ipc::ResponseRejectReason, true>::ResolveOrRejectValue&)>::*)
    (const mozilla::MozPromise<nsTArray<mozilla::dom::SSCacheCopy>,
    mozilla::ipc::ResponseRejectReason, true>::ResolveOrRejectValue&) const,
    mozilla::MozPromise<nsTArray<mozilla::dom::SSCacheCopy>, mozilla::ipc::
    ResponseRejectReason, true>::ResolveOrRejectValue>
    (aValue=..., aMethod=<optimized out>, 
[...]
#25 0x0000007fb78b289c in ?? () from /lib64/libc.so.6
(gdb) n
[LWP 8594 exited]
2214      if (!jsapi.Init(wrapped->GetJSObjectGlobal())) {
(gdb) 
2218      JS::RootedValue key(jsapi.cx(), Top()->PermanentKey());
(gdb) 
2220      Record<nsCString, Record<nsString, nsString>> storage;
(gdb) 
2221      JS::RootedValue update(jsapi.cx());
(gdb) 
2223      if (!aSesssionStorage.IsEmpty()) {
(gdb) 
2230        update.setNull();
(gdb) 
2233      return funcs->UpdateSessionStoreForStorage(Top()->
          GetEmbedderElement(), this,
(gdb) 
1297    ${PROJECT}/obj-build-mer-qt-xr/dist/include/js/RootingAPI.h:
        No such file or directory.
(gdb) 
JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 541:
    NS_ERROR_FILE_NOT_FOUND: 
2221      JS::RootedValue update(jsapi.cx());
(gdb) 
2220      Record<nsCString, Record<nsString, nsString>> storage;
(gdb) 
This new breakpoint is hit and once again, stepping through the code shows the problem method being called and also triggering the JavaScript error we're concerned about.

This is all good stuff. Looking through the ESR 78 code there doesn't appear to be anything equivalent in CanonicalBrowsingContext.cpp. But now that I know this is where the problem is happening, I can at least find the commit that introduced the changes and use that to find out more. Here's the output from git blame, but please forgive the terrible formatting: it's very hard to line-wrap this output cleanly.
$ git blame docshell/base/CanonicalBrowsingContext.cpp \
    -L :WriteSessionStorageToSessionStore
dd51467 (Andreas Farre 2021-05-26 2204) nsresult CanonicalBrowsingContext::
                                        WriteSessionStorageToSessionStore(
dd51467 (Andreas Farre 2021-05-26 2205)     const nsTArray<SSCacheCopy>&
                                            aSesssionStorage, uint32_t aEpoch) {
dd51467 (Andreas Farre 2021-05-26 2206)   nsCOMPtr<nsISessionStoreFunctions> funcs =
dd51467 (Andreas Farre 2021-05-26 2207)       do_ImportModule("resource://gre/
                                              modules/SessionStoreFunctions.jsm");
dd51467 (Andreas Farre 2021-05-26 2208)   if (!funcs) {
dd51467 (Andreas Farre 2021-05-26 2209)     return NS_ERROR_FAILURE;
dd51467 (Andreas Farre 2021-05-26 2210)   }
dd51467 (Andreas Farre 2021-05-26 2211) 
dd51467 (Andreas Farre 2021-05-26 2212)   nsCOMPtr<nsIXPConnectWrappedJS>
                                          wrapped = do_QueryInterface(funcs);
dd51467 (Andreas Farre 2021-05-26 2213)   AutoJSAPI jsapi;
dd51467 (Andreas Farre 2021-05-26 2214)   if (!jsapi.Init(wrapped->
                                            GetJSObjectGlobal())) {
dd51467 (Andreas Farre 2021-05-26 2215)     return NS_ERROR_FAILURE;
dd51467 (Andreas Farre 2021-05-26 2216)   }
dd51467 (Andreas Farre 2021-05-26 2217) 
2b70b9d (Kashav Madan  2021-06-26 2218)   JS::RootedValue key(jsapi.cx(),
                                            Top()->PermanentKey());
2b70b9d (Kashav Madan  2021-06-26 2219) 
dd51467 (Andreas Farre 2021-05-26 2220)   Record<nsCString, Record<nsString,
                                            nsString>> storage;
dd51467 (Andreas Farre 2021-05-26 2221)   JS::RootedValue update(jsapi.cx());
dd51467 (Andreas Farre 2021-05-26 2222) 
dd51467 (Andreas Farre 2021-05-26 2223)   if (!aSesssionStorage.IsEmpty()) {
dd51467 (Andreas Farre 2021-05-26 2224)     SessionStoreUtils::
                                              ConstructSessionStorageValues(this,
                                              aSesssionStorage,
dd51467 (Andreas Farre 2021-05-26 2225)                                storage);
dd51467 (Andreas Farre 2021-05-26 2226)     if (!ToJSValue(jsapi.cx(), storage,
                                              &update)) {
dd51467 (Andreas Farre 2021-05-26 2227)       return NS_ERROR_FAILURE;
dd51467 (Andreas Farre 2021-05-26 2228)     }
dd51467 (Andreas Farre 2021-05-26 2229)   } else {
dd51467 (Andreas Farre 2021-05-26 2230)     update.setNull();
dd51467 (Andreas Farre 2021-05-26 2231)   }
dd51467 (Andreas Farre 2021-05-26 2232) 
2b70b9d (Kashav Madan  2021-06-26 2233)   return funcs->
                                            UpdateSessionStoreForStorage(
                                            Top()->GetEmbedderElement(), this,
2b70b9d (Kashav Madan  2021-06-26 2234)                    key, aEpoch, update);
dd51467 (Andreas Farre 2021-05-26 2235) }
dd51467 (Andreas Farre 2021-05-26 2236) 
If you can get past the terrible formatting you should be able to see there are two commits of interest here. The first is dd51467c228cb from Andreas Farre and the second which was layered on top is 2b70b9d821c8e from Kashav Madan. Let's find out more about them both.
$ git log -1 dd51467c228cb
commit dd51467c228cb5c9ec9d9efbb6e0339037ec7fd5
Author: Andreas Farre <farre@mozilla.com>
Date:   Wed May 26 07:14:06 2021 +0000

    Part 7: Bug 1700623 - Make session storage session store work with Fission.
        r=nika
    
    Use the newly added session storage data getter to access the session
    storage in the parent and store it in session store without a round
    trip to content processes.
    
    Depends on D111433
    
    Differential Revision: https://phabricator.services.mozilla.com/D111434
For some reason the Phabricator link doesn't work for me, but we can still see the revision directly in the repository. 
$ git log -1 2b70b9d821c8e commit 2b70b9d821c8eaf0ecae987cfc57e354f0f9cc20 Author: Kashav Madan <kshvmdn@gmail.com> Date: Sat Jun 26 20:25:29 2021 +0000 Bug 1703692 - Store the latest embedder's permanent key on CanonicalBrowsingContext, r=nika,mccr8 And include it in Session Store flushes to avoid dropping updates in case the browser is unavailable. Differential Revision: https://phabricator.services.mozilla.com/D118385 There aren't many clues in these changes, in particular there's no hint of how the build system was changed to have these files included. However, digging around in this code has given me a better understanding of the structure and purpose of the different directories.

It seems to me that, in essence, the gecko-dev/browser/components/ directory where the missing files can be found contains modules that relate to the browser chrome and Firefox user interface, rather than the rendering engine. Typically this kind of content would be replicated on Sailfish OS by adding amended versions into the embedlite-components project. If it's browser-specific material, that would make sense.

As an example, in Firefox we can find AboutRedirector.h and AboutRedirector.cpp files , whereas on Sailfish OS there's a replacement AboutRedirectory.js file in embedlite-components. Similarly Firefox has a DownloadsManager.jsm file that can be found in gecko-dev/browser/components/newtab/lib/. This seems to be replaced by EmbedliteDownloadManager.js in embedlite-components. Both have similar functionality based on the names of the methods contained in them, but the implementations are quite different.

Assuming this is correct, probably the right way to tackle the missing SessionStore.jsm and related files would be to move copies into embedlite-components. They'll need potentially quite big changes to align them with sailfish-browser, although hopefully this will largely be removing functionality that has already been implemented elsewhere (for example cookie save and restore).

I think I'll give these changes a go tomorrow.

Another thing I've pretty-much concluded while looking through this code is that it looks like it probably has nothing to do with the issue that's blocking the Back and Forward buttons from working after all. So I'll also need to make a separate task to track down the real source of that problem.

Right now I have a stack of tasks: SessionStore; Back/Forward failures; DuckDuckGo rendering. I mustn't lose site of the fact that the real goal right now is to get DuckDuckGo rendering correctly. The other tasks are secondary, albeit with DuckDuckGo rendering potentially dependent on them.

That's it for today. More tomorrow and Thursday, but then a bit of a break.

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

Comments

Uncover Disqus comments