Plumbs URLRequestContext and CacheBlobStorageContext down to cache.

Plumbs URLRequestContext and CacheBlobStorageContext down to cache.

This will be needed for request/response blobs in the ServiceWorkerCache.

BUG=392621
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289326

[jkarlin, 2014-08-11 18:27:39]

michaeln@chromium.org: Please review changes in everything

falken@chromium.org: Please review changes in everything

gavinp@chromium.org: FYI

[michaeln, 2014-08-12 03:48:53]

i have some suggestions about ptr types and such...

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_cache.h (right):

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_cache.h:33:
ChromeBlobStorageContext* blob_context);
Since these classes are used on the IO thread, it might make more sense to hang
on to a WeakPtr<webkit_blob::BlobStorageContext> instead of a raw ptr to the
refcounted 'ChromeXxx' class. So you can test for whether its still valid w/o
influencing its lifetime.

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_cache.h:59:
net::URLRequestContext* request_context_;
always makes me queasy to see the rawptrs, but what's a programmer to do with
this class other than this?

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_cache.h:60:
ChromeBlobStorageContext* blob_storage_context_;
ditto base::WeakPtr<webkit_blob::BlobStorageContext>

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_cache_storage.h (right):

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_cache_storage.h:52:
ChromeBlobStorageContext* blob_context);
ditto base::WeakPtr<webkit_blob::BlobStorageContext>

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_cache_storage_manager.h
(right):

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_cache_storage_manager.h:76:
ChromeBlobStorageContext* blob_storage_context() const {
until these getters need to be public, can you keep them as private. these are
definitely needed by the impl of this cache backend stuff but i think it should
be more of an imple detail than something with public accessors

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_cache_storage_manager.h:103:
ChromeBlobStorageContext* blob_context_;
ditto base::WeakPtr<webkit_blob::BlobStorageContext>

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_context_core.cc (right):

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_context_core.cc:105:
wrapper->blob_storage_context())),
which in turn will pass them to the along here...

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_context_wrapper.cc (right):

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_context_wrapper.cc:183: this));
in place of public getters can we unwrap the webkit::BlobStorageContext and
net::URLRequestContext here and pass ownership of them to the ContextCore ctor
and then clear the refcounted members of the ContextWrapper

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_context_wrapper.h (right):

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_context_wrapper.h:85:
ChromeBlobStorageContext* blob_storage_context() {
this swsystem needs to use these things, but it doesn't need to vend them to
others, i'd like to not have these accessors in the public section.

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_context_wrapper.h:113:
net::URLRequestContextGetter* request_context_getter_;
scoped_refptr<T> for both of these new data refcounted members? We can clear the
values in InitInternal after constructing a Core with them

[jkarlin, 2014-08-12 18:45:18]

I was accessing the wrong URLRequestContext, I have to get it *after* storage
creation.  Copying IndexedDB's approach and grabbing the URLRequestContext and
blob storage in the dispatcher host.

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_cache_storage_manager.h
(right):

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_cache_storage_manager.h:76:
ChromeBlobStorageContext* blob_storage_context() const {
On 2014/08/12 03:48:52, michaeln wrote:
> until these getters need to be public, can you keep them as private. these are
> definitely needed by the impl of this cache backend stuff but i think it
should
> be more of an imple detail than something with public accessors

Done.

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_cache_storage_manager.h:103:
ChromeBlobStorageContext* blob_context_;
On 2014/08/12 03:48:52, michaeln wrote:
> ditto base::WeakPtr<webkit_blob::BlobStorageContext>

Done.

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_context_wrapper.cc (right):

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_context_wrapper.cc:183: this));
On 2014/08/12 03:48:52, michaeln wrote:
> in place of public getters can we unwrap the webkit::BlobStorageContext and
> net::URLRequestContext here and pass ownership of them to the ContextCore ctor
> and then clear the refcounted members of the ContextWrapper

