Bug 209254 - Delay calling 'notifyThisWebProcessPoolWasCreated' until constructor finishes
Summary: Delay calling 'notifyThisWebProcessPoolWasCreated' until constructor finishes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-03-18 16:11 PDT by Brent Fulgham
Modified: 2020-03-18 17:41 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.60 KB, patch)
2020-03-18 16:31 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch for landing (1.61 KB, patch)
2020-03-18 16:58 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch for landing (1.63 KB, patch)
2020-03-18 17:04 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2020-03-18 16:11:23 PDT
The constructor for WebProcessPool calls a method 'notifyThisWebProcessPoolWasCreated', which calls a set of functions registered by observers interested in knowing that the pool was constructed.

However, since this call is made during the construction of the object, code that attempts to do things like Ref<> the WebProcessPool do so with a partially-constructed object.

Instead, we should enqueue the notification call so that it fires on the next spin of the runloop, after the current object is finished being constructed.
Comment 1 Brent Fulgham 2020-03-18 16:15:46 PDT
<rdar://problem/60564526>
Comment 2 Brent Fulgham 2020-03-18 16:31:12 PDT
Created attachment 393914 [details]
Patch
Comment 3 Chris Dumez 2020-03-18 16:43:00 PDT
Comment on attachment 393914 [details]
Patch

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

I think this is likely fine. I have added Brady in cc though because I think he added this code.

> Source/WebKit/UIProcess/WebProcessPool.cpp:309
> +    ASSERT(isMainThread());

We should not be using isMainThread() in the UIProcess because an app on iOS may be using for WK1 and WK2. You want RunLoop::isMain() instead.

> Source/WebKit/UIProcess/WebProcessPool.cpp:310
> +    callOnMainThread([weakPtr = makeWeakPtr(this)] {

makeWeakPtr(*this) will save a branch.

Also this needs to be RunLoop::main() dispatch() for same reasons as above.
Comment 4 Brent Fulgham 2020-03-18 16:58:27 PDT
Created attachment 393918 [details]
Patch for landing
Comment 5 Brent Fulgham 2020-03-18 17:04:16 PDT
Created attachment 393920 [details]
Patch for landing
Comment 6 EWS 2020-03-18 17:41:41 PDT
Committed r258671: <https://trac.webkit.org/changeset/258671>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 393920 [details].