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

[bugzilla-daemon at webkit.org]

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


Dmitry Titov <dimich at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #55815|review?                     |review-
               Flag|                            |




--- Comment #28 from Dmitry Titov <dimich at chromium.org>  2010-05-13 11:58:28 PST ---
(From update of attachment 55815)
Looks great, would r+ with "change on landing" but since cq is needed, we need a perfect patch.

One small thing:

WebKit/chromium/src/WebWorkerBase.cpp:217
 +      // The controller might be 0 when the worker context is in the process of shutting down.
I see the discussion in the comments about this check. It is not really needed. There is another method which returns a V8 proxy (WorkerScriptController::proxy()) which indeed returns 0 when the termination of JS in the worker is requested. The WorkerScriptController::controllerForContext() always returns a non-null controller in worker context.
See http://trac.webkit.org/changeset/56580. That change split the old function that could return 0 if either the context is not WorkerContext or the termination was requested, to be able to make these checks separately. This is why I think there is no real case when we need an "if(!controller)" here and rather could just have an assert, or even better no check at all since the very next statement is going to crash reliably anyways.

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