Added ServiceWorkerResponses to GetJobResponse callback

Added ServiceWorkerResponses to GetJobResponse callback

This CL does two things. First, it creates the ServiceWorkerRepsonse
objects corresponding to each RequestInfo structure and fills out partial
data there.

Second, it removes the asynchronous calls to build each file backed blob
in favor of a single call which inititalizes the blob storage context. A
future CL will move that initialization into the BackgroundFetchContext
so that the DataManager doesn't need to know about the BrowserContext.

Also added unit tests for the response.

BUG=692621
Review-Url: https://codereview.chromium.org/2774263003
Cr-Commit-Position: refs/heads/master@{#459813}
Committed: https://chromium.googlesource.com/chromium/src/+/1aa34de9d58f09594f907e6aff43ca07c798164d

[harkness, 2017-03-27 10:28:18]

harkness@chromium.org changed reviewers:
+ peter@chromium.org

[harkness, 2017-03-27 10:28:24]

The CQ bit was checked by harkness@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 10:28:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-27 10:30:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-27 10:30:51]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[harkness, 2017-03-27 10:59:50]

The CQ bit was checked by harkness@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 11:00:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Peter Beverloo, 2017-03-27 11:05:16]

https://codereview.chromium.org/2774263003/diff/40001/content/browser/backgro...
File content/browser/background_fetch/background_fetch_data_manager.cc (right):

https://codereview.chromium.org/2774263003/diff/40001/content/browser/backgro...
content/browser/background_fetch/background_fetch_data_manager.cc:102:
BrowserThread::IO, FROM_HERE,
BrowserThread::UI

This class lives on the IO thread, ChromeBlobStorageContext::GetFor() must be
invoked on the UI thread.

