List items
Items from the current list are shown below.
Blog
16 Jan 2024 : Day 140 #
I'm up early today in the hope of finding evidence of why DuckDuckGo is serving ESR 91 a completely different — and quite broken — version of the search site. Yesterday we established, pretty convincingly I think, that the page being served is different and that it's not the ESR 91 renderer that's the problem. If I view the exact same pages on ESR 78 or a desktop browser I get the same blank page.
Today I plan to look into the request and response headers, and especially the user agent in more detail. Maybe there's something in the request headers which differs between ESR 78 and ESR 91 that I've not noticed.
But I have to warn you: it's a long one today. I recommend skipping the response headers and backtraces if you're in a rush.
And before getting in to the headers I also want to thank Simon (simonschmeisser) and Adrian (amcewen) for their helpful interjections on the Sailfish Forum and Mastodon respectively.
Both of you correctly noticed that one of the files was broken on the copy I made of DuckDuckGo built from the data downloaded using the ESR 91 library. This shows up quite clearly a desktop browser using the dev console (as in Adrian's picture below) and if you try to download the file directly.
The broken URL is the following:
Interestingly it still doesn't render with ESR 91. Which makes me wonder if the issue is related to the locale. But I'm going to have to come back to this because I've committed myself to looking at request headers today. Nevertheless, a big thank you to both Simon and Adrian. Not only is it reassuring to know my work is being checked, but this could also provide a critical piece of the puzzle.
We have a lot to get through today though, so let's now move on to request headers. We're particularly interested in the index.html file because, as anyone who's worked with webservers before will know, this is the first file to get downloaded and the one that kicks off all of the other downloads. Only files referenced in index.html, or where there's a reference chain back to index.html are going to get downloaded.
Here are the request headers for the index page sent to DuckDuckGo when using ESR 78.
The emphasis here is theirs. In our case we're sending document as the value, which from the docs means:
This feels pretty accurate. How about Sec-Fetch-Mode and its value of navigate?
Again, this all looks pretty unexceptional. Finally the Sec-Fetch-Site header and its value cross-site? According to the docs...
This last one looks suspicious to me. ESR 91 is sending a cross-site value, which is the most risky of all the options, because it's essentially telling DuckDuckGo that the request is happening across different domains. From the docs, it looks like none would be a more appropriate value for this:
Checking the code I notice there's also a fourth Sec-Fetch header in the set which is the Sec-Fetch-User header. This isn't being sent and this might also be important, because the request we're making is user-initiated, so it might be reasonable to expect this header to be included. The docs say this about the header:
In effect, by omitting the header the browser is telling the site that the request isn't initiated by the user, but rather by something else, such as being a document referenced inside some other document.
An obvious thing to test would be to remove these headers and see whether that makes any difference. But before getting on to that let's take a look at the response headers as well.
First the response headers when using ESR 78:
Next the response headers when using ESR 91 and the standard user agent:
If there's nothing particularly interesting to see in the response headers it means we can go back to experimenting with the three Sec-Fetch headers discussed earlier.
Digging through the code and the SecFetch.cpp file in particular, I can see that the headers are added in this method:
This time the headers no longer have any of the Sec-Fetch headers included:
But then I try with the ESR 78 user agent and Sec-Fetch headers disabled... and it works! The site shows correctly, just as it does in ESR 78.
It's a little hard to express the strange mix of jubilation and frustration that I'm feeling right now. Jubilation because we've finally reached the point where it's certain that it will be possible to fix this. Frustration because it's taken quite so long to reach this point.
This feeling pretty much sums up my experience of software development in general. Despite the frustration, this is what I really love about it!
Before claiming that this is fixed, it's worth focusing a little on what the real problem is here. There is a user-agent issue for sure. And arguably DuckDuckGo is trying to frustrate certain users (bots, essentially) by serving different pages to different clients. But the real issue here is that these Sec-Fetch headers are actually broken in our version of gecko ESR 91. That's not the fault of the upstream code: it's a failure of the way the front-end is interacting with the backend code.
So the correct way to fix this issue (at least from the client-side) is to fix those headers. Fixing it for DuckDuckGo is likely to have a positive effect on other sites as well, so fixing it will be worthwhile effort.
That's what I'll now move on to.
As noted above the current (incorrect) values set for the headers are the following:
I'm going to use the debugger to try to find out how these values get set.
That'll have to change now. But it looks like this will be just one of many such flags that will need adding in to the Sailfish code.
Let's focus on LOAD_FLAGS_FROM_EXTERNAL first. This is set in browser.js when a call is made to getContentWindowOrOpenURI(). This ends up running this piece of code:
What's the equivalent for Sailfish Browser? There are three ways it may end up opening an external URL that I can think of. The first is if the application is executed with a URL on the command line:
Let's follow the path in the case of the D-Bus call. The entry point for this is in browserservice.cpp where the D-Bus object is registered. The call looks like this:
Trying to follow all these routes is like trying to follow a droplet of water down a waterfall. I think I've reached the limit of my indirection capacity here; I'm going to need to pick this up again tomorrow.
But, so that I don't lose my thread, a note about two further things I need to do with this.
First I need to follow the process through until I hit the point at which the EmbedLiteViewParent::SendLoadURL() is called. This is the point at which the flag needs to be set. It looks like the common way for this to get called is through the following call:
Second I need to ensure the flag gets passed from the point at which we know what its value needs to be (which is the D-Bus interface) to this call to SendLoadURL(), so that EmbedLiteViewChild::RecvLoadURL() can set it appropriately.
Once I have those two pieces everything will tie together and at that point it will be possible to set the LOAD_FLAGS_FROM_EXTERNAL flag appropriately.
That's it for today. Tomorrow, onwards.
If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.
Today I plan to look into the request and response headers, and especially the user agent in more detail. Maybe there's something in the request headers which differs between ESR 78 and ESR 91 that I've not noticed.
But I have to warn you: it's a long one today. I recommend skipping the response headers and backtraces if you're in a rush.
And before getting in to the headers I also want to thank Simon (simonschmeisser) and Adrian (amcewen) for their helpful interjections on the Sailfish Forum and Mastodon respectively.
Both of you correctly noticed that one of the files was broken on the copy I made of DuckDuckGo built from the data downloaded using the ESR 91 library. This shows up quite clearly a desktop browser using the dev console (as in Adrian's picture below) and if you try to download the file directly.
The broken URL is the following:
https://duckduckgo.com/_next/static/chunks/pages/%5Blocale%5D/home-74b6f93f10631d81.jsIt's broken because of the %5B and %5D in the path. These are characters encoded using "URL" or "percent" encoding as originally codified in RFC 3986. The actually represent the left and right square brackets respectively. What's happened is that the Python script I used back on Day 135 has quite correctly output the URL in this encoded format. When I uploaded it to S3 I should have decoded it so that the file was stored in a folder called [locale] like this:
https://duckduckgo.com/_next/static/chunks/pages/[locale]/home-74b6f93f10631d81.jsInstead I left the encoded form in. Oh dear! The file itself is tiny, containing just the following text:
(self.webpackChunk_N_E=self.webpackChunk_N_E||[]).push([[43803],{98387:function (n,_,u){(window.__NEXT_P=window.__NEXT_P||[]).push(["/[locale]/home",function() {return u(39265)}])}},function(n){n.O(0,[41966,93432,18040,81125,39337,94623, 95665,55015,61754,55672,38407,49774,92888,40179],(function(){return _=98387, n(n.s=_);var _}));var _=n.O();_N_E=_}]);On hearing about my mistake from Simon and Adrian I thought this error would be too small to make any real difference. How wrong I was! Now that I've fixed this the page actually renders now, both in the Sailfish Browser on ESR 78 and on desktop Firefox.
Interestingly it still doesn't render with ESR 91. Which makes me wonder if the issue is related to the locale. But I'm going to have to come back to this because I've committed myself to looking at request headers today. Nevertheless, a big thank you to both Simon and Adrian. Not only is it reassuring to know my work is being checked, but this could also provide a critical piece of the puzzle.
We have a lot to get through today though, so let's now move on to request headers. We're particularly interested in the index.html file because, as anyone who's worked with webservers before will know, this is the first file to get downloaded and the one that kicks off all of the other downloads. Only files referenced in index.html, or where there's a reference chain back to index.html are going to get downloaded.
Here are the request headers for the index page sent to DuckDuckGo when using ESR 78.
[ Request details ------------------------------------------- ] Request: GET status: 200 OK URL: https://duckduckgo.com/ [ Request headers --------------------------------------- ] Host : duckduckgo.com User-Agent : Mozilla/5.0 (Mobile; rv:78.0) Gecko/78.0 Firefox/78.0 Accept : text/html,application/xhtml+xml,application/xml; q=0.9,image/webp,*/*;q=0.8 Accept-Language : en-GB,en;q=0.5 Accept-Encoding : gzip, deflate, br Connection : keep-alive Upgrade-Insecure-Requests : 1And here are the equivalent headers when accessing the same page of DuckDuckGo when using ESR 91 using the default user agent.
[ Request details ------------------------------------------- ] Request: GET status: 200 OK URL: https://duckduckgo.com/ [ Request headers --------------------------------------- ] Host : duckduckgo.com User-Agent : Mozilla/5.0 (X11; Linux aarch64; rv:91.0) Gecko/20100101 Firefox/91.0 Accept : text/html,application/xhtml+xml,application/xml;q=0.9, image/webp,*/*;q=0.8 Accept-Language : en-GB,en;q=0.5 Accept-Encoding : gzip, deflate, br Connection : keep-alive Upgrade-Insecure-Requests : 1 Sec-Fetch-Dest : document Sec-Fetch-Mode : navigate Sec-Fetch-Site : cross-siteFinally, when setting the user agent to match that of the iPhone the request headers look like this:
[ Request details ------------------------------------------- ] Request: GET status: 200 OK URL: https://duckduckgo.com/ [ Request headers --------------------------------------- ] Host : duckduckgo.com User-Agent : Mozilla/5.0 (iPhone12,1; U; CPU iPhone OS 13_0 like Mac OS X) AppleWebKit/602.1.50 (KHTML, like Gecko) Version/10.0 Mobile/15E148 Safari/602.1 Accept : text/html,application/xhtml+xml,application/xml;q=0.9, image/webp,*/*;q=0.8 Accept-Language : en-GB,en;q=0.5 Accept-Encoding : gzip, deflate, br Connection : keep-alive Upgrade-Insecure-Requests : 1 Sec-Fetch-Dest : document Sec-Fetch-Mode : navigate Sec-Fetch-Site : cross-siteWhat do we notice from these? Obviously the user agents are different. The standard ESR 91 user agent isn't identifying itself as a Mobile variant and that's something that needs to be fixed for all sites. The remaining fields in the ESR 78 list are identical to those in ESR 91. However ESR 91 does have some additional fields:
Sec-Fetch-Dest : document Sec-Fetch-Mode : navigate Sec-Fetch-Site : cross-siteLet's find out what these do. According to the Mozilla docs, Sec-Fetch-Dest...
...allows servers determine whether to service a request based on whether it is appropriate for how it is expected to be used. For example, a request with an audio destination should request audio data, not some other type of resource (for example, a document that includes sensitive user information).
The emphasis here is theirs. In our case we're sending document as the value, which from the docs means:
The destination is a document (HTML or XML), and the request is the result of a user-initiated top-level navigation (e.g. resulting from a user clicking a link).
This feels pretty accurate. How about Sec-Fetch-Mode and its value of navigate?
Broadly speaking, this allows a server to distinguish between: requests originating from a user navigating between HTML pages, and requests to load images and other resources. For example, this header would contain navigate for top level navigation requests, while no-cors is used for loading an image. For example, this header would contain navigate for top level navigation requests, while no-cors is used for loading an image.
navigate: The request is initiated by navigation between HTML documents.
navigate: The request is initiated by navigation between HTML documents.
Again, this all looks pretty unexceptional. Finally the Sec-Fetch-Site header and its value cross-site? According to the docs...
...this header tells a server whether a request for a resource is coming from the same origin, the same site, a different site, or is a "user initiated" request. The server can then use this information to decide if the request should be allowed.
cross-site: The request initiator and the server hosting the resource have a different site (i.e. a request by "potentially-evil.com" for a resource at "example.com").
cross-site: The request initiator and the server hosting the resource have a different site (i.e. a request by "potentially-evil.com" for a resource at "example.com").
This last one looks suspicious to me. ESR 91 is sending a cross-site value, which is the most risky of all the options, because it's essentially telling DuckDuckGo that the request is happening across different domains. From the docs, it looks like none would be a more appropriate value for this:
none: This request is a user-originated operation. For example: entering a URL into the address bar, opening a bookmark, or dragging-and-dropping a file into the browser window.
Checking the code I notice there's also a fourth Sec-Fetch header in the set which is the Sec-Fetch-User header. This isn't being sent and this might also be important, because the request we're making is user-initiated, so it might be reasonable to expect this header to be included. The docs say this about the header:
The Sec-Fetch-User fetch metadata request header is only sent for requests initiated by user activation, and its value will always be ?1.
A server can use this header to identify whether a navigation request from a document, iframe, etc., was originated by the user.
A server can use this header to identify whether a navigation request from a document, iframe, etc., was originated by the user.
In effect, by omitting the header the browser is telling the site that the request isn't initiated by the user, but rather by something else, such as being a document referenced inside some other document.
An obvious thing to test would be to remove these headers and see whether that makes any difference. But before getting on to that let's take a look at the response headers as well.
First the response headers when using ESR 78:
[ Response headers -------------------------------------- ] server : nginx date : Sun, 07 Jan 2024 16:07:57 GMT content-type : text/html; charset=UTF-8 content-length : 2357 vary : Accept-Encoding etag : "65983d82-935" content-encoding : br strict-transport-security : max-age=31536000 permissions-policy : interest-cohort=() content-security-policy : [...] ; x-frame-options : SAMEORIGIN x-xss-protection : 1;mode=block x-content-type-options : nosniff referrer-policy : origin expect-ct : max-age=0 expires : Sun, 07 Jan 2024 16:07:56 GMT cache-control : no-cache X-Firefox-Spdy : h2I've removed the content-security-policy value for brevity in all of these responses. They all happen to be the same.
Next the response headers when using ESR 91 and the standard user agent:
[ Response headers -------------------------------------- ] server : nginx date : Sun, 07 Jan 2024 16:09:35 GMT content-type : text/html; charset=UTF-8 content-length : 18206 vary : Accept-Encoding etag : "65983d80-471e" content-encoding : br strict-transport-security : max-age=31536000 permissions-policy : interest-cohort=() content-security-policy : [...] ; x-frame-options : SAMEORIGIN x-xss-protection : 1;mode=block x-content-type-options : nosniff referrer-policy : origin expect-ct : max-age=0 expires : Sun, 07 Jan 2024 16:09:34 GMT cache-control : no-cache X-Firefox-Spdy : h2And finally the response headers when using ESR 91 and the iPhone user agent:
[ Response headers -------------------------------------- ] server : nginx date : Sat, 13 Jan 2024 22:20:49 GMT content-type : text/html; charset=UTF-8 content-length : 18221 vary : Accept-Encoding etag : "65a1788c-472d" content-encoding : br strict-transport-security : max-age=31536000 permissions-policy : interest-cohort=() content-security-policy : [...] ; x-frame-options : SAMEORIGIN x-xss-protection : 1;mode=block x-content-type-options : nosniff referrer-policy : origin expect-ct : max-age=0 expires : Sat, 13 Jan 2024 22:20:48 GMT cache-control : no-cache X-Firefox-Spdy : h2All three share the same response header keys and values apart from the following three which have different values across all three responses:
date : Sun, 07 Jan 2024 16:07:57 GMT content-length : 2357 etag : "65983d82-935" expires : Sun, 07 Jan 2024 16:07:56 GMTIt's not surprising these are different across the three. In fact, if they weren't, that might suggest something more sinister. The only one that's really problematic is the content-length value, not because it's incorrect, but because the three different values highlights the fact we're being served three different pages depending on the request.
If there's nothing particularly interesting to see in the response headers it means we can go back to experimenting with the three Sec-Fetch headers discussed earlier.
Digging through the code and the SecFetch.cpp file in particular, I can see that the headers are added in this method:
void mozilla::dom::SecFetch::AddSecFetchHeader(nsIHttpChannel* aHTTPChannel) { // if sec-fetch-* is prefed off, then there is nothing to do if (!StaticPrefs::dom_security_secFetch_enabled()) { return; } nsCOMPtr<nsIURI> uri; nsresult rv = aHTTPChannel->GetURI(getter_AddRefs(uri)); if (NS_WARN_IF(NS_FAILED(rv))) { return; } // if we are not dealing with a potentially trustworthy URL, then // there is nothing to do here if (!nsMixedContentBlocker::IsPotentiallyTrustworthyOrigin(uri)) { return; } AddSecFetchDest(aHTTPChannel); AddSecFetchMode(aHTTPChannel); AddSecFetchSite(aHTTPChannel); AddSecFetchUser(aHTTPChannel); }The bit I'm interested in is the first condition which bails out of the method if a specific preference is disabled. Happily this preference was easy to establish as being exposed through about:config as dom.security.secFetch.enabled. I've now disabled it and can try loading the site again.
This time the headers no longer have any of the Sec-Fetch headers included:
[ Request details ------------------------------------------- ] Request: GET status: 200 OK URL: https://duckduckgo.com/ [ Request headers --------------------------------------- ] Host : duckduckgo.com User-Agent : Mozilla/5.0 (X11; Linux aarch64; rv:91.0) Gecko/20100101 Firefox/91.0 Accept : text/html,application/xhtml+xml,application/xml;q=0.9, image/webp,*/*;q=0.8 Accept-Language : en-GB,en;q=0.5 Accept-Encoding : gzip, deflate, br Connection : keep-alive Upgrade-Insecure-Requests : 1Sadly this doesn't fix the issue, I still get the same blank screen. I try again with the iPhone user agent, same result.
But then I try with the ESR 78 user agent and Sec-Fetch headers disabled... and it works! The site shows correctly, just as it does in ESR 78.
It's a little hard to express the strange mix of jubilation and frustration that I'm feeling right now. Jubilation because we've finally reached the point where it's certain that it will be possible to fix this. Frustration because it's taken quite so long to reach this point.
This feeling pretty much sums up my experience of software development in general. Despite the frustration, this is what I really love about it!
Before claiming that this is fixed, it's worth focusing a little on what the real problem is here. There is a user-agent issue for sure. And arguably DuckDuckGo is trying to frustrate certain users (bots, essentially) by serving different pages to different clients. But the real issue here is that these Sec-Fetch headers are actually broken in our version of gecko ESR 91. That's not the fault of the upstream code: it's a failure of the way the front-end is interacting with the backend code.
So the correct way to fix this issue (at least from the client-side) is to fix those headers. Fixing it for DuckDuckGo is likely to have a positive effect on other sites as well, so fixing it will be worthwhile effort.
That's what I'll now move on to.
As noted above the current (incorrect) values set for the headers are the following:
Sec-Fetch-Dest : document Sec-Fetch-Mode : navigate Sec-Fetch-Site : cross-siteWhat we'd expect and want to see for a user-triggered access to DuckDuckGo is this:
Sec-Fetch-Dest : document Sec-Fetch-Mode : navigate Sec-Fetch-Site : none Sec-Fetch-User : ?1Let's check the code for the two incorrect values. First the site value:
void mozilla::dom::SecFetch::AddSecFetchSite(nsIHttpChannel* aHTTPChannel) { nsAutoCString site("same-origin"); bool isSameOrigin = IsSameOrigin(aHTTPChannel); if (!isSameOrigin) { bool isSameSite = IsSameSite(aHTTPChannel); if (isSameSite) { site = "same-site"_ns; } else { site = "cross-site"_ns; } } if (IsUserTriggeredForSecFetchSite(aHTTPChannel)) { site = "none"_ns; } nsresult rv = aHTTPChannel->SetRequestHeader("Sec-Fetch-Site"_ns, site, false); mozilla::Unused << NS_WARN_IF(NS_FAILED(rv)); }We can infer that isSameOrigin and isSameSite are both set to false. This is a bit strange but it's actually not the bit we have to worry about. The result of going through the initial condition will be overwritten if IsUserTriggeredForSecFetchSite() returns true so that's where we should focus.
I'm going to use the debugger to try to find out how these values get set.
$ rm -rf ~/.local/share/org.sailfishos/browser/.mozilla/cache2/ \ ~/.local/share/org.sailfishos/browser/.mozilla/startupCache/ ~/.local/share/org.sailfishos/browser/.mozilla/cookies.sqlite $ gdb sailfish-browser [...] (gdb) r [...] (gdb) b LoadInfo::SetHasValidUserGestureActivation Breakpoint 4 at 0x7fb9d34424: file netwerk/base/LoadInfo.cpp, line 1609. (gdb) b LoadInfo::SetLoadTriggeredFromExternal Breakpoint 5 at 0x7fb9d34300: file netwerk/base/LoadInfo.cpp, line 1472. (gdb) c Thread 8 "GeckoWorkerThre" hit Breakpoint 4, mozilla::net::LoadInfo:: SetHasValidUserGestureActivation (this=this@entry=0x7f89b00da0, aHasValidUserGestureActivation=false) at netwerk/base/LoadInfo.cpp:1609 1609 mHasValidUserGestureActivation = aHasValidUserGestureActivation; (gdb) bt #0 mozilla::net::LoadInfo::SetHasValidUserGestureActivation (this=this@entry=0x7f89b00da0, aHasValidUserGestureActivation=false) at netwerk/base/LoadInfo.cpp:1609 #1 0x0000007fb9ffd86c in mozilla::net::CreateDocumentLoadInfo (aBrowsingContext=aBrowsingContext@entry=0x7f88c6dde0, aLoadState=aLoadState@entry=0x7f894082d0) at netwerk/ipc/DocumentLoadListener.cpp:149 #2 0x0000007fb9ffd9b8 in mozilla::net::DocumentLoadListener::OpenDocument (this=0x7f89a8c3e0, aLoadState=0x7f894082d0, aCacheKey=0, aChannelId=..., aAsyncOpenTime=..., aTiming=0x0, aInfo=..., aUriModified=..., aIsXFOError=..., aPid=aPid@entry=0, aRv=aRv@entry=0x7f9f3eda34) at netwerk/ipc/DocumentLoadListener.cpp:744 #3 0x0000007fb9ffe88c in mozilla::net::ParentProcessDocumentChannel::AsyncOpen (this=0x7f89bf02e0, aListener=0x7f895a7770) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/MaybeStorageBase.h:80 #4 0x0000007fba4663bc in nsURILoader::OpenURI (this=0x7f887b46c0, channel=0x7f89bf02e0, aFlags=0, aWindowContext=0x7f886044f0) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsCOMPtr.h:859 #5 0x0000007fbc7fa824 in nsDocShell::OpenInitializedChannel (this=this@entry=0x7f886044c0, aChannel=0x7f89bf02e0, aURILoader=0x7f887b46c0, aOpenFlags=0) at docshell/base/nsDocShell.cpp:10488 #6 0x0000007fbc7fb5e4 in nsDocShell::DoURILoad (this=this@entry=0x7f886044c0, aLoadState=aLoadState@entry=0x7f894082d0, aCacheKey=..., aRequest=aRequest@entry=0x7f9f3edf90) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsCOMPtr.h:859 #7 0x0000007fbc7fc1a4 in nsDocShell::InternalLoad (this=this@entry=0x7f886044c0, aLoadState=aLoadState@entry=0x7f894082d0, aCacheKey=...) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsCOMPtr.h:1363 #8 0x0000007fbc801c20 in nsDocShell::ReloadDocument (aDocShell=aDocShell@entry=0x7f886044c0, aDocument=<optimized out>, aLoadType=aLoadType@entry=2, aBrowsingContext=0x7f88c6dde0, aCurrentURI=0x7f887e62c0, aReferrerInfo=0x0, aNotifiedBeforeUnloadListeners=aNotifiedBeforeUnloadListeners@entry=false) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/MaybeStorageBase.h:79 #9 0x0000007fbc803984 in nsDocShell::Reload (this=0x7f886044c0, aReloadFlags=0) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsCOMPtr.h:859 #10 0x0000007fbc94d05c in nsWebBrowser::Reload (this=<optimized out>, aReloadFlags=<optimized out>) at toolkit/components/browser/nsWebBrowser.cpp:507 #11 0x0000007fbcb0ab70 in mozilla::embedlite::EmbedLiteViewChild::RecvReload (this=<optimized out>, aHardReload=<optimized out>) at mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:533 #12 0x0000007fba1915d0 in mozilla::embedlite::PEmbedLiteViewChild:: OnMessageReceived (this=0x7f88767020, msg__=...) at PEmbedLiteViewChild.cpp:1152 #13 0x0000007fba17f05c in mozilla::embedlite::PEmbedLiteAppChild:: OnMessageReceived (this=<optimized out>, msg__=...) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/ipc/ProtocolUtils.h:675 #14 0x0000007fba06b85c in mozilla::ipc::MessageChannel::DispatchAsyncMessage (this=this@entry=0x7f88b3e8a8, aProxy=aProxy@entry=0x7ebc003140, aMsg=...) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/ipc/ProtocolUtils.h:675 [...] #40 0x0000007fb78b289c in ?? () from /lib64/libc.so.6 (gdb) c Continuing. Thread 8 "GeckoWorkerThre" hit Breakpoint 5, mozilla::net::LoadInfo:: SetLoadTriggeredFromExternal (this=this@entry=0x7f89b00da0, aLoadTriggeredFromExternal=false) at netwerk/base/LoadInfo.cpp:1472 1472 mLoadTriggeredFromExternal = aLoadTriggeredFromExternal; (gdb) bt #0 mozilla::net::LoadInfo::SetLoadTriggeredFromExternal (this=this@entry=0x7f89b00da0, aLoadTriggeredFromExternal=false) at netwerk/base/LoadInfo.cpp:1472 #1 0x0000007fbc7f20f8 in nsDocShell::CreateAndConfigureRealChannelForLoadState (aBrowsingContext=aBrowsingContext@entry=0x7f88c6dde0, aLoadState=aLoadState@entry=0x7f894082d0, aLoadInfo=aLoadInfo@entry=0x7f89b00da0, aCallbacks=aCallbacks@entry=0x7f89724110, aDocShell=aDocShell@entry=0x0, aOriginAttributes=..., aLoadFlags=aLoadFlags@entry=2689028, aCacheKey=aCacheKey@entry=0, aRv=@0x7f9f3eda34: nsresult::NS_OK, aChannel=aChannel@entry=0x7f89a8c438) at docshell/base/nsDocShellLoadState.cpp:709 #2 0x0000007fb9ff9c4c in mozilla::net::DocumentLoadListener::Open (this=this@entry=0x7f89a8c3e0, aLoadState=aLoadState@entry=0x7f894082d0, aLoadInfo=aLoadInfo@entry=0x7f89b00da0, aLoadFlags=2689028, aCacheKey=aCacheKey@entry=0, aChannelId=..., aAsyncOpenTime=..., aTiming=aTiming@entry=0x0, aInfo=..., aUrgentStart=aUrgentStart@entry=false, aPid=aPid@entry=0, aRv=aRv@entry=0x7f9f3eda34) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsCOMPtr.h:359 #3 0x0000007fb9ffda34 in mozilla::net::DocumentLoadListener::OpenDocument (this=0x7f89a8c3e0, aLoadState=0x7f894082d0, aCacheKey=0, aChannelId=..., aAsyncOpenTime=..., aTiming=0x0, aInfo=..., aUriModified=..., aIsXFOError=..., aPid=aPid@entry=0, aRv=aRv@entry=0x7f9f3eda34) at netwerk/ipc/DocumentLoadListener.cpp:750 #4 0x0000007fb9ffe88c in mozilla::net::ParentProcessDocumentChannel::AsyncOpen (this=0x7f89bf02e0, aListener=0x7f895a7770) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/MaybeStorageBase.h:80 #5 0x0000007fba4663bc in nsURILoader::OpenURI (this=0x7f887b46c0, channel=0x7f89bf02e0, aFlags=0, aWindowContext=0x7f886044f0) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsCOMPtr.h:859 #6 0x0000007fbc7fa824 in nsDocShell::OpenInitializedChannel (this=this@entry=0x7f886044c0, aChannel=0x7f89bf02e0, aURILoader=0x7f887b46c0, aOpenFlags=0) at docshell/base/nsDocShell.cpp:10488 #7 0x0000007fbc7fb5e4 in nsDocShell::DoURILoad (this=this@entry=0x7f886044c0, aLoadState=aLoadState@entry=0x7f894082d0, aCacheKey=..., aRequest=aRequest@entry=0x7f9f3edf90) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsCOMPtr.h:859 #8 0x0000007fbc7fc1a4 in nsDocShell::InternalLoad (this=this@entry=0x7f886044c0, aLoadState=aLoadState@entry=0x7f894082d0, aCacheKey=...) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsCOMPtr.h:1363 #9 0x0000007fbc801c20 in nsDocShell::ReloadDocument (aDocShell=aDocShell@entry=0x7f886044c0, aDocument=<optimized out>, aLoadType=aLoadType@entry=2, aBrowsingContext=0x7f88c6dde0, aCurrentURI=0x7f887e62c0, aReferrerInfo=0x0, aNotifiedBeforeUnloadListeners=aNotifiedBeforeUnloadListeners@entry=false) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/MaybeStorageBase.h:79 #10 0x0000007fbc803984 in nsDocShell::Reload (this=0x7f886044c0, aReloadFlags=0) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsCOMPtr.h:859 #11 0x0000007fbc94d05c in nsWebBrowser::Reload (this=<optimized out>, aReloadFlags=<optimized out>) at toolkit/components/browser/nsWebBrowser.cpp:507 #12 0x0000007fbcb0ab70 in mozilla::embedlite::EmbedLiteViewChild::RecvReload (this=<optimized out>, aHardReload=<optimized out>) at mobile/sailfishos/embedshared/EmbedLiteViewChild.cpp:533 #13 0x0000007fba1915d0 in mozilla::embedlite::PEmbedLiteViewChild:: OnMessageReceived (this=0x7f88767020, msg__=...) at PEmbedLiteViewChild.cpp:1152 #14 0x0000007fba17f05c in mozilla::embedlite::PEmbedLiteAppChild:: OnMessageReceived (this=<optimized out>, msg__=...) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/ipc/ProtocolUtils.h:675 #15 0x0000007fba06b85c in mozilla::ipc::MessageChannel::DispatchAsyncMessage (this=this@entry=0x7f88b3e8a8, aProxy=aProxy@entry=0x7ebc003140, aMsg=...) at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/ipc/ProtocolUtils.h:675 [...] #41 0x0000007fb78b289c in ?? () from /lib64/libc.so.6 (gdb) c [...]Digging deeper to find out where the value for mHasValidUserGestureActivation is coming from. It's being set to false and we need to know why.
(gdb) disable break (gdb) break nsDocShell.cpp:4108 Breakpoint 6 at 0x7fbc801bec: file docshell/base/nsDocShell.cpp, line 4108. (gdb) c Continuing. [Switching to LWP 29933] Thread 8 "GeckoWorkerThre" hit Breakpoint 6, nsDocShell::ReloadDocument (aDocShell=aDocShell@entry=0x7f886044c0, aDocument=<optimized out>, aLoadType=aLoadType@entry=2, aBrowsingContext=0x7f88c6dde0, aCurrentURI=0x7f887e62c0, aReferrerInfo=0x0, aNotifiedBeforeUnloadListeners=aNotifiedBeforeUnloadListeners@entry=false) at docshell/base/nsDocShell.cpp:4108 4108 loadState->SetHasValidUserGestureActivation( (gdb) p context $14 = {mRawPtr = 0x7ed4009980} (gdb) p context.mRawPtr $15 = (mozilla::dom::WindowContext *) 0x7ed4009980 (gdb) p *(context.mRawPtr) $16 = {<nsISupports> = {_vptr.nsISupports = 0x7fbf75c7a0 <vtable for mozilla::dom::WindowGlobalParent+16>}, <nsWrapperCache> = { [...] mInnerWindowId = 16, mOuterWindowId = 1, mBrowsingContext = { mRawPtr = 0x7f88c6dde0}, mWindowGlobalChild = {mRef = {mRawPtr = 0x7f895aa790}}, mChildren = {<nsTArray_Impl<RefPtr<mozilla::dom::BrowsingContext>, nsTArrayInfallibleAllocator>> = { <nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_RelocateUsingMemutils>> = { mHdr = 0x7fbe0c86b8 <sEmptyTArrayHeader>}, <nsTArray_TypedBase<RefPtr<mozilla::dom::BrowsingContext>, nsTArray_Impl<RefPtr<mozilla::dom::BrowsingContext>, nsTArrayInfallibleAllocator> >> = { <nsTArray_SafeElementAtHelper<RefPtr<mozilla::dom::BrowsingContext>, nsTArray_Impl<RefPtr<mozilla::dom::BrowsingContext>, nsTArrayInfallibleAllocator> >> = {<nsTArray_SafeElementAtSmartPtrHelper<RefPtr<mozilla::dom::BrowsingContext>, nsTArray_Impl<RefPtr<mozilla::dom::BrowsingContext>, nsTArrayInfallibleAllocator> >> = {<detail::nsTArray_CopyDisabler> = {<No data fields>}, <No data fields>}, <No data fields>}, <No data fields>}, static NoIndex = 18446744073709551615}, <No data fields>}, mIsDiscarded = false, mIsInProcess = true, mCanExecuteScripts = true, mUserGestureStart = {mValue = {mUsedCanonicalNow = 0, mTimeStamp = 0}}} (gdb)I eventually end up here in nsDocShell.cpp:
aLoadInfo->SetLoadTriggeredFromExternal( aLoadState->HasLoadFlags(LOAD_FLAGS_FROM_EXTERNAL));This is important because of external are considered user triggered as mentioned in the comments in SecFetch::AddSecFetchUser():
// sec-fetch-user only applies if the request is user triggered. // requests triggered by an external application are considerd user triggered. if (!loadInfo->GetLoadTriggeredFromExternal() && !loadInfo->GetHasValidUserGestureActivation()) { return; }These load flags are set all over the place, although the LOAD_FLAGS_FROM_EXTERNAL specifically is only set in tabbrowser.js in the upstream code, which I believe is code we don't use for the Sailfish Browser. Instead we set these flags in EmbedLiteViewChild.cpp. It's possible there are some new flags which we need to add there. Let's check the history using git blame. I happen to see from the source code that the flag is being set on line 1798 of the tabbrowser.js file:
$ git blame browser/base/content/tabbrowser.js -L1798,1798 f9f59140398bc (Victor Porof 2019-07-05 09:48:57 +0200 1798) flags |= Ci.nsIWebNavigation.LOAD_FLAGS_FROM_EXTERNAL; $ git log -1 --oneline f9f59140398bc f9f59140398b Bug 1561435 - Format browser/base/, a=automatic-formatting $ git log -1 f9f59140398bc commit f9f59140398bc4d04d840e8217c04e0d7eafafb9 Author: Victor Porof <vporof@mozilla.com> Date: Fri Jul 5 09:48:57 2019 +0200 Bug 1561435 - Format browser/base/, a=automatic-formatting # ignore-this-changeset Differential Revision: https://phabricator.services.mozilla.com/D36041 --HG-- extra : source : 96b3895a3b2aa2fcb064c85ec5857b7216884556This commit is just reformatting the file so we need to look at the change prior to this one. Checking the diff of the automatic formatting I can see that before this the relevant line of code was line 1541 in the same file.
$ git blame f9f59140398bc~ browser/base/content/tabbrowser.js -L1541,1541 082b6eb1e7ed2 (James Willcox 2019-03-12 20:20:58 +0000 1541) flags |= Ci.nsIWebNavigation.LOAD_FLAGS_FROM_EXTERNAL; $ git log -1 082b6eb1e7ed2 commit 082b6eb1e7ed20de7424aea94fb7ce40b1b39c36 Author: James Willcox <snorp@snorp.net> Date: Tue Mar 12 20:20:58 2019 +0000 Bug 1524992 - Treat command line URIs as external r=mconley Differential Revision: https://phabricator.services.mozilla.com/D20890 --HG-- extra : moz-landing-system : landoThis is the important change. It added the flag specifically for URIs triggered at the command line. As it happens that's currently one of the ways I've been testing, so I should fix this. It's worth noting that this flag was introduced in ESR 67 so has actually been around for a while. But I guess skipping it didn't have any obvious negative effects, so nobody noticed it needed to be handled.
That'll have to change now. But it looks like this will be just one of many such flags that will need adding in to the Sailfish code.
Let's focus on LOAD_FLAGS_FROM_EXTERNAL first. This is set in browser.js when a call is made to getContentWindowOrOpenURI(). This ends up running this piece of code:
default: // OPEN_CURRENTWINDOW or an illegal value browsingContext = window.gBrowser.selectedBrowser.browsingContext; if (aURI) { let loadFlags = Ci.nsIWebNavigation.LOAD_FLAGS_NONE; if (isExternal) { loadFlags |= Ci.nsIWebNavigation.LOAD_FLAGS_FROM_EXTERNAL; } else if (!aTriggeringPrincipal.isSystemPrincipal) { // XXX this code must be reviewed and changed when bug 1616353 // lands. loadFlags |= Ci.nsIWebNavigation.LOAD_FLAGS_FIRST_LOAD; } gBrowser.loadURI(aURI.spec, { triggeringPrincipal: aTriggeringPrincipal, csp: aCsp, loadFlags, referrerInfo, }); }We end up here when there's a call made to handURIToExistingBrowser() in BrowserContentHandler.jsm which then calls browserDOMWindow.openURI() with the aFlags parameter set to Ci.nsIBrowserDOMWindow.OPEN_EXTERNAL.
What's the equivalent for Sailfish Browser? There are three ways it may end up opening an external URL that I can think of. The first is if the application is executed with a URL on the command line:
$ sailfish-browser https://www.flypig.co.uk/geckoAnother is if xdg-open is called:
$ xdg-open https://www.flypig.co.uk/geckoThe third is if the browser is called via its D-Bus interface:
$ gdbus call --session --dest org.sailfishos.browser.ui --object-path /ui \ --method org.sailfishos.browser.ui.openUrl \ "['https://www.flypig.co.uk/gecko']"All of these should end up setting the LOAD_FLAGS_FROM_EXTERNAL flag.
Let's follow the path in the case of the D-Bus call. The entry point for this is in browserservice.cpp where the D-Bus object is registered. The call looks like this:
void BrowserUIService::openUrl(const QStringList &args) { if(args.count() > 0) { emit openUrlRequested(args.first()); } else { emit openUrlRequested(QString()); } }This gets picked up by the browser object via a connection in main.cpp:
browser->connect(uiService, &BrowserUIService::openUrlRequested, browser, &Browser::openUrl);Which triggers the following method:
void Browser::openUrl(const QString &url) { Q_D(Browser); DeclarativeWebUtils::instance()->openUrl(url); }The version of the method in DeclarativeWebUtils sanitises the url before sending it on its way by emitting a signal:
emit openUrlRequested(tmpUrl);Finally this is picked up by BrowserPage.qml which ends up doing one of three things with it:
Connections { target: WebUtils onOpenUrlRequested: { [...] if (webView.tabModel.activateTab(url)) { webView.releaseActiveTabOwnership() } else if (!webView.tabModel.loaded) { webView.load(url) } else { webView.clearSelection() webView.tabModel.newTab(url) overlay.dismiss(true, !Qt.application.active /* immadiate */) }So it either activates an existing tab, calls the user interface to create the very first tab, or adds a new tab to the existing list. This approach contrasts with the route taken when the user enters a URL. In this case the process is handled in Overlay.qml and the loadPage() function there. This does a bunch of checks before calling this:
webView.load(pageUrl)Notice that this is also one of the methods that's called in the case of the D-Bus trigger as well. That's important, because we need to distinguish between these two routes. The WebView component inherits load() from DeclarativeWebContainer where the method looks like this:
void DeclarativeWebContainer::load(const QString &url, bool force) { QString tmpUrl = url; if (tmpUrl.isEmpty() || !browserEnabled()) { tmpUrl = ABOUT_BLANK; } if (!canInitialize()) { m_initialUrl = tmpUrl; } else if (m_webPage && m_webPage->completed()) { if (m_loading) { m_webPage->stop(); } m_webPage->loadTab(tmpUrl, force); Tab *tab = m_model->getTab(m_webPage->tabId()); if (tab) { tab->setRequestedUrl(tmpUrl); } } else if (m_model && m_model->count() == 0) { // Browser running all tabs are closed. m_model->newTab(tmpUrl); } }Notice that this mimics the D-Bus route with there being three options: record the URL in case there's a failure, load the URL into an existing tab or create a new tab if there are none already available.
Trying to follow all these routes is like trying to follow a droplet of water down a waterfall. I think I've reached the limit of my indirection capacity here; I'm going to need to pick this up again tomorrow.
But, so that I don't lose my thread, a note about two further things I need to do with this.
First I need to follow the process through until I hit the point at which the EmbedLiteViewParent::SendLoadURL() is called. This is the point at which the flag needs to be set. It looks like the common way for this to get called is through the following call:
void EmbedLiteView::LoadURL(const char* aUrl) { LOGT("url:%s", aUrl); Unused << mViewParent->SendLoadURL(NS_ConvertUTF8toUTF16(nsDependentCString(aUrl))); }I should check this with the debugger to make sure.
Second I need to ensure the flag gets passed from the point at which we know what its value needs to be (which is the D-Bus interface) to this call to SendLoadURL(), so that EmbedLiteViewChild::RecvLoadURL() can set it appropriately.
Once I have those two pieces everything will tie together and at that point it will be possible to set the LOAD_FLAGS_FROM_EXTERNAL flag appropriately.
That's it for today. Tomorrow, onwards.
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