2055233 - External links fail to open on cold launch on macOS (AppleEvents GURL blocked by TCC/Hardened Runtime timeout during session restore)

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Rustam Himadiiev]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:152.0) Gecko/20100101 Firefox/152.0

Steps to reproduce:

1. Set Firefox as your default browser on macOS.
2. Enable "Restore previous session" in Firefox settings.
3. Fully close Firefox (Cmd+Q).
4. Click an external HTTP/HTTPS link in any other native macOS application.

Actual results:

Firefox launches, restores the previous session, and opens the clicked link in a new tab.

Expected results:

Firefox launches, restores the previous session, but the clicked link is completely ignored and lost.

Comment 1

[Rustam Himadiiev]

There was an error in the description

Environment:

• OS: macOS (15.x / 16.x)
• Architecture: arm64 (Apple Silicon)

Comment 2

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Cocoa' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 4

[Rustam Himadiiev]

Donal Meehan [:dmeehan]

It's not fixed in 153.

Comment 5

[Donal Meehan [:dmeehan]]

(In reply to Rustam Himadiiev from comment #4)

Donal Meehan [:dmeehan]

It's not fixed in 153.

Thanks for flagging, reopening the bug, and setting Bug 1988774 as the regressor for further investigation

Comment 6

[BugBot [:suhaib / :marco]]

:emilio, since you are the author of the regressor, bug 1988774, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Do we really know this is a regression from bug 1988774? I guess it does smell a lot like bug 2036237 indeed. I can reproduce on macOS, but not on a local build... Logan does this ring a bell?

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Ah, I can repro in a local build but it's a bit harder because different default profiles...

Comment 9

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1988774

Comment 10

[Haik Aftandilian [:haik]]

Emilio, are you looking into this?

Comment 11

[Logan Rosen [:Logan]]

I confirmed this is a startup/session-restore ordering race. Firefox receives and briefly opens the Apple Event URL, but SessionStore then sees only the original about:home startup argument and restores with overwriteTabs: true, removing the new tab.

If the event arrives early enough to become a startup argument, or session restore is disabled, the URL survives. Deferring the pending-window handoff until SessionStore initializes also preserved both tabs in repeated testing.

Mozregression points to Bug 1988774, likely because the SDK change altered event timing. Bug 2036237 still fixed its original buffering issue; this is a separate session-restore interaction.

Comment 12

[Emilio Cobos Álvarez [:emilio]]

So I added some logging to the application code, (basically dumping to a temporary file) and what I see in this case is something like this:

applicationWillFinishLaunching()
applicationDidFinishLaunching()
TakeStartupURLs()
StartupURLCollectionComplete()
openURLs(1)
openURLs [https://phabricator.services.mozilla.com/D311827]

So the StartupURLs stuff doesn't seem to be working as intended, instead, we call openURLs later, but session restore probably clobbers it? That happens even if I force overwriteTabs to false tho:

diff --git a/browser/components/sessionstore/SessionStore.sys.mjs b/browser/components/sessionstore/SessionStore.sys.mjs
index 33d06eef09ac..ed8f101a7289 100644
--- a/browser/components/sessionstore/SessionStore.sys.mjs
+++ b/browser/components/sessionstore/SessionStore.sys.mjs
@@ -6011,7 +6011,7 @@ var SessionStoreInternal = {
    *        external link as well
    */
   restoreWindow: function ssi_restoreWindow(aWindow, winData, aOptions = {}) {
-    let overwriteTabs = aOptions && aOptions.overwriteTabs;
+    let overwriteTabs = aOptions && aOptions.overwriteTabs && false;
     let firstWindow = aOptions && aOptions.firstWindow;

     this.restoreSidebar(aWindow, winData.sidebar, winData.isPopup);

So this seems like at the very least a pre-existing race that could happen during startup.

Even without session-store enabled, I get:

applicationDidFinishLaunching()
TakeStartupURLs()
StartupURLCollectionComplete()
openURLs(1)
openURLs [https://phabricator.services.mozilla.com/D312766]

So it seems to me like:

• The startup url stuff is not working at all.
• Session restore races with a very eager openURLs which creates a remote command line here.

I'm not familiar with either of those things tho...

Comment 13

[Emilio Cobos Álvarez [:emilio]]

Will keep digging but if anyone more familiar than me can it'd be greatly appreciated :)

Comment 14

[Emilio Cobos Álvarez [:emilio]]

Ok now I'm seeing it work by just adding some log statements to the command line handler code, awesome.

Comment 15

[Emilio Cobos Álvarez [:emilio]]

So... My understanding is that the SDK update broke the whole StartupURLs mechanism. Bug 2036237 fixed the issue that when the openURLs calls arrived after the collection was complete they were just dropped on the floor rather than passed to the remote command line handler.

Logan does that match your understanding? Have you observed the StartupURLs mechanism work as intended after the SDK update?

Comment 16

[Emilio Cobos Álvarez [:emilio]]

(And now this bug should probably fix those URIs getting dropped or clobbered during session restore...)

Comment 17

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 2055233 - Don't stop the InitMacApplication() runloop until first idle. r=Logan,#mac-reviewers

Before the SDK update, macOS did the openURLs calls sync, before
applicationDidFinishLaunching, so applicationDidFinishLaunching was a
nice place to stop the app again.

However those calls are just posted to the event loop, so this mechanism
was not working at all.

Bug 2036237 fixed the opening of these urls so that they went through
the remote service instead of getting dropped, but that still races with
session restore, which looks at whether the command line was empty for
example.

This is the lower risk fix, which fixes this mechanism to wait for those
events. In order to do that without arbitrary timeouts, we just wait for
the event loop to idle.

There's a (maybe better) long term fix, which is removing this startup
url mechanism stuff altogether, handle all the openURLs method in the
remote service, and ensure session restore doesn't mess with them.

But that seems more annoying and risky, and this doesn't feel too
terrible? And it seems we might need the eager initialization anyways
for accessibility, so...

Comment 18

[Emilio Cobos Álvarez [:emilio]]

Dave, I think you're familiar with our remote / command-line handling stuff... Could you take a look at:

There's a (maybe better) long term fix, which is removing this startup
url mechanism stuff altogether, handle all the openURLs method in the
remote service, and ensure session restore doesn't mess with them.

I didn't dig too much but when getting an openURLs call very early on, I didn't even get to handURItoExistingBrowser and co. Do you know if that's expected? I don't know when exactly BrowserContentHandler gets registered off the top of my head...

Comment 19

[Dave Townsend [:mossop]]

I can look tomorrow, though this sounds very similar to bug 1600153.

Comment 20

[Emilio Cobos Álvarez [:emilio]]

Yeah I think that my patch effectively fixes bug 1600153 in another (maybe more minimal / less intrusive?) way

Comment 21

[Logan Rosen [:Logan]]

Yes, that matches what I observed. In the instrumented post-SDK cold launches I tested, application:openURLs: ran after TakeStartupURLs() / StartupURLCollectionComplete(), so the external LaunchServices URL was not captured by StartupURLs; I did not observe that mechanism working as intended after the SDK update.

With session restore disabled, the same URL still arrived through the later remote-command-line path and survived. That is consistent with bug 2036237 preventing the late URL from being dropped while leaving this session-restore race exposed. D313704 restores the intended startup collection ordering for this launch path.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/aa48e8bd99bb
https://hg.mozilla.org/integration/autoland/rev/8bf265da09cd
Don't stop the InitMacApplication() runloop until first idle. r=Logan

Comment 23

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/8bf265da09cd

Comment 24

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected, along with ESR.
:emilio, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta and ESR approvals.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox154 and the ESR status flag(s) to wontfix.

For more information, please visit BugBot documentation.

Comment 25

[Emilio Cobos Álvarez [:emilio]]

This has been shipping as is since bug 1988774 (FF 145) and is non-trivial so probably can ride the trains. Once it hits release we can try to uplift to ESR if we think it's worth it.