[webkit-reviews] review denied: [Bug 36795] [chromium] Problems passing allowDatabase() off to the chrome layer : [Attachment 52024] Patch

[bugzilla-daemon at webkit.org]

Jeremy Orlow <jorlow at chromium.org> has denied jochen at chromium.org's request for
review:
Bug 36795: [chromium] Problems passing allowDatabase() off to the chrome layer
https://bugs.webkit.org/show_bug.cgi?id=36795

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

------- Additional Comments from Jeremy Orlow <jorlow at chromium.org>
> diff --git a/WebKit/chromium/ChangeLog b/WebKit/chromium/ChangeLog
> index 551c957..bdec06d 100644
> --- a/WebKit/chromium/ChangeLog
> +++ b/WebKit/chromium/ChangeLog
> @@ -1,3 +1,17 @@
> +2010-03-30  Jochen Eisinger	<jochen at chromium.org>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Remove dysfunctional implementation of canEstablishDatabase for
> +	   Workers. I postpone this implementation until Workers can actually
> +	   access Web Databases.
> +
> +	   https://bugs.webkit.org/show_bug.cgi?id=36795
> +
> +	   * src/DatabaseObserver.cpp:
> +	   (WebCore::DatabaseObserver::canEstablishDatabase):
> +	   * src/WebWorkerBase.h:
> +
>  2010-03-29  Rafael Weinstein  <rafaelw at chromium.org>
>  
>	   Reviewed by Adam Barth.
> diff --git a/WebKit/chromium/src/DatabaseObserver.cpp
b/WebKit/chromium/src/DatabaseObserver.cpp
> index e89bd5d..9ff5c8a 100644
> --- a/WebKit/chromium/src/DatabaseObserver.cpp
> +++ b/WebKit/chromium/src/DatabaseObserver.cpp
> @@ -50,17 +50,17 @@ namespace WebCore {
>  bool DatabaseObserver::canEstablishDatabase(ScriptExecutionContext*
scriptExecutionContext, const String& name, const String& displayName, unsigned
long estimatedSize)
>  {
>      ASSERT(scriptExecutionContext->isContextThread());
> -    ASSERT(scriptExecutionContext->isDocument() ||
scriptExecutionContext->isWorkerContext());
> +    // TODO(jochen): add support for the case

FIXME: not TODO():

> +    // scriptExecutionContext()->isWorker() once workers implement web
> +    // databases.

No 80 line wrapping.

> +    ASSERT(scriptExecutionContext->isDocument());
>      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* worker =
static_cast<WorkerContext*>(scriptExecutionContext);
> -	   WorkerLoaderProxy* workerLoaderProxy =
&worker->thread()->workerLoaderProxy();
> -	   WebWorkerImpl* webWorker =
reinterpret_cast<WebWorkerImpl*>(workerLoaderProxy);
> -	   return
webWorker->allowDatabase(WebSecurityOrigin(scriptExecutionContext->securityOrig
in()), name, displayName, estimatedSize);
>      }
> +
> +    return true;
>  }
>  
>  void DatabaseObserver::databaseOpened(Database* database)
> diff --git a/WebKit/chromium/src/WebWorkerBase.h
b/WebKit/chromium/src/WebWorkerBase.h
> index c50d4a3..d7e51fa 100644
> --- a/WebKit/chromium/src/WebWorkerBase.h
> +++ b/WebKit/chromium/src/WebWorkerBase.h
> @@ -80,9 +80,6 @@ public:
>      virtual void postTaskForModeToWorkerContext(
>	   PassOwnPtr<WebCore::ScriptExecutionContext::Task>, const
WebCore::String& mode);
>  
> -    // Controls whether access to Web Databases is allowed for this worker.
> -    virtual bool allowDatabase(const WebSecurityOrigin&, const WebString&,
const WebString&, unsigned long) { return true; }
> -
>      // Executes the given task on the main thread.
>      static void
dispatchTaskToMainThread(PassOwnPtr<WebCore::ScriptExecutionContext::Task>);
>