Bug 196659 – [Cocoa] Awaken UIProcess if WebContent process is awakened from suspensions unexpectedly.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

[Cocoa] Awaken UIProcess in WebContent process is awakened from suspensions unexpectedly.

[Jer Noble]

Created attachment 366849 [details]
WIP

[Chris Dumez]

Comment on attachment 366849 [details]
WIP

View in context: https://bugs.webkit.org/attachment.cgi?id=366849&action=review

> Source/WebKit/Shared/Cocoa/ProcessTaskStateObserver.h:40
> +    ProcessTaskStateObserver(Client&);

explicit?

> Source/WebKit/UIProcess/Cocoa/WebProcessProxyCocoa.mm:44
> +static const Seconds UnexpectedActivityDuration = 10_s;

I think our statics usually start with a lowercase letter.

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:306
> +        uiProcessAssertion->setState(AssertionState::Suspended);

Do we really need to do this? Why doesn't destroying the ProcessAssertion suffice?

[Jer Noble]

(In reply to Chris Dumez from comment #2)
> Comment on attachment 366849 [details]
> WIP
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=366849&action=review
> 
> > Source/WebKit/Shared/Cocoa/ProcessTaskStateObserver.h:40
> > +    ProcessTaskStateObserver(Client&);
> 
> explicit?

Sure.

> > Source/WebKit/UIProcess/Cocoa/WebProcessProxyCocoa.mm:44
> > +static const Seconds UnexpectedActivityDuration = 10_s;
> 
> I think our statics usually start with a lowercase letter.

Ok.

> > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:306
> > +        uiProcessAssertion->setState(AssertionState::Suspended);
> 
> Do we really need to do this? Why doesn't destroying the ProcessAssertion
> suffice?

It probably does suffice; I was taking a belt-and-suspenders approach. I'll remove this.

[Geoffrey Garen]

Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ -arch x86_64 -dynamiclib -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -L/Volumes/Data/EWS/WebKit/WebKitBuild/Release -L/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks/WebKit.framework/Versions/A/Frameworks/WebCore.framework/Versions/A/Frameworks -F/Volumes/Data/EWS/WebKit/WebKitBuild/Release -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/PrivateFrameworks -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks -filelist /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/WebKit.LinkFileList -Xlinker -reexport_framework -Xlinker WebKitLegacy -Xlinker -reexport_framework -Xlinker WebCore -install_name /System/Library/Frameworks/WebKit.framework/Versions/A/WebKit -mmacosx-version-min=10.13 -dead_strip -Xlinker -object_path_lto -Xlinker /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/WebKit_lto.o -stdlib=libc++ -fobjc-link-runtime -iframework/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/PrivateFrameworks -Wl,-unexported_symbol -Wl,__ZTISt9bad_alloc -Wl,-unexported_symbol -Wl,__ZTISt9exception -Wl,-unexported_symbol -Wl,__ZTSSt9bad_alloc -Wl,-unexported_symbol -Wl,__ZTSSt9exception -Wl,-unexported_symbol -Wl,__ZdlPvS_ -Wl,-unexported_symbol -Wl,__ZnwmPv -Wl,-unexported_symbol -Wl,__Znwm -Wl,-unexported_symbol -Wl,__ZNSt3__18functionIFvN7WebCore12PolicyActionEEEC2EOS4_ -Wl,-unexported_symbol -Wl,__ZNSt3__18functionIFvN7WebCore12PolicyActionEEEC1EOS4_ -Wl,-unexported_symbol -Wl,__ZNSt3__18functionIFvN7WebCore12PolicyActionEEEaSEDn -Wl,-unexported_symbol -Wl,__ZNKSt3__18functionIFvN7WebCore12PolicyActionEEEclES2_ -Wl,-unexported_symbol -Wl,__ZNSt3__18functionIFvN7WebCore12PolicyActionEEE4swapERS4_ -Wl,-unexported_symbol -Wl,__ZNSt3__18functionIFvN7WebCore12PolicyActionEEEC1ERKS4_ -Wl,-unexported_symbol -Wl,__ZNSt3__18functionIFvN7WebCore12PolicyActionEEEC2ERKS4_ -Wl,-unexported_symbol -Wl,__ZNSt3__18functionIFvN7WebCore12PolicyActionEEED1Ev -Wl,-unexported_symbol -Wl,__ZNSt3__18functionIFvN7WebCore12PolicyActionEEED2Ev -Wl,-unexported_symbol -Wl,__ZNSt3__18functionIFvN7WebCore12PolicyActionEEEaSERKS4_ -Wl,-unexported_symbol -Wl,__ZTVNSt3__117bad_function_callE -Wl,-unexported_symbol -Wl,__ZTCNSt3__118basic_stringstreamIcNS_11char_traitsIcEENS_9allocatorIcEEEE0_NS_13basic_istreamIcS2_EE -Wl,-unexported_symbol -Wl,__ZTCNSt3__118basic_stringstreamIcNS_11char_traitsIcEENS_9allocatorIcEEEE0_NS_14basic_iostreamIcS2_EE -Wl,-unexported_symbol -Wl,__ZTCNSt3__118basic_stringstreamIcNS_11char_traitsIcEENS_9allocatorIcEEEE16_NS_13basic_ostreamIcS2_EE -Wl,-unexported_symbol -Wl,__ZTTNSt3__118basic_stringstreamIcNS_11char_traitsIcEENS_9allocatorIcEEEE -Wl,-unexported_symbol -Wl,__ZTVNSt3__115basic_stringbufIcNS_11char_traitsIcEENS_9allocatorIcEEEE -Wl,-unexported_symbol -Wl,__ZTVNSt3__118basic_stringstreamIcNS_11char_traitsIcEENS_9allocatorIcEEEE -Wl,-unexported_symbol -Wl,__ZTCNSt3__118basic_stringstreamIcNS_11char_traitsIcEENS_9allocatorIcEEEE8_NS_13basic_ostreamIcS2_EE -lobjc -framework CFNetwork -framework CoreAudio -framework CoreFoundation -framework CoreGraphics -framework CoreText -framework Foundation -framework ImageIO -framework IOKit -framework WebKitLegacy -lnetwork -framework AppKit -framework Carbon -weak_framework CorePrediction -framework CoreServices -framework IOSurface -lsandbox -weak-lwebrtc -framework OpenGL -weak_framework SafariSafeBrowsing -framework SecurityInterface -weak_framework WebInspectorUI -Wl,-not_for_dyld_shared_cache -framework JavaScriptCore -licucore -framework QuartzCore -framework Security -framework WebCore -compatibility_version 1 -current_version 608.1.16 -Xlinker -dependency_info -Xlinker /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/WebKit_dependency_info.dat -o /Volumes/Data/EWS/WebKit/WebKitBuild/Release/WebKit.framework/Versions/A/WebKit
Undefined symbols for architecture x86_64:
  "WebKit::ProcessAssertion::ProcessAssertion(int, WTF::String const&, WebKit::AssertionState, WebKit::AssertionReason)", referenced from:
      WebKit::WebProcess::processTaskStateDidChange(WebKit::ProcessTaskStateObserver::TaskState) in UnifiedSource51-mm.o
  "_OBJC_CLASS_$_BKSProcess", referenced from:
      objc-class-ref in ProcessTaskStateObserver.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

[Darin Adler]

Comment on attachment 366849 [details]
WIP

View in context: https://bugs.webkit.org/attachment.cgi?id=366849&action=review

> Source/WebKit/ChangeLog:3
> +        [Cocoa] Awaken UIProcess in WebContent process is awakened from suspensions unexpectedly.

Should be "if" instead of "in", right?

[Jer Noble]

(In reply to Darin Adler from comment #5)
> Comment on attachment 366849 [details]
> WIP
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=366849&action=review
> 
> > Source/WebKit/ChangeLog:3
> > +        [Cocoa] Awaken UIProcess in WebContent process is awakened from suspensions unexpectedly.
> 
> Should be "if" instead of "in", right?

Yep!

(In reply to Geoffrey Garen from comment #4)
> Undefined symbols for architecture x86_64:
>   "WebKit::ProcessAssertion::ProcessAssertion(int, WTF::String const&,
> WebKit::AssertionState, WebKit::AssertionReason)", referenced from:
>      
> WebKit::WebProcess::processTaskStateDidChange(WebKit::
> ProcessTaskStateObserver::TaskState) in UnifiedSource51-mm.o

Yeah, I'll fix this; the non-iOS ProcessAssertion needs to have a constructor that takes a reason.

>   "_OBJC_CLASS_$_BKSProcess", referenced from:
>       objc-class-ref in ProcessTaskStateObserver.o

Ooh, looks like we need to soft-link the BKSProcess class.

[Jer Noble]

> (In reply to Geoffrey Garen from comment #4)
> >   "_OBJC_CLASS_$_BKSProcess", referenced from:
> >       objc-class-ref in ProcessTaskStateObserver.o
> 
> Ooh, looks like we need to soft-link the BKSProcess class.

Aha, this is more prosaic: we don't link against AssertionServices.framework on macOS. While technically we could, maybe I'll just make this entire code-path iOS-only.

[Jer Noble]

Created attachment 366959 [details]
Patch for landing

[Jer Noble]

Created attachment 366961 [details]
Patch for landing

[Jer Noble]

Created attachment 366969 [details]
Patch for landing

[Jer Noble]

Created attachment 367040 [details]
Patch for landing

[Jer Noble]

Created attachment 367042 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 367042 [details]
Patch for landing

Clearing flags on attachment: 367042

Committed r244091: <https://trac.webkit.org/changeset/244091>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/49745738>