Pipe response_time from FetchManager to CacheStorage and ServiceWorkerURLRequestJob.

Pipe response_time from FetchManager to CacheStorage and ServiceWorkerURLRequestJob.

Currently response_time is used to identify the response in HttpCache when storing the V8 code cache to HttpCache.

We are planning to support the V8 code caching in CacheStorage.
To support this, we have to pipe the response_time from FetchManager to CacheStorage and ServiceWorkerURLRequestJob.
Design doc: https://docs.google.com/document/d/1dVlgbXls-G65koU2vlmAXy1EVsoY9Z55jaEX7cZgfDA

BUG=581613
Committed: https://crrev.com/3fbfbe6e7690dd2b169cfaaae2ad7ac89537d74b
Cr-Commit-Position: refs/heads/master@{#377862}

[horo, 2016-02-25 06:10:15]

Patchset #1 (id:1) has been deleted

[horo, 2016-02-25 06:10:31]

Patchset #1 (id:20001) has been deleted

[horo, 2016-02-25 06:12:29]

Description was changed from

==========
response time

BUG=
==========

to

==========
Pipe response_time from FetchManager to CacheStorage and
ServiceWorkerURLRequestJob.

Currently response_time is used to identify the response in HttpCache when
storeing the V8 code cache to HttpCache.

We are planning to support the V8 code caching in CacheStorage.
To support this, we have to pipe the response_time from FetchManager to
CacheStorage and ServiceWorkerURLRequestJob.
Design doc:
https://docs.google.com/document/d/1dVlgbXls-G65koU2vlmAXy1EVsoY9Z55jaEX7cZgfDA

BUG=581613
==========

[horo, 2016-02-25 06:14:17]

horo@chromium.org changed reviewers:
+ nhiroki@chromium.org

[horo, 2016-02-25 06:14:18]

nhiroki@
Could you please review this?

[nhiroki, 2016-02-25 07:40:08]

LGTM

(In CL description) s/storeing/storing/

[horo, 2016-02-25 07:42:20]

Description was changed from

==========
Pipe response_time from FetchManager to CacheStorage and
ServiceWorkerURLRequestJob.

Currently response_time is used to identify the response in HttpCache when
storeing the V8 code cache to HttpCache.

We are planning to support the V8 code caching in CacheStorage.
To support this, we have to pipe the response_time from FetchManager to
CacheStorage and ServiceWorkerURLRequestJob.
Design doc:
https://docs.google.com/document/d/1dVlgbXls-G65koU2vlmAXy1EVsoY9Z55jaEX7cZgfDA

BUG=581613
==========

to

==========
Pipe response_time from FetchManager to CacheStorage and
ServiceWorkerURLRequestJob.

Currently response_time is used to identify the response in HttpCache when
storing the V8 code cache to HttpCache.

We are planning to support the V8 code caching in CacheStorage.
To support this, we have to pipe the response_time from FetchManager to
CacheStorage and ServiceWorkerURLRequestJob.
Design doc:
https://docs.google.com/document/d/1dVlgbXls-G65koU2vlmAXy1EVsoY9Z55jaEX7cZgfDA

BUG=581613
==========

[horo, 2016-02-25 07:42:36]

On 2016/02/25 07:40:08, nhiroki wrote:
> LGTM
> 
> (In CL description) s/storeing/storing/

Done. Thank you!

[horo, 2016-02-25 07:44:27]

horo@chromium.org changed reviewers:
+ junov@chromium.org

[horo, 2016-02-25 07:44:27]

junov@chromium.org: Please review changes in 

Could you please review
third_party/WebKit/Source/platform/exported/WebServiceWorkerResponse.cpp?

[horo, 2016-02-25 08:14:46]

horo@chromium.org changed reviewers:
+ dcheng@chromium.org

[horo, 2016-02-25 08:14:47]

dcheng@
Could you please review content/common/service_worker/service_worker_messages.h?

[dcheng, 2016-02-25 18:43:53]

ipc lgtm

https://codereview.chromium.org/1738723002/diff/40001/content/common/service_...
File content/common/service_worker/service_worker_types.h (right):

https://codereview.chromium.org/1738723002/diff/40001/content/common/service_...
content/common/service_worker/service_worker_types.h:165: const base::Time&
response_time);
Nit: I believe we prefer to pass base::Time by value.

https://codereview.chromium.org/1738723002/diff/40001/content/common/service_...
content/common/service_worker/service_worker_types.h:178: base::Time
response_time;
Nit: consider making this const: I don't think this field is ever changed
outside the constructor.

[horo, 2016-02-26 02:29:57]

Thank you!

https://codereview.chromium.org/1738723002/diff/40001/content/common/service_...
File content/common/service_worker/service_worker_types.h (right):

https://codereview.chromium.org/1738723002/diff/40001/content/common/service_...
content/common/service_worker/service_worker_types.h:165: const base::Time&
response_time);
On 2016/02/25 18:43:53, dcheng wrote:
> Nit: I believe we prefer to pass base::Time by value.

Done.

https://codereview.chromium.org/1738723002/diff/40001/content/common/service_...
content/common/service_worker/service_worker_types.h:178: base::Time
response_time;
On 2016/02/25 18:43:53, dcheng wrote:
> Nit: consider making this const: I don't think this field is ever changed
> outside the constructor.

I think we can't use "const" in the structures which are sent in IPC messages.


