Bug 150770 – [GTK] Use RunLoop in WorkQueue implementation

• Component: Web Template Framework
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

Instead of using GMainLoop directly. RunLoop already abstracts the GMainLoop details and uses persistent sources making it more efficient.

[Carlos Garcia Campos]

Created attachment 264509 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WTF/wtf/glib/WorkQueueGLib.cpp:89:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/glib/WorkQueueGLib.cpp:95:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/glib/WorkQueueGLib.cpp:112:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 3 in 6 files


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

[Darin Adler]

Comment on attachment 264509 [details]
Patch

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

> Source/WTF/wtf/WorkQueue.h:43
> +#include <wtf/RunLoop.h>

Do we actually need this header to be included? Can we just forward-declare RunLoop? This header doesn’t actually depend on the content of RunLoop, just that it’s a pointer to some class.

> Source/WTF/wtf/WorkQueue.h:113
> +    RunLoop* m_runLoop;

Would be nice if we could find a way to make this a reference rather than a pointer.

[Carlos Garcia Campos]

(In reply to comment #3)
> Comment on attachment 264509 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=264509&action=review
> 
> > Source/WTF/wtf/WorkQueue.h:43
> > +#include <wtf/RunLoop.h>
> 
> Do we actually need this header to be included? Can we just forward-declare
> RunLoop? This header doesn’t actually depend on the content of RunLoop, just
> that it’s a pointer to some class.

Not only the pointer, we also have:

RunLoop& runLoop() const { return *m_runLoop; }

> > Source/WTF/wtf/WorkQueue.h:113
> > +    RunLoop* m_runLoop;
> 
> Would be nice if we could find a way to make this a reference rather than a
> pointer.

If we make it a reference we would need to initialize it on construction, but the RunLoop has to be created by the worker thread. We could use Optional, but I'm not sure it would make the code better in this particular case.

[Carlos Garcia Campos]

Committed r191878: <http://trac.webkit.org/changeset/191878>