Bug 173660 – Allow constructing a WTF:Function from a function pointer

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Chris Dumez]

Allow constructing a WTF:Function from a function pointer.

[Chris Dumez]

Created attachment 313526 [details]
Patch

[Chris Dumez]

Created attachment 313530 [details]
Patch

[Chris Dumez]

Created attachment 313531 [details]
Patch

[Darin Adler]

Comment on attachment 313531 [details]
Patch

Looks great. I remember one example from one of the std::function patches that was taking a bool that I don’t see you changing back here.

[Darin Adler]

Could add a unit test for this too since we have Function.cpp now.

[Chris Dumez]

(In reply to Darin Adler from comment #4)
> Comment on attachment 313531 [details]
> Patch
> 
> Looks great. I remember one example from one of the std::function patches
> that was taking a bool that I don’t see you changing back here.

networkStateChanged. Found it. Will fix as well, thanks.

[Chris Dumez]

(In reply to Darin Adler from comment #5)
> Could add a unit test for this too since we have Function.cpp now.

Good idea. Will do.

[Chris Dumez]

Created attachment 313538 [details]
Patch

[Darin Adler]

Comment on attachment 313538 [details]
Patch

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

> Source/WebCore/page/Page.cpp:284
> -        networkStateNotifier().addNetworkStateChangeListener([] (bool isOnLine) { networkStateChanged(isOnLine); });
> +        networkStateNotifier().addNetworkStateChangeListener(networkStateChanged);

My fault you added this, but it’s not compiling on Windows, maybe due to the different flavors of function pointer that exist on Windows.

> Source/WebCore/workers/Worker.cpp:62
> -        networkStateNotifier().addNetworkStateChangeListener([] (bool isOnLine) { networkStateChanged(isOnLine); });
> +        networkStateNotifier().addNetworkStateChangeListener(networkStateChanged);

Same here.

[Chris Dumez]

Created attachment 313552 [details]
Patch

[Build Bot]

Attachment 313552 [details] did not pass style-queue:


ERROR: Source/WebCore/workers/Worker.cpp:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/page/Page.cpp:284:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Comment on attachment 313552 [details]
Patch

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

> Source/WebCore/page/Page.cpp:284
> +        networkStateNotifier().addNetworkStateChangeListener(WTF::Function<void (bool)> { networkStateChanged });

Somehow Windows requires the explicit WTF::Function<void (bool)> { } here.

[WebKit Commit Bot]

Comment on attachment 313552 [details]
Patch

Clearing flags on attachment: 313552

Committed r218660: <http://trac.webkit.org/changeset/218660>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

(In reply to WebKit Commit Bot from comment #13)
> Comment on attachment 313552 [details]
> Patch
> 
> Clearing flags on attachment: 313552
> 
> Committed r218660: <http://trac.webkit.org/changeset/218660>

This change broke the Windows Debug build:
https://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/2230

[Chris Dumez]

(In reply to Ryan Haddad from comment #15)
> (In reply to WebKit Commit Bot from comment #13)
> > Comment on attachment 313552 [details]
> > Patch
> > 
> > Clearing flags on attachment: 313552
> > 
> > Committed r218660: <http://trac.webkit.org/changeset/218660>
> 
> This change broke the Windows Debug build:
> https://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/
> 2230

Will fix now, thanks.

[Chris Dumez]

(In reply to Chris Dumez from comment #16)
> (In reply to Ryan Haddad from comment #15)
> > (In reply to WebKit Commit Bot from comment #13)
> > > Comment on attachment 313552 [details]
> > > Patch
> > > 
> > > Clearing flags on attachment: 313552
> > > 
> > > Committed r218660: <http://trac.webkit.org/changeset/218660>
> > 
> > This change broke the Windows Debug build:
> > https://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/
> > 2230
> 
> Will fix now, thanks.

<http://trac.webkit.org/changeset/218671>

[Chris Dumez]

Comment on attachment 313552 [details]
Patch

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

>> Source/WebCore/page/Page.cpp:284
>> +        networkStateNotifier().addNetworkStateChangeListener(WTF::Function<void (bool)> { networkStateChanged });
> 
> Somehow Windows requires the explicit WTF::Function<void (bool)> { } here.

I really need to find a way to make Window build without this. This is ugly :/