Bug 158010 – Use lambda capture with initializer instead of StringCapture

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: WebCore Misc.

[Chris Dumez]

Use lambda capture with initializer instead of StringCapture now that we support C++14.

[Brady Eidson]

👍👍👍

[Chris Dumez]

Created attachment 279625 [details]
Patch

[Chris Dumez]

Created attachment 279665 [details]
Patch

[Antti Koivisto]

Comment on attachment 279665 [details]
Patch

Please also remove StringCapture when you get through all these.

[WebKit Commit Bot]

Comment on attachment 279665 [details]
Patch

Clearing flags on attachment: 279665

Committed r201341: <http://trac.webkit.org/changeset/201341>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

Reverted r201341 for reason:

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

Committed r201354: <http://trac.webkit.org/changeset/201354>

[Ryan Haddad]

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

[Chris Dumez]

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

[Chris Dumez]

(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 :/

[Brady Eidson]

(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

[Chris Dumez]

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

[Brady Eidson]

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

[Brady Eidson]

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

[Chris Dumez]

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

[Chris Dumez]

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

[Chris Dumez]

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.

[Brady Eidson]

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

[Chris Dumez]

Created attachment 279807 [details]
Patch

[Chris Dumez]

Now moving std::functions around to make sure we do not copy the captured strings after we've created the isolated copy.

[WebKit Commit Bot]

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.

[Chris Dumez]

Created attachment 279822 [details]
Patch

[WebKit Commit Bot]

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.

[Chris Dumez]

Created attachment 279823 [details]
Patch

[Chris Dumez]

Hopefully fixes the GTK and Windows build.

[WebKit Commit Bot]

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.

[Chris Dumez]

Created attachment 279827 [details]
Patch

[WebKit Commit Bot]

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.

[Build Bot]

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

[Build Bot]

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

[Chris Dumez]

Comment on attachment 279827 [details]
Patch

Looks like it still crashes on iOS. I'll investigate.

[Chris Dumez]

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.

[Chris Dumez]

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