Initializing an appcache host in a worker process.

Add some more IPC plumbing and scaffolding to support having appcache work in workers. Everything is still stubbed out at runtime (runtime feature is still disabled in the worker process, and the values in the IPC messages are all zero'd out).

* Widen the CreateWorker IPC message sent from the browser to the worker process to contain additional data needed to initialize an appcache for that worker.

* Add a new worker specific WorkerWebApplicationCacheHostImpl class and instantiate one with the initialization data received in the IPC.

* Give the WorkerThread an AppCacheDispatcher.

* Propagate the cmd-line argument to disable the appcache to the worker process.

* Fixup DEPs to show that chrome/workers depends on webkit/appcache

BUG=39368
TEST=thinking about what tests to put together for this CL

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=46765

[michaeln, 2010-05-07 01:33:49]

Please take a look

[Andrew T Wilson (Slow), 2010-05-07 22:49:17]

This looks good from a fit-in-with-workers pov, although I don't completely
understand the appcache framework so I had some questions about a few parts.

Anyhow, LGTM, and you can enlighten me offline if you like :)

http://codereview.chromium.org/1719007/diff/48002/78014
File chrome/worker/worker_thread.cc (right):

http://codereview.chromium.org/1719007/diff/48002/78014#newcode38
chrome/worker/worker_thread.cc:38:
channel()->AddFilter(db_message_filter_.get());
Is it OK to move these lines up before we enable/disable the database? I think
it' should be fine because I think the WebRuntimeFeatures stuff just hides APIs
from javascript, but it's something to be careful with.

http://codereview.chromium.org/1719007/diff/48002/78015
File chrome/worker/worker_thread.h (right):

http://codereview.chromium.org/1719007/diff/48002/78015#newcode43
chrome/worker/worker_thread.h:43: scoped_ptr<AppCacheDispatcher>
appcache_dispatcher_;
Since this is the WorkerThread object, it seems like appcache_dispatcher_ can't
outlive us. Is there a reason to allocate it on the heap?

http://codereview.chromium.org/1719007/diff/48002/78016
File chrome/worker/worker_webapplicationcachehost_impl.cc (right):

http://codereview.chromium.org/1719007/diff/48002/78016#newcode26
chrome/worker/worker_webapplicationcachehost_impl.cc:26: WebKit::WebURLRequest&)
{
So, not certain I completely understand the future data flow - are we filling in
code for these callbacks in the future, or will they always remain empty (don't
care - just filling out the interface0/ If the latter, I'd probably biff the
explicit return statements.

http://codereview.chromium.org/1719007/diff/48002/78017
File chrome/worker/worker_webapplicationcachehost_impl.h (right):

http://codereview.chromium.org/1719007/diff/48002/78017#newcode25
chrome/worker/worker_webapplicationcachehost_impl.h:25:
parent_appcache_host_id(host_id), main_resource_appcache_id(cache_id) {
Is it useful to add stuff like DCHECK(is_shared || main_resource_appcache_id ==
0), etc here to validate that we pass valid values in for dedicated vs shared
workers?

[michaeln, 2010-05-07 23:14:31]

http://codereview.chromium.org/1719007/diff/48002/78014
File chrome/worker/worker_thread.cc (right):

http://codereview.chromium.org/1719007/diff/48002/78014#newcode38
chrome/worker/worker_thread.cc:38:
channel()->AddFilter(db_message_filter_.get());
On 2010/05/07 22:49:17, Andrew T Wilson wrote:
> Is it OK to move these lines up before we enable/disable the database? I think
> it' should be fine because I think the WebRuntimeFeatures stuff just hides
APIs
> from javascript, but it's something to be careful with.
> 

Should be ok. The object ctors and functions being called don't check the state
of the runtime enabled flag.

http://codereview.chromium.org/1719007/diff/48002/78015
File chrome/worker/worker_thread.h (right):

http://codereview.chromium.org/1719007/diff/48002/78015#newcode43
chrome/worker/worker_thread.h:43: scoped_ptr<AppCacheDispatcher>
appcache_dispatcher_;
On 2010/05/07 22:49:17, Andrew T Wilson wrote:
> Since this is the WorkerThread object, it seems like appcache_dispatcher_
can't
> outlive us. Is there a reason to allocate it on the heap?

This could be alloc'd inline. I have it as a separate heap alloc simply because
that is how the RenderThread, which also uses this class, is put together. It
does help us avoid an include in this .h file.

http://codereview.chromium.org/1719007/diff/48002/78016
File chrome/worker/worker_webapplicationcachehost_impl.cc (right):

http://codereview.chromium.org/1719007/diff/48002/78016#newcode26
chrome/worker/worker_webapplicationcachehost_impl.cc:26: WebKit::WebURLRequest&)
{
On 2010/05/07 22:49:17, Andrew T Wilson wrote:
> So, not certain I completely understand the future data flow - are we filling
in
> code for these callbacks in the future, or will they always remain empty
(don't
> care - just filling out the interface0/ If the latter, I'd probably biff the
> explicit return statements.

These will always be empty methods, I'll just put the empty method bodies in the
.h file instead.

http://codereview.chromium.org/1719007/diff/48002/78017
File chrome/worker/worker_webapplicationcachehost_impl.h (right):

http://codereview.chromium.org/1719007/diff/48002/78017#newcode25
chrome/worker/worker_webapplicationcachehost_impl.h:25:
parent_appcache_host_id(host_id), main_resource_appcache_id(cache_id) {
On 2010/05/07 22:49:17, Andrew T Wilson wrote:
> Is it useful to add stuff like DCHECK(is_shared || main_resource_appcache_id
==
> 0), etc here to validate that we pass valid values in for dedicated vs shared
> workers?

I think DCHECKs like that can be useful for self-documenting code purposes. I
don't have anything like here for two reasons.
1. To avoid including logging.h in this .h file.
2. The comments adjacent the data members about 10 lines up from here do that
documenting already.