[webkit-reviews] review denied: [Bug 38742] [chromium] Implement canEstablishDatabase call for workers. : [Attachment 55367] Patch

[bugzilla-daemon at webkit.org]

Dmitry Titov <dimich at chromium.org> has denied jochen at chromium.org's request for
review:
Bug 38742: [chromium] Implement canEstablishDatabase call for workers.
https://bugs.webkit.org/show_bug.cgi?id=38742

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

------- Additional Comments from Dmitry Titov <dimich at chromium.org>
Looks reasonable, couple of notes:

WebKit/chromium/src/WebWorkerBase.cpp:157
 +	    return false;
It is enough to have simple ASSERT(controller) here, since this method is not
supposed to be called for non-worker contexts and there is no meaningful
scenario when we actually want to return false from here.

WebKit/chromium/src/WebWorkerBase.h:150
 +	class AllowDatabaseMainThreadBridge;
This class is a detail of implementation and should be declared/defined in cpp
file.

WebKit/chromium/src/WebWorkerBase.h:152
 +	static void allowDatabaseTask(
This can be just a static method in cpp file, it does not need to be in .h

WebKit/chromium/src/WebWorkerBase.cpp:198
 +  void WebWorkerBase::AllowDatabaseMainThreadBridge::cancel()
Where is this method used?

WebKit/chromium/src/WebWorkerBase.cpp:206
 +	MutexLocker lock(m_synchronousMutex);
Why this mutex is necessary? It is usually not needed to protect bool values
like that.