Bug 165255 - Make _WKFullscreenDelegate available to users of the WebKit2 C-API.
Summary: Make _WKFullscreenDelegate available to users of the WebKit2 C-API.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-01 09:20 PST by Jer Noble
Modified: 2017-01-24 13:26 PST (History)
2 users (show)

See Also:

Attachments
Patch (2.86 KB, patch)
2016-12-01 09:21 PST, Jer Noble 		no flags Details | Formatted Diff | Diff
Patch (2.91 KB, patch)
2016-12-01 12:50 PST, Jer Noble 		andersca: review+
Details | Formatted Diff | Diff
Patch for landing (2.96 KB, patch)
2017-01-17 10:20 PST, Jer Noble 		jer.noble: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (2.96 KB, patch)
2017-01-17 10:57 PST, Jer Noble 		no flags Details | Formatted Diff | Diff
Patch for landing (2.91 KB, patch)
2017-01-23 17:24 PST, Jer Noble 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) 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 Jer Noble 2016-12-01 09:20:26 PST 
Make _WKFullscreenDelegate available to users of the WebKit2 C-API.
Comment 1 Jer Noble 2016-12-01 09:21:43 PST 
Created attachment 295854 [details]
Patch
Comment 2 Jer Noble 2016-12-01 12:50:04 PST 
Created attachment 295881 [details]
Patch
Comment 3 Jer Noble 2016-12-01 13:49:30 PST 
The GTK error is in WebCore::jsSubtleCryptoPrototypeFunctionUnwrapKey(), unrelated to this patch.
Comment 4 Anders Carlsson 2016-12-01 14:09:12 PST 
Comment on attachment 295881 [details]
Patch

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

> Source/WebKit2/UIProcess/API/C/mac/WKPagePrivateMac.h:59
> +WK_EXPORT id <_WKFullscreenDelegate> WKPageGetFullscreenDelegate(WKPageRef page);

If possible, please use __weak id here.
Comment 5 Jer Noble 2017-01-06 14:34:34 PST 
(In reply to comment #4)
> Comment on attachment 295881 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=295881&action=review
> 
> > Source/WebKit2/UIProcess/API/C/mac/WKPagePrivateMac.h:59
> > +WK_EXPORT id <_WKFullscreenDelegate> WKPageGetFullscreenDelegate(WKPageRef page);
> 
> If possible, please use __weak id here.

Doesn't seem to be possible, as __weak only make sense when compiling under ARC, and WKPagePrivateMac.mm doesn't. Adding __weak here just generates a warning.
Comment 6 Jer Noble 2017-01-10 17:38:12 PST 
(In reply to comment #5)
> (In reply to comment #4)
> > Comment on attachment 295881 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=295881&action=review
> > 
> > > Source/WebKit2/UIProcess/API/C/mac/WKPagePrivateMac.h:59
> > > +WK_EXPORT id <_WKFullscreenDelegate> WKPageGetFullscreenDelegate(WKPageRef page);
> > 
> > If possible, please use __weak id here.
> 
> Doesn't seem to be possible, as __weak only make sense when compiling under
> ARC, and WKPagePrivateMac.mm doesn't. Adding __weak here just generates a
> warning.

We're already storing the delegate in a WeakObjCPtr, but there doesn't seem to a way to mark the return value of a C function as __weak (or a parameter as __weak) without generating a compiler error.

I guess I could do something like:

#if !defined WEAK
#if __has_feature(objc_arc)
#define WEAK _weak
#else
#define WEAK
#endif

and then mark up the methods as WK_EXPORT WEAK id <...>; Does that seem right? We don't seem to use __weak in any of the other C-style APIs.
Comment 7 Jer Noble 2017-01-17 10:20:15 PST 
Created attachment 299039 [details]
Patch for landing
Comment 8 Jer Noble 2017-01-17 10:57:31 PST 
Created attachment 299044 [details]
Patch for landing
Comment 9 Jer Noble 2017-01-23 17:24:49 PST 
Created attachment 299558 [details]
Patch for landing
Comment 10 WebKit Commit Bot 2017-01-24 11:57:13 PST 
Comment on attachment 299558 [details]
Patch for landing

Clearing flags on attachment: 299558

Committed r211094: <http://trac.webkit.org/changeset/211094>