flypig.co.uk

FOSDEM'24 is over and it's back to semi-normal again now. In particular, as promised, I'm going to be back to writing daily dev diaries and hopefully getting the Sailfish Browser into better shape running ESR 91. My first action is going to be to continue in my attempt to get the back and forwards buttons working by fixing the sessionHistory. Then I'll move back up my task stack to return to the Set-Fetch-* headers and DuckDuckGo.

But before jumping back into coding let me also say how great it was to meet so many interesting people — both old friends and new &mdsah; at FOSDEM. The Linux on Mobile stand was buzzing the entire time with a crowd of curious FOSS enthusiasts. I was also really happy to have the chance to talk about these dev diaries in the FOSS on Mobile Devices devroom. In case you missed it there's a video available, as well as the slides and the LaTeX source for the slides.

I'm not going to go over my talk again here, but I will take the opportunity to share two of the images from the slides.

First, to reiterate my thanks for everyone who has been following this dev diary, helping with coding, sharing ideas, performing testing, writing their own code changes (to either this or other packages that gecko relies on), producing images, boosting or liking on Mastodon, commenting on things. I really appreciate it all. I've tried to capture everyone, but I apologise if I manage to miss anyone off.
 
The other graphic I wanted to share summarises my progress to date. This is of course a significant simplification: it's been a lot more messy than this in practice. But it serves as some kind of overview.
 
As you can see, this takes us right up to today and the session history.

Before this latest two week break I wrote myself some notes to help me get back up to speed when I returned. Here's what my notes say:
 
I'm glad I wrote this, because otherwise I'd have completely forgotten the details of what I was working on by now. So let's continue where we left off.

First off I'll try debugging ESR 78 to get a backtrace for the call to the nsFrameLoader constructor.

Unfortunately, although when I place a breakpoint on nsFrameLoader::Create() it does get hit when using ESR 78, it's not possible to extract a backtrace. The debugger just complains and tries to output a core dump.

Happily by placing breakpoints on promising looking methods that call it or its parents, I'm able to step back through the calls and place a breakpoint on nsGenericHTMLFrameElement::GetContentWindow() which, if hit, is guaranteed to then call nsFrameLoader::Create(). And it does git hit. And from this method it's possible to extract a backtrace:

Thread 10 "GeckoWorkerThre" hit Breakpoint 4, nsGenericHTMLFrameElement::
    GetContentWindowInternal (this=this@entry=0x7fba4f9560)
    at dom/html/nsGenericHTMLFrameElement.cpp:100
100       EnsureFrameLoader();
(gdb) bt
#0  nsGenericHTMLFrameElement::GetContentWindowInternal
    (this=this@entry=0x7fba4f9560)
    at dom/html/nsGenericHTMLFrameElement.cpp:100
#1  0x0000007ff39194bc in nsGenericHTMLFrameElement::GetContentWindow
    (this=this@entry=0x7fba4f9560)
    at dom/html/nsGenericHTMLFrameElement.cpp:116
#2  0x0000007ff3518bf0 in mozilla::dom::HTMLIFrameElement_Binding::
    get_contentWindow (cx=0x7fb82263f0, obj=..., void_self=0x7fba4f9560,
    args=...)
    at HTMLIFrameElementBinding.cpp:857
#3  0x0000007ff35b8290 in mozilla::dom::binding_detail::GenericGetter
    <mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::
    binding_detail::ThrowExceptions> (cx=0x7fb82263f0, argc=<optimized out>,
    vp=<optimized out>)
    at dist/include/js/CallArgs.h:245
#4  0x0000007ff4e19610 in CallJSNative (args=..., reason=js::CallReason::Getter, 
    native=0x7ff35b80c0 <mozilla::dom::binding_detail::GenericGetter
    <mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::
    binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*)>,
    cx=0x7fb82263f0)
    at dist/include/js/CallArgs.h:285
#5  js::InternalCallOrConstruct (cx=cx@entry=0x7fb82263f0, args=...,
    construct=construct@entry=js::NO_CONSTRUCT, 
    reason=reason@entry=js::CallReason::Getter) at js/src/vm/Interpreter.cpp:585
