[webkit-changes] [WebKit/WebKit] bf341c: REGRESSION (260813 at main): Jamf Assessment app cras...

Wenson Hsieh noreply at github.com
Tue Oct 10 09:19:15 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: bf341c88387aafbac42d78a7e9f1b9ee929aaede
      https://github.com/WebKit/WebKit/commit/bf341c88387aafbac42d78a7e9f1b9ee929aaede
  Author: Wenson Hsieh <wenson_hsieh at apple.com>
  Date:   2023-10-10 (Tue, 10 Oct 2023)

  Changed paths:
    M Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm
    M Source/WebKit/UIProcess/WebProcessPool.cpp
    M Source/WebKit/UIProcess/WebProcessPool.h
    M Source/WebKit/UIProcess/glib/WebProcessPoolGLib.cpp
    M Source/WebKit/UIProcess/playstation/WebProcessPoolPlayStation.cpp
    M Source/WebKit/UIProcess/win/WebProcessPoolWin.cpp
    M Tools/TestWebKitAPI/SourcesCocoa.txt
    M Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
    A Tools/TestWebKitAPI/Tests/WebKitCocoa/InitializeWebViewWhenChangingUserDefaults.mm

  Log Message:
  -----------
  REGRESSION (260813 at main): Jamf Assessment app crashes on launch
https://bugs.webkit.org/show_bug.cgi?id=262931
rdar://115072267

Reviewed by Chris Dumez.

After the changes in 260813 at main, `registerUserDefaults` (called from `platformInitialize`) is no
longer robust against reentrancy. This causes infinite recursion in the Jamf Assessment app, which
listens to `NSUserDefaultsDidChangeNotification` and initializes a `WKWebView` instance in response
to the notification.

We can fix this while preserving the fact that there's a single "global static initialization" flag
in WebKit2, by simply setting the flag upon reading it in the constructor of `WebProcessPool`,
rather than at the very end of the `WebProcessPool` constructor.

Test: WebKit2.NoCrashWhenInitializeWebViewWhenChangingUserDefaults

* Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:
(WebKit::WebProcessPool::initializeShouldCrashWhenCreatingWebProcess):

Also remove a debug assertion that's no longer needed, since `ENABLE(WEBCONTENT_CRASH_TESTING)` is
no longer relevant.

(WebKit::WebProcessPool::platformInitialize):
* Source/WebKit/UIProcess/WebProcessPool.cpp:
* Source/WebKit/UIProcess/WebProcessPool.h:

Additionally use an enum class to represent this boolean flag (since it's now passed as an argument
to a function), and also negate it to represent whether or not we need static global initialization,
since it's no longer only set after completing static global initialization in `WebProcessPool`.

* Source/WebKit/UIProcess/glib/WebProcessPoolGLib.cpp:
(WebKit::WebProcessPool::platformInitialize):
* Source/WebKit/UIProcess/playstation/WebProcessPoolPlayStation.cpp:
(WebKit::WebProcessPool::platformInitialize):
* Source/WebKit/UIProcess/win/WebProcessPoolWin.cpp:
(WebKit::WebProcessPool::platformInitialize):
* Tools/TestWebKitAPI/SourcesCocoa.txt:
* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/InitializeWebViewWhenChangingUserDefaults.mm: Copied from Source/WebKit/UIProcess/playstation/WebProcessPoolPlayStation.cpp.

Add an API test to exercise the infinite recursion.

(-[UserDefaultChangeObserver init]):
(-[UserDefaultChangeObserver defaultsChanged:]):
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/269145@main




More information about the webkit-changes mailing list