Closed Bug 639707 Opened 13 years ago Closed 1 year ago

Add resume session support on macOS

Categories

(Core :: Widget: Cocoa, defect, P2)

All
macOS
defect

Tracking

()

RESOLVED FIXED
111 Branch
Tracking Status
firefox111 --- fixed

People

(Reporter: sgreenlay, Assigned: spohl)

References

(Blocks 3 open bugs, )

Details

(Whiteboard: [fxgrowth], tpi:+,[mac:integration])

Attachments

(4 files, 4 obsolete files)

      No description provided.
Okay, I had to poke around a little more to find docs describing this behavior.
Scott - can you clarify which feature you mean, re: the last two comments?
I would guess that this is the behavior we'd want to make sure it well supported:
http://www.tuaw.com/2011/07/22/os-x-lions-resume-feature-lets-you-pick-up-where-you-left-off/

For us that would mean closing firefox without any prompts and during the resume restoring the profile. Ideally we could also store state for each tab (scroll position, text entry etc...) but this may not be feasible.
The way I see it (though I might we wrong) it's like the session restore feature we already have, just faster because it's using OS features. So this would basically replace session restore on Lion. It should especially be way faster when rebooting, if one chooses to save and restore all windows.
I'm not sure about automatic termination, does anyone know how that works? Like, does the OS automatically terminate any apps that support resume?
Anyone with a Mac developer account (even the free one) can view the WWDC session on the subject of both Automatic Termination and Resume. Go to https://developer.apple.com/videos/wwdc/2011/ and look for "Resume and Automatic Termination in Lion".

To summarize though: automatic termination and resume are independent, but the former works better if you support the latter.

Also, the resume APIs are suited to integrating with existing mechanisms like session restore.
As I understand it (and I may be wrong) Resume will restore the last state of the application before it was quit, but automatic termination has something to do with the application (explicitly) telling OS X when it can be terminated by the operating system. However, I believe automatic termination won't apply unless the application explicitly states that it supports it.
Alex, automatic termination is significantly more complicated than that, but your general view of it is correct. The WWDC video really is the best resource, but you can think of it as decoupling the idea of "process" and "application"; in an automatic-termination-aware application you can have an app with no process, a process with no app, or the more usual combinations. This allows the system to optimize both memory usage (killing processes when the user can't tell), and relaunch time (not killing processes when memory isn't short).
Blocks: 714147
Summary: [10.7] add resume support for Mac OS X 10.7 Lion → [10.7] add resume session support for Mac OS X 10.7 Lion
I think the info.plist option in bug 967970 explicitly disabled our support for this but according to [1], Apple was forcing this result for org.mozilla.firefox already anyways.

I think we need to do something along the lines of (untested):
1) Remove NSDisablePersistence or set it to False (in order to override the org.mozilla.firefox special handling?)
2) Implement application:shouldSaveApplicationState: and/or window:setRestorable: and properly handle private windows and ensure no private window titles leak to storage [2]. We don't actually need/want to store/use any specific new state as session restore can already handle all of this in a cross-platform manner, we just need OS X to think we can restore.
3) Test that the above works for org.mozilla.firefox, not just nightly. If not, figure out how to bypass Apple's hard-coding.

[2] is a good walk-though on all of this.

[1] https://trac.torproject.org/projects/tor/ticket/8987#comment:7
[2] https://developer.apple.com/library/ios/documentation/iPhone/Conceptual/iPhoneOSProgrammingGuide/StrategiesforImplementingYourApp/StrategiesforImplementingYourApp.html#//apple_ref/doc/uid/TP40007072-CH5-SW33
Hardware: x86 → All
Whiteboard: [fxgrowth]
What are next steps for this?

Who can check through the above documentation to test this out?
Flags: needinfo?(MattN+bmo)
André/Stephen, is this something you can take a look at? This hurts user retention since other browsers resume but not Firefox.
Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(areinald.bug)
I don't know much about the flags in the plist. But the following comes to my mind:

