Bug 128353

Summary: Consider making m_page in WebVideoFullscreenManagerProxy a reference
Product: WebKit Reporter: Jon Lee <jonlee>
Component: MediaAssignee: Jeremy Jones <jeremyj-wk>
Status: RESOLVED INVALID    
Severity: Normal CC: buildbot, jer.noble, mjs, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: iPhone / iPad   
OS: Unspecified   
Attachments:
Description Flags
Patch
buildbot: commit-queue-
Archive of layout-test-results from ews122 for ios-simulator-wk2 none

Jon Lee
Reported 2014-02-06 18:11:09 PST
Follow-up from Simon's review for 128143: > Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.h:60 > + WebPageProxy* m_page; Can this be a reference?
Attachments
Patch (11.37 KB, patch)
2017-11-02 12:10 PDT, Jeremy Jones
buildbot: commit-queue-
Archive of layout-test-results from ews122 for ios-simulator-wk2 (4.18 MB, application/zip)
2017-11-02 14:37 PDT, Build Bot
no flags
Jeremy Jones
Comment 1 2014-02-07 10:09:13 PST
(In reply to comment #0) > Follow-up from Simon's review for 128143: > > > Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.h:60 > > + WebPageProxy* m_page; > > Can this be a reference? This follows the same pattern in WebFullScreenManagerProxy. WebPageProxy has a RefPointer to the WebVideoFullscreenManagerProxy, so I think using a RefPtr here would create a retain cycle.
Simon Fraser (smfr)
Comment 2 2014-02-07 10:50:04 PST
(In reply to comment #1) > (In reply to comment #0) > > Follow-up from Simon's review for 128143: > > > > > Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.h:60 > > > + WebPageProxy* m_page; > > > > Can this be a reference? > > This follows the same pattern in WebFullScreenManagerProxy. > > WebPageProxy has a RefPointer to the WebVideoFullscreenManagerProxy, so I think using a RefPtr here would create a retain cycle. Not RefPtr. WebPageProxy&.
Jeremy Jones
Comment 3 2017-11-02 12:05:09 PDT
Jeremy Jones
Comment 4 2017-11-02 12:10:38 PDT
Simon Fraser (smfr)
Comment 5 2017-11-02 13:00:44 PDT
Comment on attachment 325742 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325742&action=review > Source/WebKit/ChangeLog:9 > + Cleanup. Use a WebPageProxy& instead of a pointer. Are we sure that we can do this, given than a WKWebView can outlive the WebPageProxy (see -[WKWebView _isValid])?
Build Bot
Comment 6 2017-11-02 14:37:08 PDT
Comment on attachment 325742 [details] Patch Attachment 325742 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5079739 New failing tests: imported/w3c/web-platform-tests/service-workers/service-worker/fetch-canvas-tainting.https.html
Build Bot
Comment 7 2017-11-02 14:37:09 PDT
Created attachment 325770 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Jeremy Jones
Comment 8 2017-11-03 10:48:14 PDT
Behaves correctly.
Maciej Stachowiak
Comment 9 2020-06-02 01:10:24 PDT
Comment on attachment 325742 [details] Patch Unflagging and obsoleting, since this bug was resolved as invalid.
Note You need to log in before you can comment on or make changes to this bug.