Bug 158010

Summary: Use lambda capture with initializer instead of StringCapture
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: andersca, beidson, commit-queue, koivisto, ryanhaddad, sam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 158111    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
buildbot: commit-queue-
Archive of layout-test-results from ews123 for ios-simulator-wk2 none

Description Chris Dumez 2016-05-23 21:14:06 PDT 
Use lambda capture with initializer instead of StringCapture now that we support C++14.
Comment 1 Brady Eidson 2016-05-23 21:19:16 PDT 
đź‘Ťđź‘Ťđź‘Ť
Comment 2 Chris Dumez 2016-05-23 22:28:01 PDT 
Created attachment 279625 [details]
Patch
Comment 3 Chris Dumez 2016-05-24 09:50:11 PDT 
Created attachment 279665 [details]
Patch
Comment 4 Antti Koivisto 2016-05-24 11:40:44 PDT 
Comment on attachment 279665 [details]
Patch

Please also remove StringCapture when you get through all these.
Comment 5 WebKit Commit Bot 2016-05-24 12:02:35 PDT 
Comment on attachment 279665 [details]
Patch

Clearing flags on attachment: 279665

Committed r201341: <http://trac.webkit.org/changeset/201341>
Comment 6 WebKit Commit Bot 2016-05-24 12:02:41 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 7 Ryan Haddad 2016-05-24 14:37:46 PDT 
Reverted r201341 for reason:

This change may have caused LayoutTests to crash on Mac and iOS

