[Webkit-unassigned] [Bug 38742] [chromium] Implement canEstablishDatabase call for workers.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon May 10 13:59:52 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=38742
--- Comment #19 from Andrew Wilson <atwilson at chromium.org> 2010-05-10 13:59:51 PST ---
(From update of attachment 55540)
> diff --git a/WebKit/chromium/ChangeLog b/WebKit/chromium/ChangeLog
> index 13e117d3f520ad993695e5793be6b66eb7f22b6a..8d0886515d3e350977640e8bec0a7132d6df23db 100644
> --- a/WebKit/chromium/ChangeLog
> +++ b/WebKit/chromium/ChangeLog
> @@ -1,3 +1,26 @@
> +2010-05-07 Jochen Eisinger <jochen at chromium.org>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + Implement canEstablishDatabase call for workers.
> + https://bugs.webkit.org/show_bug.cgi?id=38742
> +
> + * public/WebCommonWorkerClient.h:
> + * src/DatabaseObserver.cpp:
> + (WebCore::DatabaseObserver::canEstablishDatabase):
> + * src/WebWorkerBase.cpp:
> + (WebKit::WebWorkerBase::allowDatabase):
> + (WebKit::WebWorkerBase::allowDatabaseTask):
> + (WebKit::WebWorkerBase::AllowDatabaseMainThreadBridge::AllowDatabaseMainThreadBridge):
> + (WebKit::WebWorkerBase::AllowDatabaseMainThreadBridge::cancel):
> + (WebKit::WebWorkerBase::AllowDatabaseMainThreadBridge::done):
> + (WebKit::WebWorkerBase::AllowDatabaseMainThreadBridge::result):
> + (WebKit::WebWorkerBase::AllowDatabaseMainThreadBridge::signalCompleted):
> + (WebKit::WebWorkerBase::AllowDatabaseMainThreadBridge::didComplete):
> + * src/WebWorkerBase.h:
> + * src/WebWorkerClientImpl.h:
> + (WebKit::WebWorkerClientImpl::allowDatabase):
> +
> 2010-05-08 Jens Alfke <snej at chromium.org>
>
> Reviewed by Darin Fisher.
> diff --git a/WebKit/chromium/public/WebCommonWorkerClient.h b/WebKit/chromium/public/WebCommonWorkerClient.h
> index 13603cbc9315085430393a84c7f695e1e305d256..cea647123a52f5032850890dc84552c41644ff2d 100644
> --- a/WebKit/chromium/public/WebCommonWorkerClient.h
> +++ b/WebKit/chromium/public/WebCommonWorkerClient.h
> @@ -35,6 +35,7 @@ namespace WebKit {
>
> class WebApplicationCacheHost;
> class WebApplicationCacheHostClient;
> +class WebFrame;
> class WebNotificationPresenter;
> class WebString;
> class WebWorker;
> @@ -79,6 +80,9 @@ public:
> // Called on the main webkit thread in the worker process during initialization.
> virtual WebApplicationCacheHost* createApplicationCacheHost(WebApplicationCacheHostClient*) = 0;
>
> + // Called on the main webkit thread before opening a web database.
> + virtual bool allowDatabase(WebFrame*, const WebString& name, const WebString& displayName, unsigned long estimatedSize) = 0;
> +
> protected:
> ~WebCommonWorkerClient() { }
> };
> diff --git a/WebKit/chromium/src/DatabaseObserver.cpp b/WebKit/chromium/src/DatabaseObserver.cpp
> index 6a2e2a70ac08441adbb8e22e0e49355475507c6c..1c5117c0f57bdff1ca51dcc8774af6b3cd27912b 100644
> --- a/WebKit/chromium/src/DatabaseObserver.cpp
> +++ b/WebKit/chromium/src/DatabaseObserver.cpp
> @@ -50,12 +50,16 @@ namespace WebCore {
> bool DatabaseObserver::canEstablishDatabase(ScriptExecutionContext* scriptExecutionContext, const String& name, const String& displayName, unsigned long estimatedSize)
> {
> ASSERT(scriptExecutionContext->isContextThread());
> - // FIXME: add support for the case scriptExecutionContext()->isWorker() once workers implement web databases.
> - ASSERT(scriptExecutionContext->isDocument());
> + ASSERT(scriptExecutionContext->isDocument() || scriptExecutionContext->isWorkerContext());
> if (scriptExecutionContext->isDocument()) {
> Document* document = static_cast<Document*>(scriptExecutionContext);
> WebFrameImpl* webFrame = WebFrameImpl::fromFrame(document->frame());
> return webFrame->client()->allowDatabase(webFrame, name, displayName, estimatedSize);
> + } else {
> + WorkerContext* workerContext = static_cast<WorkerContext*>(scriptExecutionContext);
> + WorkerLoaderProxy* workerLoaderProxy = &workerContext->thread()->workerLoaderProxy();
> + WebWorkerBase* webWorker = static_cast<WebWorkerBase*>(workerLoaderProxy);
> + return webWorker->allowDatabase(0, name, displayName, estimatedSize);
> }
>
> return true;
> diff --git a/WebKit/chromium/src/WebWorkerBase.cpp b/WebKit/chromium/src/WebWorkerBase.cpp
> index 2c4b4f632341a2ee8b82ecb51ac172c8a50119b8..b844345ab160a2b6ee9ee1e053692d4b4a44466a 100644
> --- a/WebKit/chromium/src/WebWorkerBase.cpp
> +++ b/WebKit/chromium/src/WebWorkerBase.cpp
> @@ -31,6 +31,7 @@
> #include "config.h"
> #include "WebWorkerBase.h"
>
> +#include "DatabaseTask.h"
> #include "GenericWorkerTask.h"
> #include "MessagePortChannel.h"
> #include "PlatformMessagePortChannel.h"
> @@ -44,6 +45,7 @@
> #include "WebView.h"
> #include "WebWorkerClient.h"
>
> +#include "WorkerScriptController.h"
> #include "WorkerThread.h"
> #include <wtf/MainThread.h>
>
> @@ -53,6 +55,72 @@ namespace WebKit {
>
> #if ENABLE(WORKERS)
>
> +static const char allowDatabaseMode[] = "allowDatabaseMode";
> +
> +namespace {
> +
> +// This class is used to route the result of the WebWorkerBase::allowDatabase
> +// call back to the worker context.
> +class AllowDatabaseMainThreadBridge : public ThreadSafeShared<AllowDatabaseMainThreadBridge> {
> +public:
> + static PassRefPtr<AllowDatabaseMainThreadBridge> create(WebWorkerBase* worker, const WebCore::String& mode, WebCommonWorkerClient* commonClient, WebFrame* frame, const WebCore::String& name, const WebCore::String& displayName, unsigned long estimatedSize) { return adoptRef(new AllowDatabaseMainThreadBridge(worker, mode, commonClient, frame, name, displayName, estimatedSize)); }
> +
> + // These methods are invoked on the worker context.
> + void cancel()
> + {
> + MutexLocker locker(m_mutex);
> + m_worker = 0;
> + }
> +
> + bool done()
> + {
> + return m_completed;
> + }
> +
> + bool result()
> + {
> + return m_result;
> + }
> +
> + // This method is invoked on the main thread.
> + void signalCompleted(bool result)
> + {
> + MutexLocker locker(m_mutex);
> + if (m_worker)
> + m_worker->postTaskForModeToWorkerContext(createCallbackTask(&didComplete, this, result), m_mode);
> + }
> +
> +private:
> + AllowDatabaseMainThreadBridge(WebWorkerBase* worker, const WebCore::String& mode, WebCommonWorkerClient* commonClient, WebFrame* frame, const WebCore::String& name, const WebCore::String& displayName, unsigned long estimatedSize)
> + : m_completed(false)
> + , m_worker(worker)
> + , m_mode(mode)
> + {
> + worker->dispatchTaskToMainThread(createCallbackTask(&allowDatabaseTask, commonClient, frame, String(name), String(displayName), estimatedSize, this));
> + }
> +
> + static void allowDatabaseTask(WebCore::ScriptExecutionContext* context, WebCommonWorkerClient* commonClient, WebFrame* frame, const WebCore::String name, const WebCore::String displayName, unsigned long estimatedSize, PassRefPtr<AllowDatabaseMainThreadBridge> bridge)
> + {
> + if (!commonClient)
> + bridge->signalCompleted(false);
> + else
> + bridge->signalCompleted(commonClient->allowDatabase(frame, name, displayName, estimatedSize));
> + }
> +
> + static void didComplete(WebCore::ScriptExecutionContext* context, PassRefPtr<AllowDatabaseMainThreadBridge> bridge, bool result)
> + {
> + bridge->m_result = result;
> + bridge->m_completed = true;
> + }
> +
> + bool m_result;
> + bool m_completed;
> + Mutex m_mutex;
> + WebWorkerBase* m_worker;
> + WebCore::String m_mode;
> +};
> +}
> +
> // This function is called on the main thread to force to initialize some static
> // values used in WebKit before any worker thread is started. This is because in
> // our worker processs, we do not run any WebKit code in main thread and thus
> @@ -147,6 +215,30 @@ WebApplicationCacheHost* WebWorkerBase::createApplicationCacheHost(WebFrame*, We
> return 0;
> }
>
> +bool WebWorkerBase::allowDatabase(WebFrame*, const WebString& name, const WebString& displayName, unsigned long estimatedSize)
> +{
> + WorkerRunLoop& runLoop = m_workerThread->runLoop();
> + WorkerScriptController* controller = WorkerScriptController::controllerForContext();
> + ASSERT(controller);
> + WorkerContext* workerContext = controller->workerContext();
> +
> + // Create a unique mode just for this synchronous call.
> + String mode = allowDatabaseMode;
> + mode.append(String::number(runLoop.createUniqueId()));
> +
> + RefPtr<AllowDatabaseMainThreadBridge> bridge = AllowDatabaseMainThreadBridge::create(this, mode, commonClient(), m_webView->mainFrame(), String(name), String(displayName), estimatedSize);
> + MessageQueueWaitResult result = MessageQueueMessageReceived;
> + while (!bridge->done() && result != MessageQueueTerminated)
> + result = runLoop.runInMode(workerContext, mode);
> +
> + if (!bridge->done() && result == MessageQueueTerminated) {
> + bridge->cancel();
> + return false;
> + }
> +
> + return bridge->result();
> +}
> +
> // WorkerObjectProxy -----------------------------------------------------------
>
> void WebWorkerBase::postMessageToWorkerObject(PassRefPtr<SerializedScriptValue> message,
> diff --git a/WebKit/chromium/src/WebWorkerBase.h b/WebKit/chromium/src/WebWorkerBase.h
> index a470ee467d2f8629879d95af4c7593249597bd54..15e8013c9e5958b2765fca51e55729570c052a6d 100644
> --- a/WebKit/chromium/src/WebWorkerBase.h
> +++ b/WebKit/chromium/src/WebWorkerBase.h
> @@ -88,6 +88,9 @@ public:
> virtual void didCreateDataSource(WebFrame*, WebDataSource*);
> virtual WebApplicationCacheHost* createApplicationCacheHost(WebFrame*, WebApplicationCacheHostClient*);
>
> + // Controls whether access to Web Databases is allowed for this worker.
> + virtual bool allowDatabase(WebFrame*, const WebString& name, const WebString& displayName, unsigned long estimatedSize);
> +
> // Executes the given task on the main thread.
> static void dispatchTaskToMainThread(PassOwnPtr<WebCore::ScriptExecutionContext::Task>);
>
> diff --git a/WebKit/chromium/src/WebWorkerClientImpl.h b/WebKit/chromium/src/WebWorkerClientImpl.h
> index 907499a266a6762cfbf29acd93d10574a60f9eeb..afa9b9b337695270e72737dd71ce3995ff59e718 100644
> --- a/WebKit/chromium/src/WebWorkerClientImpl.h
> +++ b/WebKit/chromium/src/WebWorkerClientImpl.h
> @@ -95,6 +95,7 @@ public:
> return 0;
> }
> virtual WebApplicationCacheHost* createApplicationCacheHost(WebApplicationCacheHostClient*) { return 0; }
> + virtual bool allowDatabase(WebFrame*, const WebString& name, const WebString& displayName, unsigned long estimatedSize) { return true; }
>
> private:
> virtual ~WebWorkerClientImpl();
> diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
> index 4e48b364a4c12269074dafaf26b4b4d30312952f..4b57ab4260d7de0290da772e93d236f1d9724644 100644
> --- a/WebKitTools/ChangeLog
> +++ b/WebKitTools/ChangeLog
> @@ -1,3 +1,15 @@
> +2010-05-07 Jochen Eisinger <jochen at chromium.org>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + Add allowDatabase method to TestWebWorker.
> + https://bugs.webkit.org/show_bug.cgi?id=38742
> +
> + Need a short description and bug URL (OOPS!)
> +
> + * DumpRenderTree/chromium/TestWebWorker.h:
> + (TestWebWorker::allowDatabase):
> +
> 2010-05-09 Daniel Bates <dbates at rim.com>
>
> Reviewed by Chris Jerdonek.
> diff --git a/WebKitTools/DumpRenderTree/chromium/TestWebWorker.h b/WebKitTools/DumpRenderTree/chromium/TestWebWorker.h
> index f28cc2055d25754478f0622f3b71673dffb30102..94708040d50e412390c55ab8956639be8add5e09 100644
> --- a/WebKitTools/DumpRenderTree/chromium/TestWebWorker.h
> +++ b/WebKitTools/DumpRenderTree/chromium/TestWebWorker.h
> @@ -80,6 +80,7 @@ public:
> virtual WebKit::WebWorker* createWorker(WebKit::WebWorkerClient*) { return 0; }
> virtual WebKit::WebNotificationPresenter* notificationPresenter() { return 0; }
> virtual WebKit::WebApplicationCacheHost* createApplicationCacheHost(WebKit::WebApplicationCacheHostClient*) { return 0; }
> + virtual bool allowDatabase(WebKit::WebFrame*, const WebKit::WebString&, const WebKit::WebString&, unsigned long) { return true; }
>
> private:
> ~TestWebWorker() {}
WebKit/chromium/src/WebWorkerBase.cpp:221
+ WorkerScriptController* controller = WorkerScriptController::controllerForContext();
We should be checking for controller == 0 here (allowDatabase() called while worker is shutting down) and returning false.
WebKit/chromium/src/WebWorkerClientImpl.h:98
+ virtual bool allowDatabase(WebFrame*, const WebString& name, const WebString& displayName, unsigned long estimatedSize) { return true; }
This should never be called on the renderer side, so I think this should be ASSERT_NOT_REACHED()
WebKit/chromium/src/WebWorkerBase.cpp:66
+ static PassRefPtr<AllowDatabaseMainThreadBridge> create(WebWorkerBase* worker, const WebCore::String& mode, WebCommonWorkerClient* commonClient, WebFrame* frame, const WebCore::String& name, const WebCore::String& displayName, unsigned long estimatedSize) { return adoptRef(new AllowDatabaseMainThreadBridge(worker, mode, commonClient, frame, name, displayName, estimatedSize)); }
This seems really hard to read as one big long line. Can we break this up? I'd suggest putting the function body on its own lines at least:
static PassRefPtr<...>create(.....)
{
return adoptRef(...);
}
WebKit/chromium/src/WebWorkerBase.cpp:234
+ if (!bridge->done() && result == MessageQueueTerminated) {
Just curious - do we need the check for bridge->done() here and above (and any of the m_completed logic in the Bridge class)? I'm not seeing how runInMode() can return without the appropriate task having been fired (or the queue terminated).
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list