#6  0x0000007ff4e1a268 in InternalCall (reason=js::CallReason::Getter, args=...,
    cx=0x7fb82263f0)
    at js/src/vm/Interpreter.cpp:648
#7  js::Call (reason=js::CallReason::Getter, rval=..., args=..., thisv=...,
    fval=..., cx=0x7fb82263f0)
    at js/src/vm/Interpreter.cpp:665
#8  js::CallGetter (cx=cx@entry=0x7fb82263f0, thisv=..., getter=...,
    getter@entry=..., rval=...)
    at js/src/vm/Interpreter.cpp:789
[...]
#44 0x0000007fefa864ac in QThread::exec() () from /usr/lib64/libQt5Core.so.5
#45 0x0000007fefa8b0e8 in ?? () from /usr/lib64/libQt5Core.so.5
#46 0x0000007fef971a4c in ?? () from /lib64/libpthread.so.0
#47 0x0000007fef65b89c in ?? () from /lib64/libc.so.6
(gdb) 

By reading through all of the functions in this backtrace, starting at the top and moving downwards, I need to find out where the ESR 91 code diverges from ESR 78 in a way that means this method doesn't get called with ESR 91.

This involves reading through each of the methods as they are in ESR 91 to see if they're different at any point. If they are I can run ESR 91 to establish whether that difference is the actual cause of the different execution paths between the two.

But before I do that I want to review things again.

On both ESR 78 and ESR 91 the nsFrameLoader::Create() method is called. However on ESR 91 changeset D100348 means that this no longer calls InitSessionHistory().

Instead, and because of this change, on ESR 91 it's the TryRemoteBrowserInternal() method needs to be called in order for the session history to be initialised. on ESR 91 the route to that method appears to be via nsFrameLoader::GetBrowsingContext().

On ESR 78 the GetBrowsingContext() method gets called like this:

Thread 8 "GeckoWorkerThre" hit Breakpoint 3, nsFrameLoader::GetBrowsingContext
    (this=0x7f89886f80)
    at dom/base/nsFrameLoader.cpp:3194