Committed r201354: <http://trac.webkit.org/changeset/201354>
Comment 9 Chris Dumez 2016-05-24 16:10:46 PDT 
(In reply to comment #8)
> (In reply to comment #7)
> > Reverted r201341 for reason:
> > 
> > This change may have caused LayoutTests to crash on Mac and iOS
> > 
> > Committed r201354: <http://trac.webkit.org/changeset/201354>
> 
> <https://build.webkit.org/builders/
> Apple%20El%20Capitan%20Debug%20WK2%20%28Tests%29/builds/5396>
> <https://build.webkit.org/builders/
> Apple%20Yosemite%20Debug%20WK2%20%28Tests%29/builds/12355>
> <https://build.webkit.org/builders/
> Apple%20iOS%209%20Simulator%20Release%20WK2%20%28Tests%29/builds/5993>

Well, I have no idea yet why this caused crashes.
Comment 10 Chris Dumez 2016-05-24 22:09:35 PDT 
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > Reverted r201341 for reason:
> > > 
> > > This change may have caused LayoutTests to crash on Mac and iOS
> > > 
> > > Committed r201354: <http://trac.webkit.org/changeset/201354>
> > 
> > <https://build.webkit.org/builders/
> > Apple%20El%20Capitan%20Debug%20WK2%20%28Tests%29/builds/5396>
> > <https://build.webkit.org/builders/
> > Apple%20Yosemite%20Debug%20WK2%20%28Tests%29/builds/12355>
> > <https://build.webkit.org/builders/
> > Apple%20iOS%209%20Simulator%20Release%20WK2%20%28Tests%29/builds/5993>
> 
> Well, I have no idea yet why this caused crashes.

Does not seem like I can reproduce those crashes with Wk2 ElCapitan Debug :/
Comment 11 Brady Eidson 2016-05-24 22:10:25 PDT 
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > (In reply to comment #7)
> > > > Reverted r201341 for reason:
> > > > 
> > > > This change may have caused LayoutTests to crash on Mac and iOS
> > > > 
> > > > Committed r201354: <http://trac.webkit.org/changeset/201354>
> > > 
> > > <https://build.webkit.org/builders/
> > > Apple%20El%20Capitan%20Debug%20WK2%20%28Tests%29/builds/5396>
> > > <https://build.webkit.org/builders/
> > > Apple%20Yosemite%20Debug%20WK2%20%28Tests%29/builds/12355>
> > > <https://build.webkit.org/builders/
> > > Apple%20iOS%209%20Simulator%20Release%20WK2%20%28Tests%29/builds/5993>
> > 
> > Well, I have no idea yet why this caused crashes.
> 
> Does not seem like I can reproduce those crashes with Wk2 ElCapitan Debug :/

The Yosemite EWS bots were definitely causing me pain while this was in
Comment 12 Chris Dumez 2016-05-24 22:12:20 PDT 
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (In reply to comment #8)
> > > > (In reply to comment #7)
> > > > > Reverted r201341 for reason:
> > > > > 
> > > > > This change may have caused LayoutTests to crash on Mac and iOS
> > > > > 
> > > > > Committed r201354: <http://trac.webkit.org/changeset/201354>
> > > > 
> > > > <https://build.webkit.org/builders/
> > > > Apple%20El%20Capitan%20Debug%20WK2%20%28Tests%29/builds/5396>
> > > > <https://build.webkit.org/builders/
> > > > Apple%20Yosemite%20Debug%20WK2%20%28Tests%29/builds/12355>
> > > > <https://build.webkit.org/builders/
> > > > Apple%20iOS%209%20Simulator%20Release%20WK2%20%28Tests%29/builds/5993>
> > > 
> > > Well, I have no idea yet why this caused crashes.
> > 
> > Does not seem like I can reproduce those crashes with Wk2 ElCapitan Debug :/
> 
> The Yosemite EWS bots were definitely causing me pain while this was in

Yes, the crashes showed on several bots after this patch landed and the crashes went away after the patch was rolled out. The crashes were definitely caused by this patch. Unfortunately, I do not understand why yet.
Comment 13 Brady Eidson 2016-05-24 22:26:03 PDT 
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > (In reply to comment #9)
> > > > (In reply to comment #8)
> > > > > (In reply to comment #7)
> > > > > > Reverted r201341 for reason:
> > > > > > 
> > > > > > This change may have caused LayoutTests to crash on Mac and iOS
> > > > > > 
> > > > > > Committed r201354: <http://trac.webkit.org/changeset/201354>
> > > > > 
> > > > > <https://build.webkit.org/builders/
> > > > > Apple%20El%20Capitan%20Debug%20WK2%20%28Tests%29/builds/5396>
> > > > > <https://build.webkit.org/builders/
> > > > > Apple%20Yosemite%20Debug%20WK2%20%28Tests%29/builds/12355>
> > > > > <https://build.webkit.org/builders/
> > > > > Apple%20iOS%209%20Simulator%20Release%20WK2%20%28Tests%29/builds/5993>
> > > > 
> > > > Well, I have no idea yet why this caused crashes.
> > > 
> > > Does not seem like I can reproduce those crashes with Wk2 ElCapitan Debug :/
> > 
> > The Yosemite EWS bots were definitely causing me pain while this was in
> 
> Yes, the crashes showed on several bots after this patch landed and the
> crashes went away after the patch was rolled out. The crashes were
> definitely caused by this patch. Unfortunately, I do not understand why yet.

I have a really good guess - StringCapture makes isolatedCopy for cross thread handling, and getting rid of StringCapture got rid of that.

This patch definitely has cross thread ramifications in it.

This is one of the very first major thread safety issues we introduced when we added threading to WebCore, and one of the very first we figured out how to solve.

StringImpls cannot be created on one thread and then shared with another. Bad Things Will Happen™
Comment 14 Brady Eidson 2016-05-24 22:27:26 PDT 
(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > (In reply to comment #10)
> > > > (In reply to comment #9)
> > > > > (In reply to comment #8)
> > > > > > (In reply to comment #7)
> > > > > > > Reverted r201341 for reason:
> > > > > > > 
> > > > > > > This change may have caused LayoutTests to crash on Mac and iOS
> > > > > > > 
> > > > > > > Committed r201354: <http://trac.webkit.org/changeset/201354>
> > > > > > 
> > > > > > <https://build.webkit.org/builders/
> > > > > > Apple%20El%20Capitan%20Debug%20WK2%20%28Tests%29/builds/5396>
> > > > > > <https://build.webkit.org/builders/
> > > > > > Apple%20Yosemite%20Debug%20WK2%20%28Tests%29/builds/12355>
> > > > > > <https://build.webkit.org/builders/
> > > > > > Apple%20iOS%209%20Simulator%20Release%20WK2%20%28Tests%29/builds/5993>
> > > > > 
> > > > > Well, I have no idea yet why this caused crashes.
> > > > 
> > > > Does not seem like I can reproduce those crashes with Wk2 ElCapitan Debug :/
> > > 
> > > The Yosemite EWS bots were definitely causing me pain while this was in
> > 
> > Yes, the crashes showed on several bots after this patch landed and the
> > crashes went away after the patch was rolled out. The crashes were
> > definitely caused by this patch. Unfortunately, I do not understand why yet.
> 
> I have a really good guess - StringCapture makes isolatedCopy for cross
> thread handling, and getting rid of StringCapture got rid of that.
> 
> This patch definitely has cross thread ramifications in it.
> 
> This is one of the very first major thread safety issues we introduced when
> we added threading to WebCore, and one of the very first we figured out how
> to solve.
> 
> StringImpls cannot be created on one thread and then shared with another.
> Bad Things Will Happen™

Okay, looking deeper into the patch, I see you obviously took isolatedCopy into account.

My guess is still along these lines - Was one (or more) missed?
Comment 15 Chris Dumez 2016-05-24 22:32:23 PDT 
(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > (In reply to comment #10)
> > > > (In reply to comment #9)
> > > > > (In reply to comment #8)
> > > > > > (In reply to comment #7)
> > > > > > > Reverted r201341 for reason:
> > > > > > > 
> > > > > > > This change may have caused LayoutTests to crash on Mac and iOS
> > > > > > > 
> > > > > > > Committed r201354: <http://trac.webkit.org/changeset/201354>
> > > > > > 
> > > > > > <https://build.webkit.org/builders/
> > > > > > Apple%20El%20Capitan%20Debug%20WK2%20%28Tests%29/builds/5396>
> > > > > > <https://build.webkit.org/builders/
> > > > > > Apple%20Yosemite%20Debug%20WK2%20%28Tests%29/builds/12355>
> > > > > > <https://build.webkit.org/builders/
> > > > > > Apple%20iOS%209%20Simulator%20Release%20WK2%20%28Tests%29/builds/5993>
> > > > > 
> > > > > Well, I have no idea yet why this caused crashes.
> > > > 
> > > > Does not seem like I can reproduce those crashes with Wk2 ElCapitan Debug :/
> > > 
> > > The Yosemite EWS bots were definitely causing me pain while this was in
> > 
> > Yes, the crashes showed on several bots after this patch landed and the
> > crashes went away after the patch was rolled out. The crashes were
> > definitely caused by this patch. Unfortunately, I do not understand why yet.
> 
> I have a really good guess - StringCapture makes isolatedCopy for cross
> thread handling, and getting rid of StringCapture got rid of that.
> 
> This patch definitely has cross thread ramifications in it.
> 
> This is one of the very first major thread safety issues we introduced when
> we added threading to WebCore, and one of the very first we figured out how
> to solve.
> 
> StringImpls cannot be created on one thread and then shared with another.
> Bad Things Will Happen™

Not sure I understand. How is the new approach different from StringCapture? I do still create an isolatedCopy of String upon capturing it in the lambda, which then ends up getting used (and freed) in another thread.

Presumably, the same thing happened when capturing the StringCapture by value in the lambda because it would copy the StringCapture and the StringCapture copy constructor creates an isolated copy of the String.
Comment 16 Chris Dumez 2016-05-24 22:35:46 PDT 
(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > (In reply to comment #11)
> > > > (In reply to comment #10)
> > > > > (In reply to comment #9)
> > > > > > (In reply to comment #8)
> > > > > > > (In reply to comment #7)
> > > > > > > > Reverted r201341 for reason:
> > > > > > > > 
> > > > > > > > This change may have caused LayoutTests to crash on Mac and iOS
> > > > > > > > 
> > > > > > > > Committed r201354: <http://trac.webkit.org/changeset/201354>
> > > > > > > 
> > > > > > > <https://build.webkit.org/builders/
> > > > > > > Apple%20El%20Capitan%20Debug%20WK2%20%28Tests%29/builds/5396>
> > > > > > > <https://build.webkit.org/builders/
> > > > > > > Apple%20Yosemite%20Debug%20WK2%20%28Tests%29/builds/12355>
> > > > > > > <https://build.webkit.org/builders/
> > > > > > > Apple%20iOS%209%20Simulator%20Release%20WK2%20%28Tests%29/builds/5993>
> > > > > > 
> > > > > > Well, I have no idea yet why this caused crashes.
> > > > > 
> > > > > Does not seem like I can reproduce those crashes with Wk2 ElCapitan Debug :/
> > > > 
> > > > The Yosemite EWS bots were definitely causing me pain while this was in
> > > 
> > > Yes, the crashes showed on several bots after this patch landed and the
> > > crashes went away after the patch was rolled out. The crashes were
> > > definitely caused by this patch. Unfortunately, I do not understand why yet.
> > 
> > I have a really good guess - StringCapture makes isolatedCopy for cross
> > thread handling, and getting rid of StringCapture got rid of that.
> > 
> > This patch definitely has cross thread ramifications in it.
> > 
> > This is one of the very first major thread safety issues we introduced when
> > we added threading to WebCore, and one of the very first we figured out how
> > to solve.
> > 
> > StringImpls cannot be created on one thread and then shared with another.
> > Bad Things Will Happen™
> 
> Okay, looking deeper into the patch, I see you obviously took isolatedCopy
> into account.
> 
> My guess is still along these lines - Was one (or more) missed?
(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > (In reply to comment #11)
> > > > (In reply to comment #10)
> > > > > (In reply to comment #9)
> > > > > > (In reply to comment #8)
> > > > > > > (In reply to comment #7)
> > > > > > > > Reverted r201341 for reason:
> > > > > > > > 
> > > > > > > > This change may have caused LayoutTests to crash on Mac and iOS
> > > > > > > > 
> > > > > > > > Committed r201354: <http://trac.webkit.org/changeset/201354>
> > > > > > > 
> > > > > > > <https://build.webkit.org/builders/
> > > > > > > Apple%20El%20Capitan%20Debug%20WK2%20%28Tests%29/builds/5396>
> > > > > > > <https://build.webkit.org/builders/
> > > > > > > Apple%20Yosemite%20Debug%20WK2%20%28Tests%29/builds/12355>
> > > > > > > <https://build.webkit.org/builders/
> > > > > > > Apple%20iOS%209%20Simulator%20Release%20WK2%20%28Tests%29/builds/5993>
> > > > > > 
> > > > > > Well, I have no idea yet why this caused crashes.
> > > > > 
> > > > > Does not seem like I can reproduce those crashes with Wk2 ElCapitan Debug :/
> > > > 
> > > > The Yosemite EWS bots were definitely causing me pain while this was in
> > > 
> > > Yes, the crashes showed on several bots after this patch landed and the
> > > crashes went away after the patch was rolled out. The crashes were
> > > definitely caused by this patch. Unfortunately, I do not understand why yet.
> > 
> > I have a really good guess - StringCapture makes isolatedCopy for cross
> > thread handling, and getting rid of StringCapture got rid of that.
> > 
> > This patch definitely has cross thread ramifications in it.
> > 
> > This is one of the very first major thread safety issues we introduced when
> > we added threading to WebCore, and one of the very first we figured out how
> > to solve.
> > 
> > StringImpls cannot be created on one thread and then shared with another.
> > Bad Things Will Happen™
> 
> Okay, looking deeper into the patch, I see you obviously took isolatedCopy
> into account.
> 
> My guess is still along these lines - Was one (or more) missed?

Yes, this was my initial instinct as well. So far I have found any place where I forgot to create the isolated copy. So far I haven't found one yet but I'll keep looking. It looks like the crashes only happened on WK2 and only in the UIProcess so I guess the bug is in one of the 2 last files in the patch.
Comment 17 Chris Dumez 2016-05-25 10:48:20 PDT 
I may have an explanation for the crashes. Right now, RunLoop::main().dispatch() takes an std::function parameter by value. This means that when we call it with a capturing lambda, it will copy the captured variables (I believe). In my case, the variable used to be a StringCapture (which creates an isolated copy *every time* it is copied) and it is now a String for which we create an isolated copy only upon capture but *not* upon later copying. I think this creates a race in the cross thread case.

My proposal would be to move std::functions around (e.g. in RunLoop::dispatch) instead of copying them.

Unfortunately, since I haven't been able to reproduce the crashes yet. I haven't been able to confirm this would fix it.
Comment 18 Brady Eidson 2016-05-25 10:57:03 PDT 
(In reply to comment #17)
> I may have an explanation for the crashes. Right now,
> RunLoop::main().dispatch() takes an std::function parameter by value. This
> means that when we call it with a capturing lambda, it will copy the
> captured variables (I believe). In my case, the variable used to be a
> StringCapture (which creates an isolated copy *every time* it is copied) and
> it is now a String for which we create an isolated copy only upon capture
> but *not* upon later copying. I think this creates a race in the cross
> thread case.
> 
> My proposal would be to move std::functions around (e.g. in
> RunLoop::dispatch) instead of copying them.
> 
> Unfortunately, since I haven't been able to reproduce the crashes yet. I
> haven't been able to confirm this would fix it.

EWS could help, since the bots were seeing it tons.
Comment 19 Chris Dumez 2016-05-25 14:09:12 PDT 
Created attachment 279807 [details]
Patch
Comment 20 Chris Dumez 2016-05-25 14:09:54 PDT 
Now moving std::functions around to make sure we do not copy the captured strings after we've created the isolated copy.
Comment 21 WebKit Commit Bot 2016-05-25 14:11:03 PDT 
Attachment 279807 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/MainThread.cpp:147:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/WorkQueue.h:75:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:31:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:40:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:105:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/efl/WorkQueueEfl.cpp:57:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/FunctionDispatcher.h:41:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/RunLoop.cpp:126:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/MainThread.h:43:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 24 in 38 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 22 Chris Dumez 2016-05-25 14:51:05 PDT 
Created attachment 279822 [details]
Patch
Comment 23 WebKit Commit Bot 2016-05-25 14:53:28 PDT 
Attachment 279822 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/MainThread.cpp:147:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/WorkQueue.h:75:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:31:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:40:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:105:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/efl/WorkQueueEfl.cpp:57:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/FunctionDispatcher.h:41:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/RunLoop.cpp:126:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/MainThread.h:43:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 24 in 40 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 24 Chris Dumez 2016-05-25 14:54:47 PDT 
Created attachment 279823 [details]
Patch
Comment 25 Chris Dumez 2016-05-25 14:55:31 PDT 
Hopefully fixes the GTK and Windows build.
Comment 26 WebKit Commit Bot 2016-05-25 14:56:12 PDT 
Attachment 279823 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/MainThread.cpp:147:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/WorkQueue.h:75:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:31:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:40:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:105:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/efl/WorkQueueEfl.cpp:57:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/FunctionDispatcher.h:41:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/RunLoop.cpp:126:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/MainThread.h:43:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 24 in 40 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 27 Chris Dumez 2016-05-25 15:10:42 PDT 
Created attachment 279827 [details]
Patch
Comment 28 WebKit Commit Bot 2016-05-25 15:13:18 PDT 
Attachment 279827 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/MainThread.cpp:147:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/WorkQueue.h:75:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:31:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:40:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:105:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/efl/WorkQueueEfl.cpp:57:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/FunctionDispatcher.h:41:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/RunLoop.cpp:126:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/MainThread.h:43:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 25 in 41 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 29 Build Bot 2016-05-25 16:56:44 PDT 
Comment on attachment 279827 [details]
Patch

Attachment 279827 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1382602

New failing tests:
platform/ios-simulator/fast/text/arabic-with-no-supporting-webfont.html
Comment 30 Build Bot 2016-05-25 16:56:48 PDT 
Created attachment 279840 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.4
Comment 31 Chris Dumez 2016-05-25 18:51:21 PDT 
Comment on attachment 279827 [details]
Patch

Looks like it still crashes on iOS. I'll investigate.
Comment 32 Chris Dumez 2016-05-26 09:51:56 PDT 
I think moving the std::functions around does not help because I believe a copy is made anyway when initially constructing the std::function from the lambda, which causes the captured variables to be copied.

From http://en.cppreference.com/w/cpp/utility/functional/function/function:
template< class F > function( F f );
-> Initializes the target with a copy of f.

I think we need to use something else than an std::function for RunLoop / WorkQueue's dispatch() parameter to avoid problems.
Comment 33 Chris Dumez 2016-06-01 19:18:15 PDT 

*** This bug has been marked as a duplicate of bug 158277 ***