[webkit-reviews] review denied: [Bug 65997] [Chromium] Decouple implementation of allowFileSystem, openFileSystem and allowDatabase from WebWorkerBase : [Attachment 103523] CR feedback addressed

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 10 13:23:14 PDT 2011


Jian Li <jianli at chromium.org> has denied Dmitry Lomov <dslomov at google.com>'s
request for review:
Bug 65997: [Chromium] Decouple implementation of allowFileSystem,
openFileSystem and allowDatabase from WebWorkerBase
https://bugs.webkit.org/show_bug.cgi?id=65997

Attachment 103523: CR feedback addressed
https://bugs.webkit.org/attachment.cgi?id=103523&action=review

------- Additional Comments from Jian Li <jianli at chromium.org>
Almost there. Just some more minor comments.

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


> Source/WebKit/chromium/ChangeLog:8
> +	   parametrizing them with relevant data from WebWorker.

typo: parameterizing

> Source/WebKit/chromium/ChangeLog:10
> +	   * src/DatabaseObserver.cpp:Move allowDatabase from WebWorkerBase and
update the caller.

Please add a whitespace after ':'.

> Source/WebKit/chromium/ChangeLog:20
> +	   * src/LocalFileSystemChromium.cpp:Move allowFileSystem and
openFileSystem from WebWorkerBase and update the caller.

ditto.

> Source/WebKit/chromium/ChangeLog:30
> +	   * src/WorkerFileSystemCallbacksBridge.h:

Please also add more comment here, like:
  Change WorkerFileSystemCallbacksBridge constructor to take WorkerLoaderProxy
argument.

> Source/WebKit/chromium/src/LocalFileSystemChromium.cpp:122
> +	   if (!commonClient)

Since you're already in this code, why not just making it better?

> Source/WebKit/chromium/src/LocalFileSystemChromium.cpp:184
> +

Please remove an extra blank line.


More information about the webkit-reviews mailing list