As far as I understand, persistence of opened windows/tabs is handled by Firefox itself and not by Mac OS X. So having the system relaunch Firefox on session opening would be enough. I can think of a few (Apple-supported) hacks to do just that.

A side note:

Some time ago I opened bug 996056. This bug was present in 10.9, and likely in 10.7 also. This bug is one reason (but obviously not the only one) why Firefox was not relaunched at session opening, as the user launchd was in charge of tracking and restoring user running processes.

In 10.11 (and maybe 10.10) Apple changed the launchd architecture so that there is no more "by user" launchd, only the "root" launchd. As a result, the current Firefox relaunch mechanism doesn't yield any longer the weirdness pointed in the mentioned bug. This also results in Firefox staying visible in Activity Monitor under "Applications in last 8 hours" mode, even after a relaunch (profile selection, update, crash etc).
Flags: needinfo?(areinald.bug)
There may also be some useful information in bug 1051979. I added both bugs in the "see also" list.
See Also: → 1051979, 996056
Flags: needinfo?(spohl.mozilla.bugs)
for investigation
Flags: needinfo?(MattN+bmo)
Priority: -- → P1
Whiteboard: [fxgrowth] → [fxgrowth], tpi:+
Priority: P1 → P2
Blocks: 1326181
First: this ended up not working. Or I'm using a testing methods on a Debug build that would never work anyways. Unsure.

But what :MattN said about Apple hardcoding Firefox to not be started seems to be correct. I don't know how or why, but AFAICT it's something that we need to get established as fact or fiction before we continue here!
This is something I don't know how to do without a bit of help...

I'm posting this approach here for posterity and to hopefully prove useful. I didn't port over the 'launch without opening a window when configured as a hidden login item' code, which was too hairy - the `ScopedCFTypeRef` template code proved too much for me to handle at this point and in the timeframe I gave myself.

IMHO, this is a huge gap in platform integration functionality that has been allowed to exist for way to long. How can I help to get this on a roadmap?

Haik, I'm needinfo'ing you because I saw you reached out to Apple in bug 1570451, but feel free to redirect if not applicable. Can we ping our contacts at Apple to verify if AppKit is really blacklisting us from starting on login as mentioned in comment #11 and the linked Tor bug [0]?

Looking at /System/Library/Frameworks/AppKit.framework/Versions/Current/AppKit on my machine running macOS Catalina 10.15.1 Beta I still see behavior that seems to match the Tor issue, specifically checking for _CFAppVersionLessThan(CFSTR("org.mozilla.firefox"), -1.0) somewhere inside _NSEnablePersistentUI.

[0] https://trac.torproject.org/projects/tor/ticket/8987#comment:7

Flags: needinfo?(haftandilian)

