List items
Items from the current list are shown below.
Gecko
8 Feb 2024 : Day 150 #
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:
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:
In ESR 78 we have a slightly different version of this method:
Having said that, there is also a reference to InitSessionHistory() in MaybeCreateDocShell() so we ought to check that too:
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.
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:
When I get back I'll be back to posting these dev diaries again. And as a note to myself, when I do, my first action must be to figure out why nsFrameLoader is being created in ESR 78 but not ESR 91. That might hold the key to why the sessionHistory isn't getting called in ESR 91. And as a last resort, I can always revert commit 140a4164598e0c9ed53.
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) rAnd 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.
Comments
Uncover Disqus comments