Bug 184718 - WebKit::DisplayLink maintains a strong reference to WebPageProxy, creating a reference cycle
Summary: WebKit::DisplayLink maintains a strong reference to WebPageProxy, creating a ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Conrad Shultz
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-04-17 17:02 PDT by Conrad Shultz
Modified: 2018-04-17 20:07 PDT (History)
4 users (show)

See Also:

Attachments
Patch (2.15 KB, patch)
2018-04-17 17:25 PDT, Conrad Shultz 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Conrad Shultz 2018-04-17 17:02:39 PDT 
WebKit::DisplayLink maintains a strong reference to WebPageProxy, creating a reference cycle.
Comment 1 Conrad Shultz 2018-04-17 17:03:31 PDT 
<rdar://problem/39468022>
Comment 2 Conrad Shultz 2018-04-17 17:03:46 PDT 
rdar://problem/39468022
Comment 3 Conrad Shultz 2018-04-17 17:04:11 PDT 
<rdar://problem/39468022>
Comment 4 Conrad Shultz 2018-04-17 17:06:32 PDT 
rdar://problem/39468022
Comment 5 Conrad Shultz 2018-04-17 17:07:45 PDT 
rdar://problem/39468022
Comment 6 Conrad Shultz 2018-04-17 17:08:55 PDT 
Oops...
Comment 7 Conrad Shultz 2018-04-17 17:25:23 PDT 
Created attachment 338167 [details]
Patch
Comment 8 Per Arne Vollan 2018-04-17 17:41:18 PDT 
Comment on attachment 338167 [details]
Patch

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

Thanks for finding and fixing this!

> Source/WebKit/UIProcess/mac/DisplayLink.cpp:-39
> -    : m_webPageProxy(webPageProxy)

The address of the WebPageProxy object is passed as a parameter to the function CVDisplayLinkSetOutputCallback below. This pointer is then passed as a void* parameter in the DisplayLink::displayLinkCallback method where it is casted to a WebPageProxy object. If it is guaranteed that we will not receive any callbacks after CVDisplayLinkStop is called in the destructor, then this patch is correct. Otherwise I think we have to use a weak pointer instead.
Comment 9 Tim Horton 2018-04-17 17:44:05 PDT 
Comment on attachment 338167 [details]
Patch

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

>> Source/WebKit/UIProcess/mac/DisplayLink.cpp:-39
>> -    : m_webPageProxy(webPageProxy)
> 
> The address of the WebPageProxy object is passed as a parameter to the function CVDisplayLinkSetOutputCallback below. This pointer is then passed as a void* parameter in the DisplayLink::displayLinkCallback method where it is casted to a WebPageProxy object. If it is guaranteed that we will not receive any callbacks after CVDisplayLinkStop is called in the destructor, then this patch is correct. Otherwise I think we have to use a weak pointer instead.

Good point
Comment 10 WebKit Commit Bot 2018-04-17 20:07:16 PDT 
Comment on attachment 338167 [details]
Patch

Clearing flags on attachment: 338167

Committed r230747: <https://trac.webkit.org/changeset/230747>
Comment 11 WebKit Commit Bot 2018-04-17 20:07:17 PDT 
All reviewed patches have been landed.  Closing bug.