Bug 128353 – Consider making m_page in WebVideoFullscreenManagerProxy a reference

• Component: Media
• Status: RESOLVED INVALID

[Jon Lee]

Follow-up from Simon's review for 128143:

> Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.h:60
> +    WebPageProxy* m_page;

Can this be a reference?

[Jeremy Jones]

(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)]

(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]

rdar://problem/35319903

[Jeremy Jones]

Created attachment 325742 [details]
Patch

[Simon Fraser (smfr)]

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 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]

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]

Behaves correctly.

[Maciej Stachowiak]

Comment on attachment 325742 [details]
Patch

Unflagging and obsoleting, since this bug was resolved as invalid.