639707 - Add resume session support on macOS

Status: RESOLVED FIXED

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

Comment 1

[(not currently active) Ted Mielczarek]

I presume this refers to the "Automatic Termination" feature? (In general, it's nice when bugs are filed with a little more information...)

http://developer.apple.com/library/mac/#documentation/General/Conceptual/MOSXAppProgrammingGuide/CoreAppDesign/CoreAppDesign.html

Comment 2

[David Smith]

Automatic Termination, Resume, and Auto Save are interwoven, but certainly not the same feature. Resume is about state restoration, not process management.

Comment 3

[(not currently active) Ted Mielczarek]

Okay, I had to poke around a little more to find docs describing this behavior.

Comment 4

[Josh Aas]

Scott - can you clarify which feature you mean, re: the last two comments?

Comment 5

[Benoit Girard (:BenWa)]

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.

Comment 6

[Alex]

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?

Comment 7

[David Smith]

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.

Comment 8

[AJ Jordan [:strugee] (he/they)]

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.

Comment 9

[David Smith]

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).

Comment 11

[Matthew N. [:MattN]]

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

Comment 12

[Alex Davis [:adavis]]

What are next steps for this?

Who can check through the above documentation to test this out?

Comment 13

[Matthew N. [:MattN]]

André/Stephen, is this something you can take a look at? This hurts user retention since other browsers resume but not Firefox.

Comment 14

[André Reinald]

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).

Comment 15

[André Reinald]

There may also be some useful information in bug 1051979. I added both bugs in the "see also" list.

Comment 16

[Jim Mathies [:jimm]]

for investigation

Comment 18

[Albert Scheiner [:alberts]]

As bug 1326181 has been marked as dup, is Lion still supported by Firefox or should the task be updated to macOS 10.10/.11?

Comment 19

[Markus Stange [:mstange]]

This bug was initially filed when 10.7 was brand new, that's why it was in the summary. I've updated it.

Comment 20

[Albert Scheiner [:alberts]]

Thanks Markus. I would have done it, but wasn't sure about current support.

Comment 21

[Mike de Boer [:mikedeboer]]

Attachment: My feeble attempt - by looking at and porting the Chrome code over

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?

Comment 23

[Reuben Morais [:reuben]]

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

Comment 24

[Haik Aftandilian [:haik]]

(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.

Comment 25

[Alex Davis [:adavis]]

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

Comment 26

[Stephen A Pohl [:spohl]]

(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.

Comment 27

[Matthew N. [:MattN]]

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.

Comment 29

[Matthew N. [:MattN]]

Attachment: Bug 639707 - Backed out changeset 5c2c9bbf9fcc

Comment 30

[Matthew N. [:MattN]]

Attachment: Bug 639707 - Restore the ability to open new windows in new spaces when a window is already in fullscreen mode on macOS. r=mstange

Backed out changeset ab70879b5734

Depends on D83740

Comment 31

[Matthew N. [:MattN]]

Attachment: Bug 639707 - Add resume session support on macOS.

Depends on D83741

Comment 32

[Matthew N. [:MattN]]

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.

Comment 33

[Matthew N. [:MattN]]

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.

Comment 34

[Haik Aftandilian [:haik]]

(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.

Comment 35

[Haik Aftandilian [:haik]]

(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.

Comment 36

[Stephen A Pohl [:spohl]]

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.

Comment 37

[Haik Aftandilian [:haik]]

(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.

Comment 41

[Stephen A Pohl [:spohl]]

Attachment: Bug 639707 - 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

Comment 42

[Stephen A Pohl [:spohl]]

Attachment: Bug 639707 - Add resume session support on macOS. Based on a patch by :MattN. r=mstange

Depends on D167445

Comment 43

[Stephen A Pohl [:spohl]]

Attachment: Bug 639707: Add telemetry to detect how often we are getting restored by macOS vs. users manually launching the application. r=mstange

Depends on D167447

Comment 44

[Stephen A Pohl [:spohl]]

Attachment: data review

Comment 45

[Chris H-C :chutten]

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+

Comment 46

[Pulsebot]

Pushed by [e-mail]:
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

Comment 47

[Iulian Moraru]

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

• Push with failures

• Failure log

• Backout link

[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'

Comment 48

[Pulsebot]

Pushed by [e-mail]:
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

Comment 49

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/a0893e621114
https://hg.mozilla.org/mozilla-central/rev/33700a5a9e69
https://hg.mozilla.org/mozilla-central/rev/3ef47a8cdcfd

Comment 50

[Donal Meehan [:dmeehan]]

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

Comment 51

[Stephen A Pohl [:spohl]]

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.

Comment 52

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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?

Comment 53

[Stephen A Pohl [:spohl]]

(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.

Comment 54

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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.