[webkit-reviews] review granted: [Bug 51818] GeolocationPositionCache should not use ScriptExecutionContext::Task : [Attachment 77826] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Steve Block <steveblock at google.com>'s
request for review:
Bug 51818: GeolocationPositionCache should not use ScriptExecutionContext::Task
https://bugs.webkit.org/show_bug.cgi?id=51818

Attachment 77826: Patch
https://bugs.webkit.org/attachment.cgi?id=77826&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=77826&action=review

It seems a little strange to make a function object and name it Task. But OK.

> Source/JavaScriptCore/wtf/Task.h:40
> +    virtual ~Task() {}

We put spaces between braces in cases like that.

> WebCore/fileapi/FileThread.h:58
> -    class Task : public Noncopyable {
> +    class FileTask : public Task {

Does this really need to be renamed? You could just say this:

    class Task : public WTF::Task

That would make the patch smaller, but I guess could be a bit confusing. With
the name FileTask it seems it could be made a non-member.

> WebCore/page/GeolocationPositionCache.cpp:48
> +    CacheTask(GeolocationPositionCache* cache) : m_cache(cache) {}

We put spaces in those {}