Introduce WebWorkerPermissionClientProxy to deprecate WorkerAllowMainThreadBridge

Currently we relay all allow{Database,FileSystem,IndexedDB} requests for 
workers from worker thread to renderer thread, but this introduces 
a lot of complexity and leak/u-a-f problems. This change tries to 
introduce WebWorkerPermissionClientProxy interface, which gets created 
on the renderer thread, passed on to the worker thread and then issues 
sync IPC (on the worker thread) in the embedder code.

This will allow us to remove a lot of code (in the next iteration):
- To get rid of complex (and insecure) WorkerAllowMainThreadBridge code
- To remove more WorkerRunLoop.runInMode usage
- To simplify worker client inheritance chain

Chromium-side change: https://codereview.chromium.org/46583005/

BUG=none

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161017

[kinuko, 2013-10-30 00:33:36]

chromium-side patch looks promising, so I'd like to have blink-side change
reviewed too. PTL

[abarth-chromium, 2013-10-30 06:41:08]

> - To remove more WorkerRunLoop.runInMode usage 

Yes!  Please!  runInMode is sad times.

[abarth-chromium, 2013-10-30 06:49:03]

I'm very excited about this change.  This removes a blocking issue for moving
workers to Chromium threads.  LGTM, but I'm not an expert on this code.  You
probably want michaeln to review this CL as well.

https://codereview.chromium.org/50773002/diff/1/Source/web/DatabaseObserver.cpp
File Source/web/DatabaseObserver.cpp (right):

https://codereview.chromium.org/50773002/diff/1/Source/web/DatabaseObserver.c...
Source/web/DatabaseObserver.cpp:133: bool
DatabaseObserver::canEstablishDatabase(ExecutionContext* executionContext, const
String& name, const String& displayName, unsigned long estimatedSize)
It's really lame that we have this WebCore symbol defined in the Source/web
directory.  This prevents us from splitting Source/web and Source/core into
separate DLLs.  We can leave this for now, but in a future refactoring, we might
want to make the permission client be defined in Source/core with virtual
functions and then have an "impl" object in Source/web, similar to how we handle
other "client" objects like WebFrameClient.

You don't need to worry about these issues in this CL, but I wanted to mention
it.

https://codereview.chromium.org/50773002/diff/1/Source/web/WebSharedWorkerImp...
File Source/web/WebSharedWorkerImpl.cpp (right):

https://codereview.chromium.org/50773002/diff/1/Source/web/WebSharedWorkerImp...
Source/web/WebSharedWorkerImpl.cpp:291:
providePermissionClientToWorker(workerClients.get(),
adoptPtr(m_client->createWorkerPermissionClientProxy(webSecurityOrigin)));
Does the WorkerPermissionClientProxy retain the SecurityOrigin object?  It's not
thread-safe....

[kinuko, 2013-10-30 07:05:51]

Thx!

https://codereview.chromium.org/50773002/diff/1/Source/web/DatabaseObserver.cpp
File Source/web/DatabaseObserver.cpp (right):

https://codereview.chromium.org/50773002/diff/1/Source/web/DatabaseObserver.c...
Source/web/DatabaseObserver.cpp:133: bool
DatabaseObserver::canEstablishDatabase(ExecutionContext* executionContext, const
String& name, const String& displayName, unsigned long estimatedSize)
On 2013/10/30 06:49:03, abarth wrote:
> It's really lame that we have this WebCore symbol defined in the Source/web
> directory.  This prevents us from splitting Source/web and Source/core into
> separate DLLs.  We can leave this for now, but in a future refactoring, we
might
> want to make the permission client be defined in Source/core with virtual
> functions and then have an "impl" object in Source/web, similar to how we
handle
> other "client" objects like WebFrameClient.
> 
> You don't need to worry about these issues in this CL, but I wanted to mention
> it.

Totally agreed about this class shouldn't be implemented like this.

https://codereview.chromium.org/50773002/diff/1/Source/web/WebSharedWorkerImp...
File Source/web/WebSharedWorkerImpl.cpp (right):

https://codereview.chromium.org/50773002/diff/1/Source/web/WebSharedWorkerImp...
Source/web/WebSharedWorkerImpl.cpp:291:
providePermissionClientToWorker(workerClients.get(),
adoptPtr(m_client->createWorkerPermissionClientProxy(webSecurityOrigin)));
On 2013/10/30 06:49:03, abarth wrote:
> Does the WorkerPermissionClientProxy retain the SecurityOrigin object?  It's
not
> thread-safe....

No, it's only used for initialization.  I can add a comment to note that the
embedder should not retain it.

[michaeln, 2013-10-30 19:47:04]

lgtm2

also a big fan of getting rid of the nested message pumps !!!

https://codereview.chromium.org/50773002/diff/1/Source/web/WorkerPermissionCl...
File Source/web/WorkerPermissionClient.h (right):

https://codereview.chromium.org/50773002/diff/1/Source/web/WorkerPermissionCl...
Source/web/WorkerPermissionClient.h:57: WebWorkerPermissionClientProxy* proxy()
{ return m_proxy.get(); }
does this need to be public?

[kinuko, 2013-10-31 01:48:08]

Thx!

https://codereview.chromium.org/50773002/diff/1/Source/web/WorkerPermissionCl...
File Source/web/WorkerPermissionClient.h (right):

https://codereview.chromium.org/50773002/diff/1/Source/web/WorkerPermissionCl...
Source/web/WorkerPermissionClient.h:57: WebWorkerPermissionClientProxy* proxy()
{ return m_proxy.get(); }
On 2013/10/30 19:47:04, michaeln wrote:
> does this need to be public?

This is used just to see if the embedder has proxy implementation until we
completely switch the code (i.e. if proxy is null we use old code for now), and
to be removed in the next blink patch. I'll add a FIXME comment.

[commit-bot: I haz the power, 2013-10-31 01:58:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinuko@chromium.org/50773002/140001

[commit-bot: I haz the power, 2013-10-31 04:22:17]

Change committed as 161017