Bug 158187 – Make ScriptExecutionContext::Task work in terms of wtf::NoncopyableFunction instead of std::function

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

[Brady Eidson]

Make ScriptExecutionContext::Task work in terms of wtf::NoncopyableFunction instead of std::function

[Brady Eidson]

Created attachment 280054 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/dom/ScriptExecutionContext.h:136:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/dom/ScriptExecutionContext.h:143:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/dom/ScriptExecutionContext.h:149:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/dom/ScriptExecutionContext.h:166:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 4 in 6 files


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

[Brady Eidson]

Created attachment 280055 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/dom/ScriptExecutionContext.h:136:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/dom/ScriptExecutionContext.h:143:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/dom/ScriptExecutionContext.h:149:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/dom/ScriptExecutionContext.h:166:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 4 in 7 files


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

[Chris Dumez]

Comment on attachment 280055 [details]
Patch

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

> Source/WebCore/dom/ScriptExecutionContext.h:143
> +        Task(std::function<void ()> task)

How about this one? Shouldn't this be a NoncopyableFunction<void ()>&& task?

Note that would require updating TaskDispatcher::postTask() as well probably, in GenericTaskQueue.h.

[Chris Dumez]

Comment on attachment 280055 [details]
Patch

Are you planning to clean up the lambda captures at the postTask() call sites in a separate patch? Using NoncopyableFunction in Task, should allow a decent amount of clean up (including getting rid of some StringCapture uses).

[Brady Eidson]

(In reply to comment #5)
> Comment on attachment 280055 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=280055&action=review
> 
> > Source/WebCore/dom/ScriptExecutionContext.h:143
> > +        Task(std::function<void ()> task)
> 
> How about this one? Shouldn't this be a NoncopyableFunction<void ()>&& task?
> 
> Note that would require updating TaskDispatcher::postTask() as well
> probably, in GenericTaskQueue.h.

Right - I plan on moving iteratively on this so each patch is more targeted..  There should be 3 or 4 steps - This is just the first.

Making that change here blows up the patch a lot.(In reply to comment #6)

> Are you planning to clean up the lambda captures at the postTask() call
> sites in a separate patch? Using NoncopyableFunction in Task, should allow a
> decent amount of clean up (including getting rid of some StringCapture uses).

Yes - That's in progress locally. Again, plan on moving in smaller steps :P

[Chris Dumez]

Comment on attachment 280055 [details]
Patch

R=me, definitely a step in the right direction.

[WebKit Commit Bot]

Comment on attachment 280055 [details]
Patch

Clearing flags on attachment: 280055

Committed r201496: <http://trac.webkit.org/changeset/201496>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.