Bug 198945 – Handle NSProgress calling our cancellation handler on background threads (and calling it more than once)

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Brady Eidson]

Handle NSProgress calling our cancellation handler on background threads (and calling it more than once)

[Brady Eidson]

<rdar://problem/51392926>

[Brady Eidson]

Created attachment 372300 [details]
Patch

[David Quesada]

Is it actually important to prevent the progress cancellation handler from calling cancel() twice? This could still technically happen if the NSProgress is canceled while the UI process cancels the download simultaneously - after one call to cancel() is made and the download is waiting for CFNetwork to deliver the resume data, the other call to cancel() can come in. Previously, this would have caused issues due to the Download's didCancel() being called multiple times, but as of r245901, it's safe to call Download::platformCancelNetworkLoad() multiple times. Technically we could add the "don't allow canceling multiple times" check to Download::cancel(), using the value of m_wasCanceled.

[Brady Eidson]

I thought we’d stopped using the SPI path for cancel.

Since we haven’t, you’re right. Also want to add some locking because while sleeping on it I realize this isn’t thread safe the way Foundation makes concurrent calls.

New patch coming soon.

[Brady Eidson]

Created attachment 372366 [details]
Patch

[Brady Eidson]

This new patch:
- Handles the NSProgress cancellation handler being called on any thread
- Handles it being called any number of times
- Handles it being called *concurrently* on different threads
- Handles WebKit::Download::cancel() being called multiple times (e.g. once from an NSProgress cancel and once from WebKit download SPI

[WebKit Commit Bot]

Comment on attachment 372366 [details]
Patch

Clearing flags on attachment: 372366

Committed r246568: <https://trac.webkit.org/changeset/246568>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.