Bug 207185 – _WKDownload should expose the originating FrameInfo

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Brady Eidson]

_WKDownload should expose the originating FrameInfo

<rdar://problem/58022576>

[Brady Eidson]

Created attachment 389633 [details]
Patch

[Brady Eidson]

API test to come, but just wanted EWS to check the build, existing API tests, etc.

[Brady Eidson]

Created attachment 389731 [details]
Patch

[Geoffrey Garen]

Comment on attachment 389731 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/_WKDownload.mm:91
> +    if (frameInfo.page() == nullptr)
> +        frameInfo.setPage(_download->originatingPage());

Under what conditions will a FrameInfo lack a page, and why is it correct under those conditions to claim that its page was the page that originated the download? (Does the API test demonstrate this case?)

[Brady Eidson]

(In reply to Geoffrey Garen from comment #4)
> Comment on attachment 389731 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=389731&action=review
> 
> > Source/WebKit/UIProcess/API/Cocoa/_WKDownload.mm:91
> > +    if (frameInfo.page() == nullptr)
> > +        frameInfo.setPage(_download->originatingPage());
> 
> Under what conditions will a FrameInfo lack a page,

Most callers of DownloadProxy::create don't have a page available, so they are never created with a page, so the answer is "always"

> Does the API test demonstrate this case?

It does.

That said, I'll just go rework the DownloadProxy c'tor some more.

[Brady Eidson]

Created attachment 389745 [details]
Patch

[Geoffrey Garen]

Comment on attachment 389745 [details]
Patch

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

r=me

> Source/WebKit/UIProcess/API/APIFrameInfo.h:57
> +    void setPage(WebKit::WebPageProxy* page) { m_page = page; }

You can remove this setter now.

> Source/WebKit/UIProcess/API/Cocoa/_WKDownload.h:45
> +@property (nonatomic, readonly) WKFrameInfo *originatingFrameInfo WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));

Let's call this "originatingFrame". Even though the data type is WKFrameInfo, I think "frame" reads better, and it appears to be more consistent with our other WKFrameInfo APIs.

[Brady Eidson]

Created attachment 389849 [details]
Patch

[Brady Eidson]

Created attachment 389851 [details]
Patch

[Alex Christensen]

http://trac.webkit.org/r255845