3194      if (IsRemoteFrame()) {
(gdb) bt
#0  nsFrameLoader::GetBrowsingContext (this=0x7f89886f80)
    at dom/base/nsFrameLoader.cpp:3194
#1  0x0000007fbb5ad250 in mozilla::dom::HTMLIFrameElement::
    MaybeStoreCrossOriginFeaturePolicy (this=0x7f895858f0)
    at dom/html/HTMLIFrameElement.cpp:252
#2  mozilla::dom::HTMLIFrameElement::MaybeStoreCrossOriginFeaturePolicy
    (this=0x7f895858f0)
    at dom/html/HTMLIFrameElement.cpp:241
#3  0x0000007fbb5ad390 in mozilla::dom::HTMLIFrameElement::RefreshFeaturePolicy
    (this=0x7f895858f0, aParseAllowAttribute=aParseAllowAttribute@entry=true)
    at dom/html/HTMLIFrameElement.cpp:329
#4  0x0000007fbb5ad4b8 in mozilla::dom::HTMLIFrameElement::BindToBrowsingContext
    (this=<optimized out>, aBrowsingContext=<optimized out>)
    at dom/html/HTMLIFrameElement.cpp:72
#5  0x0000007fbc7abf60 in mozilla::dom::BrowsingContext::Embed
    (this=<optimized out>)
    at docshell/base/BrowsingContext.cpp:505
#6  0x0000007fbaae320c in nsFrameLoader::MaybeCreateDocShell (this=0x7f89886f80)
    at dist/include/mozilla/RefPtr.h:313
#7  0x0000007fbaae5468 in nsFrameLoader::ReallyStartLoadingInternal
    (this=this@entry=0x7f89886f80)
    at dom/base/nsFrameLoader.cpp:612
#8  0x0000007fbaae5864 in nsFrameLoader::ReallyStartLoading (
dwarf2read.c:10473: internal-error: process_die_scope::process_die_scope
    (die_info*, dwarf2_cu*): Assertion `!m_die->in_process' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.

On ESR 91 the GetBrowsingContext() method never seems to get called at all. But I've put breakpoints on most of the other places in the backtrace to check whether they get hit on ESR 91:

Breakpoint 3 at 0x7ff3925d4c: file dom/html/HTMLIFrameElement.cpp, line 221.
(gdb) b HTMLIFrameElement::MaybeStoreCrossOriginFeaturePolicy
Note: breakpoint 3 also set at pc 0x7ff3925d4c.
Breakpoint 4 at 0x7ff3925d4c: file dom/html/HTMLIFrameElement.cpp, line 221.
(gdb) b HTMLIFrameElement::RefreshFeaturePolicy
Breakpoint 5 at 0x7ff3925fb0: file dom/html/HTMLIFrameElement.cpp, line 266.
(gdb) b HTMLIFrameElement::BindToBrowsingContext
Breakpoint 6 at 0x7ff3926134: file dom/html/HTMLIFrameElement.cpp, line 70.
(gdb) b BrowsingContext::Embed
Breakpoint 7 at 0x7ff4ab2b0c: file ${PROJECT}/obj-build-mer-qt-xr/dist/include/
    mozilla/RefPtr.h, line 313.
(gdb) b nsFrameLoader::MaybeCreateDocShell
Breakpoint 8 at 0x7ff2dedc98: file dom/base/nsFrameLoader.cpp, line 2179.
(gdb) b  nsFrameLoader::ReallyStartLoadingInternal
Breakpoint 9 at 0x7ff2def63c: file dom/base/nsFrameLoader.cpp, line 664.
(gdb) info break
Num Type       Disp Enb What
2   breakpoint keep y   in nsFrameLoader::GetBrowsingContext() 
                        at dom/base/nsFrameLoader.cpp:3489
3   breakpoint keep y   in mozilla::dom::HTMLIFrameElement::
                           MaybeStoreCrossOriginFeaturePolicy() 
                        at dom/html/HTMLIFrameElement.cpp:221
4   breakpoint keep y   in mozilla::dom::HTMLIFrameElement::
                           MaybeStoreCrossOriginFeaturePolicy() 
                        at dom/html/HTMLIFrameElement.cpp:221
5   breakpoint keep y   in mozilla::dom::HTMLIFrameElement::
                           RefreshFeaturePolicy(bool) 
                        at dom/html/HTMLIFrameElement.cpp:266
6   breakpoint keep y   in mozilla::dom::HTMLIFrameElement::
                           BindToBrowsingContext(mozilla::dom::BrowsingContext*) 
                        at dom/html/HTMLIFrameElement.cpp:70
7   breakpoint keep y   in mozilla::dom::BrowsingContext::Embed() 
                        at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/
                           RefPtr.h:313
8   breakpoint keep y   in nsFrameLoader::MaybeCreateDocShell() 
                        at dom/base/nsFrameLoader.cpp:2179
9   breakpoint keep y   in nsFrameLoader::ReallyStartLoadingInternal() 
                        at dom/base/nsFrameLoader.cpp:664
(gdb) r

And one of them does get hit:

Thread 10 "GeckoWorkerThre" hit Breakpoint 9, nsFrameLoader::
    ReallyStartLoadingInternal (this=this@entry=0x7fc160da30)
    at dom/base/nsFrameLoader.cpp:664
664     nsresult nsFrameLoader::ReallyStartLoadingInternal() {
(gdb) c
Continuing.

Let's see what happens next...

Thread 10 "GeckoWorkerThre" hit Breakpoint 8, nsFrameLoader::
    MaybeCreateDocShell(this=this@entry=0x7fc160da30)
    at dom/base/nsFrameLoader.cpp:2179
2179    nsresult nsFrameLoader::MaybeCreateDocShell() {
(gdb) c
Continuing.

Thread 10 "GeckoWorkerThre" hit Breakpoint 7, mozilla::dom::BrowsingContext::
    Embed (this=0x7fc161aa70)
    at docshell/base/BrowsingContext.cpp:711
711       if (auto* frame = HTMLIFrameElement::FromNode(mEmbedderElement)) {
(gdb) c
Continuing.

Thread 10 "GeckoWorkerThre" hit Breakpoint 6, mozilla::dom::HTMLIFrameElement::
    BindToBrowsingContext (this=0x7fc14db4c0)
    at dom/html/HTMLIFrameElement.cpp:70
70      void HTMLIFrameElement::BindToBrowsingContext(BrowsingContext*) {
(gdb) c
Continuing.

Thread 10 "GeckoWorkerThre" hit Breakpoint 5, mozilla::dom::HTMLIFrameElement::
    RefreshFeaturePolicy (this=0x7fc14db4c0, 
    aParseAllowAttribute=aParseAllowAttribute@entry=true) at
    dom/html/HTMLIFrameElement.cpp:266
266     void HTMLIFrameElement::RefreshFeaturePolicy(bool aParseAllowAttribute) {
(gdb) c
Continuing.

Thread 10 "GeckoWorkerThre" hit Breakpoint 3, mozilla::dom::HTMLIFrameElement::
    MaybeStoreCrossOriginFeaturePolicy (this=this@entry=0x7fc14db4c0)
    at dom/html/HTMLIFrameElement.cpp:221
221     void HTMLIFrameElement::MaybeStoreCrossOriginFeaturePolicy() {
(gdb) c
Continuing.

Thread 10 "GeckoWorkerThre" hit Breakpoint 2, nsFrameLoader::GetBrowsingContext
    (this=0x7fc160da30)
    at dom/base/nsFrameLoader.cpp:3489
3489      if (mNotifyingCrash) {
(gdb) 

As a quick reminder, this is what the inside of this method looks like:

BrowsingContext* nsFrameLoader::GetBrowsingContext() {
  if (mNotifyingCrash) {
    if (mPendingBrowsingContext && mPendingBrowsingContext->EverAttached()) {
      return mPendingBrowsingContext;
    }
    return nullptr;
  }
  if (IsRemoteFrame()) {
    Unused << EnsureRemoteBrowser();
  } else if (mOwnerContent) {
    Unused << MaybeCreateDocShell();
  }
  return GetExtantBrowsingContext();
}

Let's compare this to the relevant variable values.

(gdb) p mNotifyingCrash
$1 = false
(gdb) p mIsRemoteFrame
$2 = false
(gdb) p mOwnerContent
$3 = (mozilla::dom::Element *) 0x7fc14db4c0
(gdb) 

With these values IsRemoteFrame() will return false and the MaybeCreateDocShell() path will be entered, rather than the EnsureRemoteBrowser() path that we want.

In ESR 78 we have a slightly different version of this method:

BrowsingContext* nsFrameLoader::GetBrowsingContext() {
  if (IsRemoteFrame()) {
    Unused << EnsureRemoteBrowser();
  } else if (mOwnerContent) {
    Unused << MaybeCreateDocShell();
  }
  return GetExtantBrowsingContext();
}

But given the values of the variables, we'll get the same result:

(gdb) p mIsRemoteFrame
$1 = false
(gdb) p mOwnerContent
$2 = (
dwarf2read.c:10473: internal-error: process_die_scope::process_die_scope
    (die_info*, dwarf2_cu*): Assertion `!m_die->in_process' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) n
(gdb) p (mOwnerContent != 0)
$3 = true
(gdb) 

It's beginning to look like the problem here is that IsRemoteFrame() is always returning false, so that the code we want to get called never gets called.

Having said that, there is also a reference to InitSessionHistory() in MaybeCreateDocShell() so we ought to check that too:

Thread 10 "GeckoWorkerThre" hit Breakpoint 8, nsFrameLoader::MaybeCreateDocShell
    (this=this@entry=0x7fc160da30)
    at dom/base/nsFrameLoader.cpp:2179
2179    nsresult nsFrameLoader::MaybeCreateDocShell() {
(gdb) n
2180      if (GetDocShell()) {
(gdb) 
nsFrameLoader::GetBrowsingContext (this=0x7fc160da30) at dom/base/
    nsFrameLoader.cpp:3500
3500      return GetExtantBrowsingContext();
(gdb) 
mozilla::dom::HTMLIFrameElement::MaybeStoreCrossOriginFeaturePolicy
    (this=this@entry=0x7fc14db4c0)
    at dom/html/HTMLIFrameElement.cpp:232
232       RefPtr<BrowsingContext> browsingContext = mFrameLoader->
    GetBrowsingContext();
(gdb) 
234       if (!browsingContext || !browsingContext->IsContentSubframe()) {
(gdb) 
238       if (ContentChild* cc = ContentChild::GetSingleton()) {
(gdb) 
232       RefPtr<BrowsingContext> browsingContext = mFrameLoader->
    GetBrowsingContext();
(gdb) 
nsFrameLoader::MaybeCreateDocShell (this=this@entry=0x7fc160da30) at
    dom/base/nsFrameLoader.cpp:2237
2237      InvokeBrowsingContextReadyCallback();
(gdb) n
2239      mIsTopLevelContent = mPendingBrowsingContext->IsTopContent();
(gdb) 
2241      if (mIsTopLevelContent) {
(gdb) 
2252      nsCOMPtr<nsIDocShellTreeOwner> parentTreeOwner;
(gdb) 
1363    ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsCOMPtr.h:
    No such file or directory.
(gdb) 
859     in ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsCOMPtr.h
(gdb) 
2258      RefPtr<EventTarget> chromeEventHandler;
(gdb) 
2259      bool parentIsContent = parentDocShell->GetBrowsingContext()->
    IsContent();
(gdb) 
2260      if (parentIsContent) {
(gdb) 
2263        parentDocShell->GetChromeEventHandler(getter_AddRefs
    (chromeEventHandler));
(gdb) 
289     ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/RefPtr.h:
    No such file or directory.
(gdb) 
2278      nsCOMPtr<nsPIDOMWindowOuter> newWindow = docShell->GetWindow();
(gdb) n
2285      newWindow->SetFrameElementInternal(mOwnerContent);
(gdb) 
2288      if (mOwnerContent->IsXULElement(nsGkAtoms::browser) &&
(gdb) 
2295      if (!docShell->Initialize()) {
(gdb) 
2301      NS_ENSURE_STATE(mOwnerContent);
(gdb) 

We've now reached this condition, which we really want the programme counter to enter:

  // If we are an in-process browser, we want to set up our session history.
  if (mIsTopLevelContent && mOwnerContent->IsXULElement(nsGkAtoms::browser) &&
      !mOwnerContent->HasAttr(kNameSpaceID_None, nsGkAtoms::disablehistory)) {
    // XXX(nika): Set this up more explicitly?
    mPendingBrowsingContext->InitSessionHistory();
  }

But it isn't to be:

(gdb) p mIsTopLevelContent 
$4 = false
(gdb) n
2304      if (mIsTopLevelContent && mOwnerContent->IsXULElement(nsGkAtoms::browser) &&
(gdb) 
2315      HTMLIFrameElement* iframe = HTMLIFrameElement::FromNode(mOwnerContent);
(gdb) 

The value of the mIsTopLevelContent comes from earlier in the same method:

  mIsTopLevelContent = mPendingBrowsingContext->IsTopContent();

Checking in BrowsingContext.h we see this:

  bool IsTopContent() const { return IsContent() && IsTop(); }
[...]
  bool IsContent() const { return mType == Type::Content; }
[...]
  bool IsTop() const { return !GetParent(); }

And to round this off, the GetParent() method is defined in BrowsingContext.cpp like this:

BrowsingContext* BrowsingContext::GetParent() const {
  return mParentWindow ? mParentWindow->GetBrowsingContext() : nullptr;
}

This call to IsTop() matches the call that the call to InitSessionHistory() is conditioned on elsewhere as well. Let's check the relevant values:

(gdb) p mPendingBrowsingContext.mRawPtr->mType
$5 = mozilla::dom::BrowsingContext::Type::Content
(gdb) p mPendingBrowsingContext.mRawPtr->mParentWindow
$6 = {mRawPtr = 0x7fc11bb430}
(gdb) 

This means that GetParent() is returning a non-null value, hence IsTo() must be returning false.

Which is why the session isn't being initialised here.

That's enough digging for today. More tomorrow.

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