https://codereview.chromium.org/2774263003/diff/40001/content/browser/backgro...
content/browser/background_fetch/background_fetch_data_manager.cc:111:
ChromeBlobStorageContext* blob_context) {
DCHECK(blob_context);

https://codereview.chromium.org/2774263003/diff/40001/content/browser/backgro...
File content/browser/background_fetch/background_fetch_job_response_data.cc
(right):

https://codereview.chromium.org/2774263003/diff/40001/content/browser/backgro...
content/browser/background_fetch/background_fetch_job_response_data.cc:29:
responses_.emplace_back(ServiceWorkerResponse(
You're now invoking the copy constructor. The arguments to emplace_back() are
the argument you'd pass to the constructor.

https://codereview.chromium.org/2774263003/diff/40001/content/browser/backgro...
content/browser/background_fetch/background_fetch_job_response_data.cc:29:
responses_.emplace_back(ServiceWorkerResponse(
To confirm, we still plan on getting rid of BackgroundFetchResponseData, right?
In the future this'd live in BackgroundFetchDataManager::BuildResponse or w/e

[commit-bot: I haz the power, 2017-03-27 12:18:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-27 12:18:38]

Dry run: This issue passed the CQ dry run.

[harkness, 2017-03-27 12:23:19]

https://codereview.chromium.org/2774263003/diff/40001/content/browser/backgro...
File content/browser/background_fetch/background_fetch_data_manager.cc (right):

https://codereview.chromium.org/2774263003/diff/40001/content/browser/backgro...
content/browser/background_fetch/background_fetch_data_manager.cc:102:
BrowserThread::IO, FROM_HERE,
On 2017/03/27 11:05:16, Peter Beverloo wrote:
> BrowserThread::UI
> 
> This class lives on the IO thread, ChromeBlobStorageContext::GetFor() must be
> invoked on the UI thread.

According to the comments, only the constructor should be called on the UI
thread. It is odd that it does a post task to the IO thread, but it doesn't seem
to require that it be called on any particular thread.

https://codereview.chromium.org/2774263003/diff/40001/content/browser/backgro...
content/browser/background_fetch/background_fetch_data_manager.cc:111:
ChromeBlobStorageContext* blob_context) {
On 2017/03/27 11:05:16, Peter Beverloo wrote:
> DCHECK(blob_context);

Done.

https://codereview.chromium.org/2774263003/diff/40001/content/browser/backgro...
File content/browser/background_fetch/background_fetch_job_response_data.cc
(right):

https://codereview.chromium.org/2774263003/diff/40001/content/browser/backgro...
content/browser/background_fetch/background_fetch_job_response_data.cc:29:
responses_.emplace_back(ServiceWorkerResponse(
On 2017/03/27 11:05:16, Peter Beverloo wrote:
> You're now invoking the copy constructor. The arguments to emplace_back() are
> the argument you'd pass to the constructor.

Done.

https://codereview.chromium.org/2774263003/diff/40001/content/browser/backgro...
content/browser/background_fetch/background_fetch_job_response_data.cc:29:
responses_.emplace_back(ServiceWorkerResponse(
On 2017/03/27 11:05:16, Peter Beverloo wrote:
> To confirm, we still plan on getting rid of BackgroundFetchResponseData,
right?
> In the future this'd live in BackgroundFetchDataManager::BuildResponse or w/e

I have questions about how we're going to update the responses asynchronously,
but if we can solve that, then this class will absolutely go away and the
vectors will just be local variables in some function.

[harkness, 2017-03-27 12:35:32]

https://codereview.chromium.org/2774263003/diff/40001/content/browser/backgro...
File content/browser/background_fetch/background_fetch_data_manager.cc (right):

https://codereview.chromium.org/2774263003/diff/40001/content/browser/backgro...
content/browser/background_fetch/background_fetch_data_manager.cc:102:
BrowserThread::IO, FROM_HERE,
On 2017/03/27 12:23:19, harkness wrote:
> On 2017/03/27 11:05:16, Peter Beverloo wrote:
> > BrowserThread::UI
> > 
> > This class lives on the IO thread, ChromeBlobStorageContext::GetFor() must
be
> > invoked on the UI thread.
> 
> According to the comments, only the constructor should be called on the UI
> thread. It is odd that it does a post task to the IO thread, but it doesn't
seem
> to require that it be called on any particular thread.

Or maybe the comments are wrong!  :)

[Peter Beverloo, 2017-03-27 13:05:02]

lgtm..

[harkness, 2017-03-27 16:20:10]

The CQ bit was checked by harkness@chromium.org

[commit-bot: I haz the power, 2017-03-27 16:20:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-27 17:18:23]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1490631610724700,
"parent_rev": "41769318b72c3ca03f8da51ada352ac998672ee8", "commit_rev":
"1aa34de9d58f09594f907e6aff43ca07c798164d"}

[commit-bot: I haz the power, 2017-03-27 17:19:12]

Description was changed from

==========
Added ServiceWorkerResponses to GetJobResponse callback

This CL does two things. First, it creates the ServiceWorkerRepsonse
objects corresponding to each RequestInfo structure and fills out partial
data there.

Second, it removes the asynchronous calls to build each file backed blob
in favor of a single call which inititalizes the blob storage context. A
future CL will move that initialization into the BackgroundFetchContext
so that the DataManager doesn't need to know about the BrowserContext.

Also added unit tests for the response.

BUG=692621
==========

to

==========
Added ServiceWorkerResponses to GetJobResponse callback

This CL does two things. First, it creates the ServiceWorkerRepsonse
objects corresponding to each RequestInfo structure and fills out partial
data there.

Second, it removes the asynchronous calls to build each file backed blob
in favor of a single call which inititalizes the blob storage context. A
future CL will move that initialization into the BackgroundFetchContext
so that the DataManager doesn't need to know about the BrowserContext.

Also added unit tests for the response.

BUG=692621

Review-Url: https://codereview.chromium.org/2774263003
Cr-Commit-Position: refs/heads/master@{#459813}
Committed:
https://chromium.googlesource.com/chromium/src/+/1aa34de9d58f09594f907e6aff43...
==========

[commit-bot: I haz the power, 2017-03-27 17:19:13]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/1aa34de9d58f09594f907e6aff43...