[Webkit-unassigned] [Bug 38742] [chromium] Implement canEstablishDatabase call for workers.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=38742





--- Comment #21 from jochen at chromium.org  2010-05-11 00:53:14 PST ---
(In reply to comment #20)
> Sorry, reviewer FAIL on that previous comment :(
> 
> Anyhow, just a quick drive-by:
> 
> 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.

Dmitry stated in Comment #7 that this was not possible
> 
> 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()


e.g. createApplicationCacheHost should not be called either, I'd rather add ASSERT_NOT_REACHED to all methods that should not be invoked (but I cannot judge which these are), or none.

wdyt?

> 
> 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(...);
> }

done

> 
> 
> 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).

done

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.