Loading: factor out common functionalities for memory cache testing

Loading: factor out common functionalities for memory cache testing

Before splitting resource type specific memory cache tests from
CachingCorrectnessTest, factor out common functionalities for memory
cache testing so that we can use it from multiple test files.

BUG=655920
Committed: https://crrev.com/aca85fb7b0661acec32e99688fc99f6de63b6e69
Cr-Commit-Position: refs/heads/master@{#435133}

[Takashi Toyoshima, 2016-11-16 09:59:08]

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

[commit-bot: I haz the power, 2016-11-16 09:59:25]

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

[Takashi Toyoshima, 2016-11-16 10:01:02]

Description was changed from

==========
Loading: factor out common functionalities for memory cache testing

Before splitting resource type specific memory cache tests from
CachingCorrectnessTest, factor out common functionalities for memory
cache testing so that we can use it from multiple test files.

BUG=655920
==========

to

==========
Loading: factor out common functionalities for memory cache testing

Before splitting resource type specific memory cache tests from
CachingCorrectnessTest, factor out common functionalities for memory
cache testing so that we can use it from multiple test files.

BUG=655920
==========

[Takashi Toyoshima, 2016-11-16 10:01:03]

toyoshim@chromium.org changed reviewers:
+ hiroshige@chromium.org, yhirano@chromium.org

[Takashi Toyoshima, 2016-11-16 10:01:30]

ptal

[commit-bot: I haz the power, 2016-11-16 11:34:42]

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

[commit-bot: I haz the power, 2016-11-16 11:34:43]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-11-17 06:24:38]

lgtm with nits.

https://codereview.chromium.org/2509593003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/fetch/MemoryCacheCorrectnessTestHelper.h
(right):

https://codereview.chromium.org/2509593003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/MemoryCacheCorrectnessTestHelper.h:24:
Resource* fetch();
Memo (not blocking this and subsequent CLs, low priority):
fetch() and createResource() returns Resource*, but I think it might be better
to return pointers to actual subclasses (e.g. RawResource*). Many test cases
compares old and new Resource* pointers whether cache is used (or not used)
expectedly, but if their actual types differ (e.g. ImageResource and
RawResource), then the test is not testing what should have been tested (cache
is never used).
Using pointers to subclasses might be a little easier to detect such errors in
writing tests.

https://codereview.chromium.org/2509593003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/MemoryCacheCorrectnessTestHelper.h:36:
virtual void SetUp();
virtual -> override (or final)?

https://codereview.chromium.org/2509593003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/MemoryCacheCorrectnessTestHelper.h:37:
virtual void TearDown();
ditto.

[Takashi Toyoshima, 2016-11-29 07:56:12]

https://codereview.chromium.org/2509593003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/fetch/MemoryCacheCorrectnessTestHelper.h
(right):

https://codereview.chromium.org/2509593003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/MemoryCacheCorrectnessTestHelper.h:24:
Resource* fetch();
OK, let me leave a TODO here.

https://codereview.chromium.org/2509593003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/MemoryCacheCorrectnessTestHelper.h:36:
virtual void SetUp();
On 2016/11/17 06:24:38, hiroshige wrote:
> virtual -> override (or final)?

Done.

https://codereview.chromium.org/2509593003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/MemoryCacheCorrectnessTestHelper.h:37:
virtual void TearDown();
On 2016/11/17 06:24:38, hiroshige wrote:
> ditto.

Done.

[Takashi Toyoshima, 2016-11-29 07:56:33]

yhirano: ptal, this patch does not have any dependency now.

[yhirano, 2016-11-29 08:25:44]

lgtm

[Takashi Toyoshima, 2016-11-29 08:27:54]

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

[commit-bot: I haz the power, 2016-11-29 08:28:14]

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

[Takashi Toyoshima, 2016-11-29 09:45:53]

toyoshim@chromium.org changed reviewers:
+ kouhei@chromium.org

[Takashi Toyoshima, 2016-11-29 09:46:11]

+kouhei for BUILD.gn

[commit-bot: I haz the power, 2016-11-29 10:13:49]

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

[commit-bot: I haz the power, 2016-11-29 10:13:50]

Dry run: This issue passed the CQ dry run.

[kouhei (in TOK), 2016-11-30 01:40:52]

rslgtm

[kouhei (in TOK), 2016-11-30 01:41:02]

On 2016/11/30 01:40:52, kouhei wrote:
> rslgtm

real lgtm

[Takashi Toyoshima, 2016-11-30 05:12:36]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2016-11-30 05:12:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
hiroshige@chromium.org
Link to the patchset: https://codereview.chromium.org/2509593003/#ps20001
(title: "review #8")

[commit-bot: I haz the power, 2016-11-30 05:13:06]

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, 2016-11-30 07:40:30]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480482756986140,
"parent_rev": "382f21ca1a4310eb55aec3a25762c4de1d067dff", "commit_rev":
"7d74e7556fe57c7e0aa52be63c84ea471cbbf2eb"}

[commit-bot: I haz the power, 2016-11-30 07:41:14]

Description was changed from

==========
Loading: factor out common functionalities for memory cache testing

Before splitting resource type specific memory cache tests from
CachingCorrectnessTest, factor out common functionalities for memory
cache testing so that we can use it from multiple test files.

BUG=655920
==========

to

==========
Loading: factor out common functionalities for memory cache testing

Before splitting resource type specific memory cache tests from
CachingCorrectnessTest, factor out common functionalities for memory
cache testing so that we can use it from multiple test files.

BUG=655920
==========

[commit-bot: I haz the power, 2016-11-30 07:41:15]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-30 07:43:51]

Description was changed from

==========
Loading: factor out common functionalities for memory cache testing

Before splitting resource type specific memory cache tests from
CachingCorrectnessTest, factor out common functionalities for memory
cache testing so that we can use it from multiple test files.

BUG=655920
==========

to

==========
Loading: factor out common functionalities for memory cache testing

Before splitting resource type specific memory cache tests from
CachingCorrectnessTest, factor out common functionalities for memory
cache testing so that we can use it from multiple test files.

BUG=655920

Committed: https://crrev.com/aca85fb7b0661acec32e99688fc99f6de63b6e69
Cr-Commit-Position: refs/heads/master@{#435133}
==========

[commit-bot: I haz the power, 2016-11-30 07:43:52]

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