In file included from ../../content/common/content_message_generator.cc:7:
In file included from ../../content/common/content_message_generator.h:7:
In file included from ../../content/common/child_process_messages.h:21:
In file included from ../../ipc/ipc_message_macros.h:207:
In file included from ../../ipc/ipc_message_templates.h:16:
../../ipc/ipc_message_utils.h:96:29: error: no member named 'Read' in
'IPC::ParamTraits<const base::Time>'
  return ParamTraits<Type>::Read(m, iter, reinterpret_cast<Type* >(p));
         ~~~~~~~~~~~~~~~~~~~^
../../content/common/service_worker/service_worker_messages.h:89:3: note: in
instantiation of function template specialization 'IPC::ReadParam<const
base::Time>' requested here
  IPC_STRUCT_TRAITS_MEMBER(response_time)
  ^
../../ipc/param_traits_read_macros.h:29:40: note: expanded from macro
'IPC_STRUCT_TRAITS_MEMBER'
#define IPC_STRUCT_TRAITS_MEMBER(name) ReadParam(m, iter, &p->name) &&
                                       ^
1 error generated.

[horo, 2016-02-26 05:10:35]

horo@chromium.org changed reviewers:
+ mkwst@chromium.org
- junov@chromium.org

[horo, 2016-02-26 05:10:35]

mkwst@
Could you please review
third_party/WebKit/Source/platform/exported/WebServiceWorkerResponse.cpp?

[horo, 2016-02-26 05:23:53]

horo@chromium.org changed reviewers:
+ junov@chromium.org

[Mike West, 2016-02-26 08:35:08]

LGTM.

[horo, 2016-02-26 08:55:02]

Thank you

[horo, 2016-02-26 08:55:12]

The CQ bit was checked by horo@chromium.org

[horo, 2016-02-26 08:55:12]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org,
nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/1738723002/#ps60001
(title: "pass base::Time by value")

[commit-bot: I haz the power, 2016-02-26 08:55:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1738723002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1738723002/60001

[commit-bot: I haz the power, 2016-02-26 10:06:26]

Description was changed from

==========
Pipe response_time from FetchManager to CacheStorage and
ServiceWorkerURLRequestJob.

Currently response_time is used to identify the response in HttpCache when
storing the V8 code cache to HttpCache.

We are planning to support the V8 code caching in CacheStorage.
To support this, we have to pipe the response_time from FetchManager to
CacheStorage and ServiceWorkerURLRequestJob.
Design doc:
https://docs.google.com/document/d/1dVlgbXls-G65koU2vlmAXy1EVsoY9Z55jaEX7cZgfDA

BUG=581613
==========

to

==========
Pipe response_time from FetchManager to CacheStorage and
ServiceWorkerURLRequestJob.

Currently response_time is used to identify the response in HttpCache when
storing the V8 code cache to HttpCache.

We are planning to support the V8 code caching in CacheStorage.
To support this, we have to pipe the response_time from FetchManager to
CacheStorage and ServiceWorkerURLRequestJob.
Design doc:
https://docs.google.com/document/d/1dVlgbXls-G65koU2vlmAXy1EVsoY9Z55jaEX7cZgfDA

BUG=581613
==========

[commit-bot: I haz the power, 2016-02-26 10:06:28]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-02-26 10:07:43]

Description was changed from

==========
Pipe response_time from FetchManager to CacheStorage and
ServiceWorkerURLRequestJob.

Currently response_time is used to identify the response in HttpCache when
storing the V8 code cache to HttpCache.

We are planning to support the V8 code caching in CacheStorage.
To support this, we have to pipe the response_time from FetchManager to
CacheStorage and ServiceWorkerURLRequestJob.
Design doc:
https://docs.google.com/document/d/1dVlgbXls-G65koU2vlmAXy1EVsoY9Z55jaEX7cZgfDA

BUG=581613
==========

to

==========
Pipe response_time from FetchManager to CacheStorage and
ServiceWorkerURLRequestJob.

Currently response_time is used to identify the response in HttpCache when
storing the V8 code cache to HttpCache.

We are planning to support the V8 code caching in CacheStorage.
To support this, we have to pipe the response_time from FetchManager to
CacheStorage and ServiceWorkerURLRequestJob.
Design doc:
https://docs.google.com/document/d/1dVlgbXls-G65koU2vlmAXy1EVsoY9Z55jaEX7cZgfDA

BUG=581613

Committed: https://crrev.com/3fbfbe6e7690dd2b169cfaaae2ad7ac89537d74b
Cr-Commit-Position: refs/heads/master@{#377862}
==========

[commit-bot: I haz the power, 2016-02-26 10:07:44]

Patchset 2 (id:??) landed as
https://crrev.com/3fbfbe6e7690dd2b169cfaaae2ad7ac89537d74b
Cr-Commit-Position: refs/heads/master@{#377862}

[aelias_OOO_until_Jul13, 2016-02-27 00:31:36]

A revert of this CL (patchset #2 id:60001) has been created in
https://codereview.chromium.org/1746483002/ by aelias@chromium.org.

The reason for reverting is: The newly added test FetchEvent_ResponseTime is
~33% flaky on Android, see http://crbug.com/590400.  The error message is
"[FATAL:weak_ptr.cc(20)] Check failed:
sequence_checker_.CalledOnValidSequencedThread() || HasOneRef(). WeakPtrs must
be invalidated on the same sequenced thread."

BUG=590400.