No longer applicable, see update.  Though there is now a public setter but I
don't see a way around that.

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_context_wrapper.h (right):

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_context_wrapper.h:85:
ChromeBlobStorageContext* blob_storage_context() {
On 2014/08/12 03:48:52, michaeln wrote:
> this swsystem needs to use these things, but it doesn't need to vend them to
> others, i'd like to not have these accessors in the public section.

No longer applicable, see update.  Now a public setter though.

https://codereview.chromium.org/459003002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_context_wrapper.h:113:
net::URLRequestContextGetter* request_context_getter_;
On 2014/08/12 03:48:52, michaeln wrote:
> scoped_refptr<T> for both of these new data refcounted members? We can clear
the
> values in InitInternal after constructing a Core with them

No longer applicable. I removed these members.

[jkarlin, 2014-08-12 18:46:16]

asvitkine@chromium.org: Please review changes in content/browser/renderer_host/ 
Thanks!

[michaeln, 2014-08-12 18:55:45]

https://codereview.chromium.org/459003002/diff/20001/content/browser/service_...
File content/browser/service_worker/service_worker_context_wrapper.cc (right):

https://codereview.chromium.org/459003002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_context_wrapper.cc:27:
request_context_getter_(browser_context->GetRequestContext()),
omg!

sorry i didn't notice that this is not the right context, to get the right
context at init time you need to fish it out of the StoragePartition
(StoragePartition::GetURLRequestContext()) 
and pass it into the contextwrapper ctor while doing
StoragePartitionImpl::Create.

i'll take a look at your new approach that doesn't init the class with these
values

[jkarlin, 2014-08-12 19:00:21]

On 2014/08/12 18:55:45, michaeln wrote:
>
https://codereview.chromium.org/459003002/diff/20001/content/browser/service_...
> File content/browser/service_worker/service_worker_context_wrapper.cc (right):
> 
>
https://codereview.chromium.org/459003002/diff/20001/content/browser/service_...
> content/browser/service_worker/service_worker_context_wrapper.cc:27:
> request_context_getter_(browser_context->GetRequestContext()),
> omg!
> 
> sorry i didn't notice that this is not the right context, to get the right
> context at init time you need to fish it out of the StoragePartition
> (StoragePartition::GetURLRequestContext()) 
> and pass it into the contextwrapper ctor while doing
> StoragePartitionImpl::Create.
> 
> i'll take a look at your new approach that doesn't init the class with these
> values

The URLRequestContext is created after StoragePartitionImpl::Create. We need to
init the class with the values sometime after ctor.  PTAL.

[Alexei Svitkine (slow), 2014-08-12 19:00:42]

-asvitkine, I'm only an owner for Mac-specific code in
content/browser/renderer_host/*

[jkarlin, 2014-08-12 19:14:28]

sadrul@chromium.org: Please review changes in content/browser/renderer_host/ 
Thanks!

[jkarlin, 2014-08-12 19:22:15]

Since you're in the middle of review I'm posting these comments here and will
fix these nits tonight.

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_cache.cc (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_cache.cc:10: #include
"content/browser/fileapi/chrome_blob_storage_context.h"
I'll remove this

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_cache.h (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_cache.h:22: class
ChromeBlobStorageContext;
I'll remove this.

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File
content/browser/service_worker/service_worker_cache_storage_manager_unittest.cc
(right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage_manager_unittest.cc:43:

I'll remove this line.

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_context_core.cc (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_context_core.cc:22: #include
"net/url_request/url_request_context_getter.h"
I'll remove this line

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_context_core.h (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_context_core.h:42: class
ChromeBlobStorageContext;
I'll remove this line.

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:23: #include
"webkit/browser/blob/blob_storage_context.h"
I'll remove this line

[michaeln, 2014-08-13 00:11:28]

alternative could you use StoragePartitionImplMap::PostCreateInitialization as
the one-time init callsite ?

https://codereview.chromium.org/459003002/diff/80001/content/browser/renderer...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_process_host_impl.cc:854:
storage_partition_impl_->GetURLRequestContext()->GetURLRequestContext(),
don't think you can access the unwrapped instance here since we're on the UI
thread

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_storage.cc (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.cc:37: const
base::WeakPtr<webkit_blob::BlobStorageContext>& blob_context)
either pass by value or const ref throughout to be self-consistent?

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_storage_manager.cc
(right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage_manager.cc:135:
blob_context_ = blob_storage_context;
if these already have values, should we DCHECK equality instead of reassigning?

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_storage_manager.h
(right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage_manager.h:19: }  //
namespace base
no need for // namespace base here

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:80:
net::URLRequestContext* request_context,
i think this would need to be a URLRequestContextGetter to go with this approach

[jkarlin, 2014-08-13 00:14:00]

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_cache.cc (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_cache.cc:10: #include
"content/browser/fileapi/chrome_blob_storage_context.h"
On 2014/08/12 19:22:15, jkarlin wrote:
> I'll remove this

Done.

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_cache.h (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_cache.h:22: class
ChromeBlobStorageContext;
On 2014/08/12 19:22:15, jkarlin wrote:
> I'll remove this.

Done.

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File
content/browser/service_worker/service_worker_cache_storage_manager_unittest.cc
(right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage_manager_unittest.cc:43:

On 2014/08/12 19:22:15, jkarlin wrote:
> I'll remove this line.

Done.

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_context_core.cc (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_context_core.cc:22: #include
"net/url_request/url_request_context_getter.h"
On 2014/08/12 19:22:15, jkarlin wrote:
> I'll remove this line

Done.

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_context_core.h (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_context_core.h:42: class
ChromeBlobStorageContext;
On 2014/08/12 19:22:15, jkarlin wrote:
> I'll remove this line.

Done.

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:23: #include
"webkit/browser/blob/blob_storage_context.h"
On 2014/08/12 19:22:15, jkarlin wrote:
> I'll remove this line

Needed to keep this one after all.

[jkarlin, 2014-08-13 00:34:41]

Thanks Michael, and double thanks for the IO thread catch.  I hope to have
layout tests to catch things like that soon.

Interesting idea with StoragePartitionImplMap::PostCreateInitialization.  I see
a lot of URLRequestContext initialization happening in the RenderProcessHost
(like IndexedDB) but none in PostCreateInitialization, so would prefer to keep
it where it is for now.

https://codereview.chromium.org/459003002/diff/80001/content/browser/renderer...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_process_host_impl.cc:854:
storage_partition_impl_->GetURLRequestContext()->GetURLRequestContext(),
On 2014/08/13 00:11:28, michaeln wrote:
> don't think you can access the unwrapped instance here since we're on the UI
> thread

Done.

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_storage.cc (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage.cc:37: const
base::WeakPtr<webkit_blob::BlobStorageContext>& blob_context)
On 2014/08/13 00:11:28, michaeln wrote:
> either pass by value or const ref throughout to be self-consistent?

Done.

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_storage_manager.cc
(right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage_manager.cc:135:
blob_context_ = blob_storage_context;
On 2014/08/13 00:11:28, michaeln wrote:
> if these already have values, should we DCHECK equality instead of
reassigning?

It's potentially possible for them to be assigned to NULL in the copy
constructor if we DeleteAndRestart or whatever that function is.  So for now I
think it's okay to assign up until the rest of the public API is used, which is
mostly verified by the DCHECK(request_context_) in
FindOrCreateServiceWorkerCacheManager.

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_storage_manager.h
(right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage_manager.h:19: }  //
namespace base
On 2014/08/13 00:11:28, michaeln wrote:
> no need for // namespace base here

Done.

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:80:
net::URLRequestContext* request_context,
On 2014/08/13 00:11:28, michaeln wrote:
> i think this would need to be a URLRequestContextGetter to go with this
approach

Done.

[michaeln, 2014-08-13 02:03:38]

lgtm

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_storage_manager.cc
(right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage_manager.cc:135:
blob_context_ = blob_storage_context;
On 2014/08/13 00:34:41, jkarlin wrote:
> On 2014/08/13 00:11:28, michaeln wrote:
> > if these already have values, should we DCHECK equality instead of
> reassigning?
> 
> It's potentially possible for them to be assigned to NULL in the copy
> constructor if we DeleteAndRestart or whatever that function is.  So for now I
> think it's okay to assign up until the rest of the public API is used, which
is
> mostly verified by the DCHECK(request_context_) in
> FindOrCreateServiceWorkerCacheManager.

They won't go from somevalue to NULL, that deleteandrestart ctor path will only
overwrite NULL with NULL.

If we ever see different non-null values for these params then those that are
set, something would be very wrong, i still think its worth a dcheck to prevent
code that would cause those really wrong conditions from getting into the
project.

https://codereview.chromium.org/459003002/diff/140001/content/browser/rendere...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/459003002/diff/140001/content/browser/rendere...
content/browser/renderer_host/render_process_host_impl.cc:1: // Copyright (c)
2012 The Chromium Authors. All rights reserved.
> but none in PostCreateInitialization

take a look at line 579 of storage_partition_impl_map.cc

https://codereview.chromium.org/459003002/diff/140001/content/browser/rendere...
content/browser/renderer_host/render_process_host_impl.cc:150: #include
"webkit/browser/blob/blob_storage_context.h"
not needed?

https://codereview.chromium.org/459003002/diff/140001/content/browser/service...
File content/browser/service_worker/service_worker_context_core.h (right):

https://codereview.chromium.org/459003002/diff/140001/content/browser/service...
content/browser/service_worker/service_worker_context_core.h:128:
ServiceWorkerCacheStorageManager* fetch_stores_manager() {
also rename the getter

https://codereview.chromium.org/459003002/diff/140001/content/browser/service...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/459003002/diff/140001/content/browser/service...
content/browser/service_worker/service_worker_dispatcher_host.cc:96: if
(blob_storage->context())
can that ever be null?

[jkarlin, 2014-08-13 11:56:42]

https://codereview.chromium.org/459003002/diff/140001/content/browser/rendere...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/459003002/diff/140001/content/browser/rendere...
content/browser/renderer_host/render_process_host_impl.cc:1: // Copyright (c)
2012 The Chromium Authors. All rights reserved.
On 2014/08/13 02:03:37, michaeln wrote:
> > but none in PostCreateInitialization
> 
> take a look at line 579 of storage_partition_impl_map.cc

You're right, this is a better place. Done.

https://codereview.chromium.org/459003002/diff/140001/content/browser/rendere...
content/browser/renderer_host/render_process_host_impl.cc:150: #include
"webkit/browser/blob/blob_storage_context.h"
On 2014/08/13 02:03:37, michaeln wrote:
> not needed?

Done.

https://codereview.chromium.org/459003002/diff/140001/content/browser/service...
File content/browser/service_worker/service_worker_context_core.h (right):

https://codereview.chromium.org/459003002/diff/140001/content/browser/service...
content/browser/service_worker/service_worker_context_core.h:128:
ServiceWorkerCacheStorageManager* fetch_stores_manager() {
On 2014/08/13 02:03:37, michaeln wrote:
> also rename the getter

Done.

https://codereview.chromium.org/459003002/diff/140001/content/browser/service...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/459003002/diff/140001/content/browser/service...
content/browser/service_worker/service_worker_dispatcher_host.cc:96: if
(blob_storage->context())
On 2014/08/13 02:03:37, michaeln wrote:
> can that ever be null?

No, not on the IO thread.  Done.

[jkarlin, 2014-08-13 11:57:54]

-sadrul as that file needed for review is removed from the CL

[jkarlin, 2014-08-13 12:12:03]

jochen@chromium.org: Please review changes in
content/browser/storage_partition_impl_map.  Thanks!

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
File content/browser/service_worker/service_worker_cache_storage_manager.cc
(right):

https://codereview.chromium.org/459003002/diff/80001/content/browser/service_...
content/browser/service_worker/service_worker_cache_storage_manager.cc:135:
blob_context_ = blob_storage_context;
On 2014/08/13 02:03:37, michaeln wrote:
> On 2014/08/13 00:34:41, jkarlin wrote:
> > On 2014/08/13 00:11:28, michaeln wrote:
> > > if these already have values, should we DCHECK equality instead of
> > reassigning?
> > 
> > It's potentially possible for them to be assigned to NULL in the copy
> > constructor if we DeleteAndRestart or whatever that function is.  So for now
I
> > think it's okay to assign up until the rest of the public API is used, which
> is
> > mostly verified by the DCHECK(request_context_) in
> > FindOrCreateServiceWorkerCacheManager.
> 
> They won't go from somevalue to NULL, that deleteandrestart ctor path will
only
> overwrite NULL with NULL.
> 
> If we ever see different non-null values for these params then those that are
> set, something would be very wrong, i still think its worth a dcheck to
prevent
> code that would cause those really wrong conditions from getting into the
> project.

Done.

[jochen (gone - plz use gerrit), 2014-08-13 12:32:26]

lgtm

[jkarlin, 2014-08-13 15:35:10]

The CQ bit was checked by jkarlin@chromium.org

[commit-bot: I haz the power, 2014-08-13 15:36:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jkarlin@chromium.org/459003002/260001

[commit-bot: I haz the power, 2014-08-13 17:16:20]

Committed patchset #13 (260001) as 289326