(In reply to Reuben Morais [:reuben] from comment #23)

Haik, I'm needinfo'ing you because I saw you reached out to Apple in bug 1570451, but feel free to redirect if not applicable. Can we ping our contacts at Apple to verify if AppKit is really blacklisting us from starting on login as mentioned in comment #11 and the linked Tor bug [0]?

Looking at /System/Library/Frameworks/AppKit.framework/Versions/Current/AppKit on my machine running macOS Catalina 10.15.1 Beta I still see behavior that seems to match the Tor issue, specifically checking for _CFAppVersionLessThan(CFSTR("org.mozilla.firefox"), -1.0) somewhere inside _NSEnablePersistentUI.

[0] https://trac.torproject.org/projects/tor/ticket/8987#comment:7

Done. I'll update the bug once I hear back.

Flags: needinfo?(haftandilian)

Just sharing this slide since it's relevant to this bug. One of the top reasons for users going to use another browser is that it's hard to find Firefox on their computer. Re-opening Firefox would help resolve that.

https://docs.google.com/presentation/d/136lS_izjRDoy3voVUxTbNSQjP1PANf3n-yVlNxApvPk/edit#slide=id.g7acb8982e8_0_49

(In reply to Alex Davis [:adavis] [PM FxA] from comment #25)

Just sharing this slide since it's relevant to this bug. One of the top reasons for users going to use another browser is that it's hard to find Firefox on their computer. Re-opening Firefox would help resolve that.

https://docs.google.com/presentation/d/136lS_izjRDoy3voVUxTbNSQjP1PANf3n-yVlNxApvPk/edit#slide=id.g7acb8982e8_0_49

I would also make sure to mention this in bugs related to writing an actual installer for macOS (rather than our current .dmg package), since an installer may be able to create a Dock icon and help in discoverability this way. :mhowell may know the state of things there.

A bundle ID deny list shouldn't hinder development of this fix since we use a different bundle identifier for development and Nightly builds (confirm with grep --after 1 CFBundleIdentifier obj-*/dist/*.app/Contents/Info.plist). If Mike's patch didn't work it's probably a bug, not due to _CFAppVersionLessThan. I suspect the issue was that we need to backout the two patches from bug 1405577 comment 20 as well (the problem is this). See that bug discussion for more info.

Once we have the patch working for non-release builds then we can figure out getting it to work for org.mozilla.firefox ones. Contact me for more info.

Severity: normal → --

Depends on D83741

I attempted to rebase Mike's patch on top of backing out the two commits in from bug 1405577 comment 20 in order to see if the patch works. In my quick test it still didn't restore after a macOS restart.

I won't have time to drive this over the finish line and would probably re-arrange some of the changes and commit messages but here it is for someone else to test and run with. I would also verify that my backouts are correct since they were very old and required a few manual fixes.

The stack actually does work now :-) I realized that I tested with mach run but then though I should have added --macos-open which did allow it to work. I didn't test just launching the .app normally but I don't see why that wouldn't work.

The stack needs some cleanup (probably looking at the Chromium code again would be good to see if there are other fixes in the last two years) and then review. Once it's working on Nightly we can address the org.mozilla.firefox issue.

(In reply to Matthew N. [:MattN] from comment #33)

The stack actually does work now :-) I realized that I tested with mach run but then though I should have added --macos-open which did allow it to work. I didn't test just launching the .app normally but I don't see why that wouldn't work.

That's great! FYI, the --macos-open mach option uses the Mac open(1) command which is meant to be equivalent to double clicking the icon in the Finder. Per the man page The open command opens a file (or a directory or URL), just as if you had double-clicked the file's icon.

(In reply to Haik Aftandilian [:haik] from comment #24)

(In reply to Reuben Morais [:reuben] from comment #23)

Haik, I'm needinfo'ing you because I saw you reached out to Apple in bug 1570451, but feel free to redirect if not applicable. Can we ping our contacts at Apple to verify if AppKit is really blacklisting us from starting on login as mentioned in comment #11 and the linked Tor bug [0]?

Looking at /System/Library/Frameworks/AppKit.framework/Versions/Current/AppKit on my machine running macOS Catalina 10.15.1 Beta I still see behavior that seems to match the Tor issue, specifically checking for _CFAppVersionLessThan(CFSTR("org.mozilla.firefox"), -1.0) somewhere inside _NSEnablePersistentUI.

[0] https://trac.torproject.org/projects/tor/ticket/8987#comment:7

Done. I'll update the bug once I hear back.

Given Matthew's progress, this may be a moot point, but Apple did get back to us and the hardcoding would not apply to builds using an SDK later than 10.7. We're using 10.11 for release builds so we should not be affected by it. And they noted that windows need to be restorable which Matthew was already testing with.

Attachment #9016357 - Attachment is obsolete: true
Whiteboard: [fxgrowth], tpi:+ → [fxgrowth], tpi:+,[mac:integration]
Blocks: 1782755
Flags: needinfo?(spohl.mozilla.bugs)

I have updated patches that I'd like to test. Do we know of a way to simulate an OS restart after an OS update? Maybe :MattN or :haik, would you know? Unfortunately I don't have a pending OS update at the moment to test with.

(In reply to Matthew N. [:MattN] from comment #33)

The stack actually does work now :-) I realized that I tested with mach run but then though I should have added --macos-open which did allow it to work. I didn't test just launching the .app normally but I don't see why that wouldn't work.

The stack needs some cleanup (probably looking at the Chromium code again would be good to see if there are other fixes in the last two years) and then review. Once it's working on Nightly we can address the org.mozilla.firefox issue.

I have verified that there have been no changes to the Chromium code since then.

Flags: needinfo?(mozilla+bmo)
Flags: needinfo?(haftandilian)

(In reply to Stephen A Pohl [:spohl] from comment #36)

I have updated patches that I'd like to test. Do we know of a way to simulate an OS restart after an OS update? Maybe :MattN or :haik, would you know? Unfortunately I don't have a pending OS update at the moment to test with.

I don't know how we could simulate that. The softwareupdate(8) man page doesn't list anything that would help with this kind of testing. However, running sudo reboot may be a good test outside of an actual update. I tested that with Terminal, Firefox, and Chrome running and after logging back in after the reboot, only Firefox was not restored.

Flags: needinfo?(haftandilian)
Duplicate of this bug: 1051979
Duplicate of this bug: 1326181
Duplicate of this bug: 1782755
Assignee: nobody → spohl.mozilla.bugs
Status: NEW → ASSIGNED
Attached file data review
Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(mozilla+bmo)
Attachment #9313355 - Flags: data-review?(chutten)
Attachment #9163955 - Attachment is obsolete: true
Attachment #9163954 - Attachment is obsolete: true
Attachment #9163953 - Attachment is obsolete: true

Comment on attachment 9313355 [details]
data review

DATA COLLECTION REVIEW RESPONSE:

Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes.

Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.

If the request is for permanent data collection, is there someone who will monitor the data over time?

No. This collection will expire in Firefox 116.

Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1, Technical.

Is the data collection request for default-on or default-off?

Default on for all channels.

Does the instrumentation include the addition of any new identifiers?

No.

Is the data collection covered by the existing Firefox privacy notice?

Yes.

Does the data collection use a third-party collection tool?

No.


Result: datareview+

Attachment #9313355 - Flags: data-review?(chutten) → data-review+
Pushed by spohl@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a3f8bdd9e837
Back out changeset 5c2c9bbf9fcc and replace with a patch to restore the ability to open new windows in new spaces when a window is already in fullscreen mode on macOS that supports macOS session restore. r=mstange
https://hg.mozilla.org/integration/autoland/rev/23c4048448bf
Add resume session support on macOS. Based on a patch by :MattN. r=mstange
https://hg.mozilla.org/integration/autoland/rev/0e733d6ed42d
Add telemetry to detect how often we are getting restored by macOS vs. users manually launching the application. r=mstange

Backed out for causing macOS build bustages on nsCocoaUtils.mm.

[task 2023-02-02T22:53:39.904Z] 22:53:39     INFO -  gmake[4]: Entering directory '/builds/worker/workspace/obj-build/widget/cocoa'
[task 2023-02-02T22:53:39.905Z] 22:53:39     INFO -  widget/cocoa/Unified_mm_widget_cocoa1.o
[task 2023-02-02T22:53:39.908Z] 22:53:39     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ -isysroot /builds/worker/fetches/MacOSX13.0.sdk -mmacosx-version-min=10.12 -stdlib=libc++ -Qunused-arguments -std=gnu++17 --target=x86_64-apple-darwin -o Unified_mm_widget_cocoa1.o -c  -fvisibility=hidden -fvisibility-inlines-hidden -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -ftrivial-auto-var-init=pattern -DDEBUG=1 -DOS_POSIX=1 -DOS_MACOSX=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/widget/cocoa -I/builds/worker/workspace/obj-build/widget/cocoa -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/checkouts/gecko/dom/events -I/builds/worker/checkouts/gecko/dom/media/platforms/apple -I/builds/worker/checkouts/gecko/layout/base -I/builds/worker/checkouts/gecko/layout/forms -I/builds/worker/checkouts/gecko/layout/generic -I/builds/worker/checkouts/gecko/layout/style -I/builds/worker/checkouts/gecko/layout/xul -I/builds/worker/checkouts/gecko/widget -I/builds/worker/checkouts/gecko/widget/headless -I/builds/worker/checkouts/gecko/gfx/skia -I/builds/worker/checkouts/gecko/gfx/skia/skia -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -fno-sized-deallocation -fno-aligned-new -fcrash-diagnostics-dir=/builds/worker/artifacts -fno-exceptions -fPIC -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -gdwarf-4 -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O3 -fno-omit-frame-pointer -funwind-tables -Werror -Wall -Wbitfield-enum-conversion -Wdeprecated-this-capture -Wempty-body -Wformat-type-confusion -Wignored-qualifiers -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtautological-constant-in-range-compare -Wtype-limits -Wno-error=tautological-type-limit-compare -Wunreachable-code -Wunreachable-code-return -Wunused-but-set-parameter -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wno-range-loop-analysis -Wc++2a-compat -Wenum-compare-conditional -Wenum-float-conversion -Wno-ambiguous-reversed-operator -Wno-error=deprecated -Wno-error=deprecated-anon-enum-enum-conversion -Wno-error=deprecated-enum-enum-conversion -Wno-error=deprecated-pragma -Wno-error=deprecated-this-capture -Wcomma -Wimplicit-fallthrough -Wduplicate-method-arg -Wduplicate-method-match -Wmissing-method-return-type -Wobjc-signed-char-bool -Wsemicolon-before-method-body -Wsuper-class-method-mismatch -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wno-error=atomic-alignment -Wno-error=deprecated-builtins -Wformat -Wformat-security -Wno-psabi -Wthread-safety -Werror=unguarded-availability-new -Wno-unknown-warning-option -fno-strict-aliasing -ffp-contract=off  -MD -MP -MF .deps/Unified_mm_widget_cocoa1.o.pp   -x objective-c++ -fobjc-exceptions   Unified_mm_widget_cocoa1.mm
[task 2023-02-02T22:53:39.909Z] 22:53:39     INFO -  In file included from Unified_mm_widget_cocoa1.mm:11:
[task 2023-02-02T22:53:39.909Z] 22:53:39  WARNING -  /builds/worker/checkouts/gecko/widget/cocoa/nsCocoaUtils.mm:262:9: warning: 'GetProcessInformation' is deprecated: first deprecated in macOS 10.9 [-Wdeprecated-declarations]
[task 2023-02-02T22:53:39.910Z] 22:53:39     INFO -    if (::GetProcessInformation(&processSerialNumber, &processInfoRec) == noErr) {
[task 2023-02-02T22:53:39.910Z] 22:53:39     INFO -          ^
[task 2023-02-02T22:53:39.911Z] 22:53:39     INFO -  /builds/worker/fetches/MacOSX13.0.sdk/System/Library/Frameworks/ApplicationServices.framework/Frameworks/HIServices.framework/Headers/Processes.h:513:1: note: 'GetProcessInformation' has been explicitly marked deprecated here
[task 2023-02-02T22:53:39.911Z] 22:53:39     INFO -  GetProcessInformation(
[task 2023-02-02T22:53:39.911Z] 22:53:39     INFO -  ^
[task 2023-02-02T22:53:39.911Z] 22:53:39     INFO -  In file included from Unified_mm_widget_cocoa1.mm:11:
[task 2023-02-02T22:53:39.912Z] 22:53:39  WARNING -  /builds/worker/checkouts/gecko/widget/cocoa/nsCocoaUtils.mm:265:11: warning: 'GetProcessInformation' is deprecated: first deprecated in macOS 10.9 [-Wdeprecated-declarations]
[task 2023-02-02T22:53:39.912Z] 22:53:39     INFO -      if (::GetProcessInformation(&processInfoRec.processLauncher, &parentProcessInfo) == noErr) {
[task 2023-02-02T22:53:39.912Z] 22:53:39     INFO -            ^
[task 2023-02-02T22:53:39.913Z] 22:53:39     INFO -  /builds/worker/fetches/MacOSX13.0.sdk/System/Library/Frameworks/ApplicationServices.framework/Frameworks/HIServices.framework/Headers/Processes.h:513:1: note: 'GetProcessInformation' has been explicitly marked deprecated here
[task 2023-02-02T22:53:39.913Z] 22:53:39     INFO -  GetProcessInformation(
[task 2023-02-02T22:53:39.913Z] 22:53:39     INFO -  ^
[task 2023-02-02T22:53:39.913Z] 22:53:39     INFO -  In file included from Unified_mm_widget_cocoa1.mm:11:
[task 2023-02-02T22:53:39.913Z] 22:53:39    ERROR -  /builds/worker/checkouts/gecko/widget/cocoa/nsCocoaUtils.mm:300:3: error: call to 'ScalarSet' is ambiguous
[task 2023-02-02T22:53:39.913Z] 22:53:39     INFO -    Telemetry::ScalarSet(Telemetry::ScalarID::STARTUP_IS_RESTORED_BY_MACOS, shouldRestore);
[task 2023-02-02T22:53:39.913Z] 22:53:39     INFO -    ^~~~~~~~~~~~~~~~~~~~
[task 2023-02-02T22:53:39.914Z] 22:53:39     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/Telemetry.h:457:6: note: candidate function
[task 2023-02-02T22:53:39.914Z] 22:53:39     INFO -  void ScalarSet(mozilla::Telemetry::ScalarID aId, uint32_t aValue);
[task 2023-02-02T22:53:39.914Z] 22:53:39     INFO -       ^
[task 2023-02-02T22:53:39.914Z] 22:53:39     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/Telemetry.h:465:6: note: candidate function
[task 2023-02-02T22:53:39.915Z] 22:53:39     INFO -  void ScalarSet(mozilla::Telemetry::ScalarID aId, bool aValue);
[task 2023-02-02T22:53:39.915Z] 22:53:39     INFO -       ^
[task 2023-02-02T22:53:39.916Z] 22:53:39     INFO -  In file included from Unified_mm_widget_cocoa1.mm:20:
[task 2023-02-02T22:53:39.916Z] 22:53:39  WARNING -  /builds/worker/checkouts/gecko/widget/cocoa/nsCocoaWindow.mm:1731:5: warning: 'NSDisableScreenUpdates' is deprecated: first deprecated in macOS 10.11 - As of 10.11 it is not generally necessary to take explicit action to achieve visual atomicity. +[NSAnimationContext runAnimationGroup:] and other similar methods can be used when a stronger than normal need for visual atomicity is required. The NSAnimationContext methods do not suffer from the same performance problems as NSDisableScreenUpdates. [-Wdeprecated-declarations]
[task 2023-02-02T22:53:39.916Z] 22:53:39     INFO -      NSDisableScreenUpdates();
[task 2023-02-02T22:53:39.917Z] 22:53:39     INFO -      ^
[task 2023-02-02T22:53:39.917Z] 22:53:39     INFO -  /builds/worker/fetches/MacOSX13.0.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSGraphics.h:235:20: note: 'NSDisableScreenUpdates' has been explicitly marked deprecated here
[task 2023-02-02T22:53:39.917Z] 22:53:39     INFO -  APPKIT_EXTERN void NSDisableScreenUpdates(void) API_DEPRECATED("As of 10.11 it is not generally necessary to take explicit action to achieve visual atomicity. +[NSAnimationContext runAnimationGroup:] and other similar methods can be used when a stronger than normal need for visual atomicity is required. The NSAnimationContext methods do not suffer from the same performance problems as NSDisableScreenUpdates.", macos(10.0,10.11));
[task 2023-02-02T22:53:39.917Z] 22:53:39     INFO -                     ^
[task 2023-02-02T22:53:39.918Z] 22:53:39     INFO -  In file included from Unified_mm_widget_cocoa1.mm:20:
[task 2023-02-02T22:53:39.918Z] 22:53:39  WARNING -  /builds/worker/checkouts/gecko/widget/cocoa/nsCocoaWindow.mm:1737:5: warning: 'NSEnableScreenUpdates' is deprecated: first deprecated in macOS 10.11 - As of 10.11 it is not generally necessary to take explicit action to achieve visual atomicity. +[NSAnimationContext runAnimationGroup:] and other similar methods can be used when a stronger than normal need for visual atomicity is required. The NSAnimationContext methods do not suffer from the same performance problems as NSEnableScreenUpdates. [-Wdeprecated-declarations]
[task 2023-02-02T22:53:39.918Z] 22:53:39     INFO -      NSEnableScreenUpdates();
[task 2023-02-02T22:53:39.918Z] 22:53:39     INFO -      ^
[task 2023-02-02T22:53:39.919Z] 22:53:39     INFO -  /builds/worker/fetches/MacOSX13.0.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSGraphics.h:237:20: note: 'NSEnableScreenUpdates' has been explicitly marked deprecated here
[task 2023-02-02T22:53:39.919Z] 22:53:39     INFO -  APPKIT_EXTERN void NSEnableScreenUpdates(void) API_DEPRECATED("As of 10.11 it is not generally necessary to take explicit action to achieve visual atomicity. +[NSAnimationContext runAnimationGroup:] and other similar methods can be used when a stronger than normal need for visual atomicity is required. The NSAnimationContext methods do not suffer from the same performance problems as NSEnableScreenUpdates.", macos(10.0,10.11));
[task 2023-02-02T22:53:39.919Z] 22:53:39     INFO -                     ^
[task 2023-02-02T22:53:39.920Z] 22:53:39     INFO -  4 warnings and 1 error generated.
[task 2023-02-02T22:53:39.920Z] 22:53:39    ERROR -  gmake[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:678: Unified_mm_widget_cocoa1.o] Error 1
[task 2023-02-02T22:53:39.920Z] 22:53:39     INFO -  gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/widget/cocoa'
Flags: needinfo?(spohl.mozilla.bugs)
Pushed by spohl@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a0893e621114
Back out changeset 5c2c9bbf9fcc and replace with a patch to restore the ability to open new windows in new spaces when a window is already in fullscreen mode on macOS that supports macOS session restore. r=mstange
https://hg.mozilla.org/integration/autoland/rev/33700a5a9e69
Add resume session support on macOS. Based on a patch by :MattN. r=mstange
https://hg.mozilla.org/integration/autoland/rev/3ef47a8cdcfd
Add telemetry to detect how often we are getting restored by macOS vs. users manually launching the application. r=mstange
Flags: needinfo?(spohl.mozilla.bugs)
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 111 Branch
Regressions: 1814889

:spohl could you consider adding a relnote nomination on this? We can add it for the fx111 release notes

Due to the difficulty in testing this feature, we are not yet sure if this feature is fully, partially or not quite working yet. I'm also encountering difficulty in running telemetry queries with the added probe and will need to investigate this next week. We may want to hold off on a relnote until we have higher confidence. Setting n-i for me to follow up.

Flags: needinfo?(spohl.mozilla.bugs)

Stephen, what would testcases have to do so that we could test this feature? I assume at least covering parts of it might be helpful. Something that can be done with forcing a shutdown of Firefox?

(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #52)

Stephen, what would testcases have to do so that we could test this feature? I assume at least covering parts of it might be helpful. Something that can be done with forcing a shutdown of Firefox?

We would need to have a way to simulate a shutdown triggered by an OS update. We do not know how the OS signals that a restart was due to a an OS update, or rather, we do not know how to signal this manually. Then, we would need to simulate a restart of Firefox that was a result of said "OS restart after OS update". We detect this by checking Firefox's parent process for a process signature of 'lgnw'. Simulating this is an unresolved challenge. The rest of the functionality relies on our existing session restore mechanism, which presumably has existing tests.

Flags: needinfo?(spohl.mozilla.bugs)

We have Marionette sessionrestore tests that run a similar scenario but for Windows:
https://searchfox.org/mozilla-central/rev/aa3ccd258b64abfd4c5ce56c1f512bc7f65b844c/browser/components/sessionstore/test/marionette/session_store_test_case.py#217

Not sure if something similar might exist for MacOS but those tests could be re-used once we know about such a way.

See Also: → 1819739
See Also: → 1832454
See Also: → 1832464
See Also: → 1847347
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: