[webkit-reviews] review denied: [Bug 90310] IndexedDB: Enable IDBFactory.deleteDatabase() and webkitGetDatabaseNames() in Workers : [Attachment 150257] Patch

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Joshua Bell <jsbell at chromium.org>'s
request for review:
Bug 90310: IndexedDB: Enable IDBFactory.deleteDatabase() and
webkitGetDatabaseNames() in Workers
https://bugs.webkit.org/show_bug.cgi?id=90310

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

------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=150257&action=review


I'm fine with others completely the review (especially since I will be out for
a while but I wanted to push this code change to reduce duplicate code).

> Source/WebCore/Modules/indexeddb/IDBFactory.cpp:134
> +#endif

I think a lot of this code would be simpler if the difference between the
worker and the document were handled down in the backend as opposed to being
exposed here.


Your call would look like this:
  RefPtr<IDBVersionChangeRequest> request =
IDBVersionChangeRequest::create(context, IDBAny::createNull(), "");
  m_backend->deleteDatabaseFromWorker(name, request, context, String());

with no ifdef workers.

Ditto for getDatabaseNamesFromWorker

> Source/WebKit/chromium/src/IDBFactoryBackendProxy.cpp:184
> +    WebSecurityOrigin origin(prpOrigin);

Continuing the idea of only having getDatabaseNames (and not
getDatabaseNamesFromWorker)


void IDBFactoryBackendProxy::getDatabaseNames(PassRefPtr<IDBCallbacks>
callbacks, ScriptExecutionContext* context, const String& dataDir)
{
    WebSecurityOrigin origin(context->securityOrigin());
    if (!allowIndexedDB(context, "Database Listing", origin, callbacks))
	return;
  
    WebFrameImpl* webFrame = getWebFrame(context);
    m_webIDBFactory->getDatabaseNames(new WebIDBCallbacksImpl(callbacks),
origin, webFrame, dataDir);
}


Then make allowIndexedDB and getWebFrame of the right thing for WorkerContext
vs Document. All your ifdef's move to that one piece of code and a lot of your
duplicate code collapses and gets removed.