platform/testing/{URL|Unit}TestHelpers improvements

platform/testing/{URL|Unit}TestHelpers improvements

URLTestHelpers assumed that the test directory is web/tests/data.
But for blink_platform_tests, we didn't have a dependency to
web/tests/data in build rules, and should use platform/testing/data
instead of web/tests/data.

This patch revises URLTestHelpers so that it works without knowing
where the test directory is, and adds interfaces that provide
test directories for web and platform.

Also this patch does some refactoring changes against caller side
files so that these interface changes should not affect so much.

Some tests that directly had test directory path literals are
fixed to use UnitTestHelpers to obtain fullpaths for test files.

BUG=655920
Review-Url: https://codereview.chromium.org/2654933003
Cr-Commit-Position: refs/heads/master@{#448227}
Committed: https://chromium.googlesource.com/chromium/src/+/8fabb1af2315f0b6fc685118b7c25b726bf3f94e

[Takashi Toyoshima, 2017-01-25 13:56:01]

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

[commit-bot: I haz the power, 2017-01-25 13:56:13]

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-01-25 14:25:17]

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

[commit-bot: I haz the power, 2017-01-25 14:25:18]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Takashi Toyoshima, 2017-01-26 07:54:19]

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

[commit-bot: I haz the power, 2017-01-26 07:54:30]

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

[Takashi Toyoshima, 2017-01-26 07:54:35]

Description was changed from

==========
wip

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

to

==========
wip

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

[Takashi Toyoshima, 2017-01-26 07:54:35]

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

[Takashi Toyoshima, 2017-01-26 08:14:01]

Description was changed from

==========
wip

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

to

==========
platform/testing/{URL|Unit}TestHelpers improvements

URLTestHelpers assumed that the test directory is web/tests/data.
But for blink_platform_tests, we didn't have a dependency to
web/tests/data in build rules, and should use platform/testing/data
instead of web/tests/data.

This patch revises URLTestHelpers so that it works without knowing
where the test directory is, and adds interfaces that provide
test directories for web and platform.

Also this patch does some refactoring changes against caller side
files so that these interface changes should not affect so much.

Some tests that directly had test directory path literals are
fixed to use UnitTestHelpers to obtain fullpaths for test files.

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

[Takashi Toyoshima, 2017-01-26 08:15:55]

ptal.
This is a patch to remove my TODO in platform/BUILD.gn.

URLTestHelpers and URLTestHelpers are main files that I modified.
Others are caller side mechanical, or refactoring changes.

[commit-bot: I haz the power, 2017-01-26 08:30:05]

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

[commit-bot: I haz the power, 2017-01-26 08:30:06]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[kinuko, 2017-01-26 09:32:41]

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp
(right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp:205: class
ScopedRegisteredURL {
A generalized form of this class could be probably put in URLTestHelpers (no
need to be considered in this change)

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/testing/URLTestHelpers.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/URLTestHelpers.cpp:57:
base::FilePath(basePath.utf8().data())
Let's use WebStringToFilePath()

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/URLTestHelpers.cpp:58:
.Append(FILE_PATH_LITERAL(fileName.utf8().data()));
Ditto

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/URLTestHelpers.cpp:61:
registerMockedURLLoad(url, String(filePath.AsUTF8Unsafe().c_str()), mimeType);
FilePathToWebString

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/testing/UnitTestHelpers.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/UnitTestHelpers.cpp:89: return
String::fromUTF8(path.AsUTF8Unsafe().c_str());
FilePathToWebString(path)

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/UnitTestHelpers.cpp:96: return
String::fromUTF8(path.AsUTF8Unsafe().c_str());
ditto

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/UnitTestHelpers.cpp:104: return
String::fromUTF8(path.AsUTF8Unsafe().c_str());
ditto

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/ExternalPopupMenuTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/ExternalPopupMenuTest.cpp:117:
WebString::fromUTF8(m_baseURL.c_str()),
fromUTF8 can directly take std::string

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/LayoutGeometryMapTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/LayoutGeometryMapTest.cpp:164:
WebString::fromUTF8(fileName.c_str()));
Not from your change, but no need of .c_str() (and we can save strlen() cost)

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/ListenerLeakTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/ListenerLeakTest.cpp:94:
WebString::fromUTF8(fileName.c_str()));
no need of .c_str()

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp:95:
WebString::fromUTF8(fileName.c_str()));
ditto

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/ViewportTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/ViewportTest.cpp:71:
WebString::fromUTF8(fileName.c_str()));
ditto

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/ViewportTest.cpp:77:
WebString::fromUTF8(fileName.c_str()));
ditto

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp
(right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp:78:
WebString::fromUTF8(baseURL.c_str()), testing::webTestDataPath(),
ditto

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebPluginContainerTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebPluginContainerTest.cpp:99:
WebString::fromUTF8(m_baseURL.c_str()), testing::webTestDataPath(),
ditto

[Takashi Toyoshima, 2017-01-26 10:57:58]

try failed on some tests, but it is passing on local host.
I'd re-run trybots, and check more details on local host too.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp
(right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp:205: class
ScopedRegisteredURL {
On 2017/01/26 09:32:40, kinuko wrote:
> A generalized form of this class could be probably put in URLTestHelpers (no
> need to be considered in this change)

Acknowledged.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/testing/URLTestHelpers.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/URLTestHelpers.cpp:57:
base::FilePath(basePath.utf8().data())
Thanks, correct conversion looks more complicated than I thought.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/URLTestHelpers.cpp:58:
.Append(FILE_PATH_LITERAL(fileName.utf8().data()));
On 2017/01/26 09:32:40, kinuko wrote:
> Ditto

Done.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/URLTestHelpers.cpp:61:
registerMockedURLLoad(url, String(filePath.AsUTF8Unsafe().c_str()), mimeType);
On 2017/01/26 09:32:41, kinuko wrote:
> FilePathToWebString

Done.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/testing/UnitTestHelpers.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/UnitTestHelpers.cpp:89: return
String::fromUTF8(path.AsUTF8Unsafe().c_str());
On 2017/01/26 09:32:41, kinuko wrote:
> FilePathToWebString(path)

Done.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/UnitTestHelpers.cpp:96: return
String::fromUTF8(path.AsUTF8Unsafe().c_str());
On 2017/01/26 09:32:41, kinuko wrote:
> ditto

Done.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/UnitTestHelpers.cpp:104: return
String::fromUTF8(path.AsUTF8Unsafe().c_str());
On 2017/01/26 09:32:41, kinuko wrote:
> ditto

Done.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/ExternalPopupMenuTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/ExternalPopupMenuTest.cpp:117:
WebString::fromUTF8(m_baseURL.c_str()),
On 2017/01/26 09:32:41, kinuko wrote:
> fromUTF8 can directly take std::string

Done.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/BrowserControlsTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/BrowserControlsTest.cpp:98:
WebString::fromUTF8(m_baseURL.c_str()), testing::webTestDataPath(),
removing c_str()

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/CompositorWorkerTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/CompositorWorkerTest.cpp:66:
WebString::fromUTF8(fileName.c_str()));
removing c_str

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/LayoutGeometryMapTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/LayoutGeometryMapTest.cpp:164:
WebString::fromUTF8(fileName.c_str()));
On 2017/01/26 09:32:41, kinuko wrote:
> Not from your change, but no need of .c_str() (and we can save strlen() cost)

Done.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/ListenerLeakTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/ListenerLeakTest.cpp:94:
WebString::fromUTF8(fileName.c_str()));
On 2017/01/26 09:32:41, kinuko wrote:
> no need of .c_str()

Done.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/ProgrammaticScrollTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/ProgrammaticScrollTest.cpp:35:
WebString::fromUTF8(m_baseURL.c_str()), testing::webTestDataPath(),
removing c_str

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/RootScrollerTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/RootScrollerTest.cpp:85:
WebString::fromUTF8(m_baseURL.c_str()), testing::webTestDataPath(),
removing c_str

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp:95:
WebString::fromUTF8(fileName.c_str()));
On 2017/01/26 09:32:41, kinuko wrote:
> ditto

Done.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/ViewportTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/ViewportTest.cpp:71:
WebString::fromUTF8(fileName.c_str()));
On 2017/01/26 09:32:41, kinuko wrote:
> ditto

Done.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/ViewportTest.cpp:77:
WebString::fromUTF8(fileName.c_str()));
On 2017/01/26 09:32:41, kinuko wrote:
> ditto

Done.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/VisualViewportTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/VisualViewportTest.cpp:154:
WebString::fromUTF8(m_baseURL.c_str()),
removing c_str

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebDocumentSubresourceFilterTest.cpp
(right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebDocumentSubresourceFilterTest.cpp:106:
WebString::fromUTF8(m_baseURL.c_str()), testing::webTestDataPath(),
removing c_str

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:213:
WebString::fromUTF8(m_baseURL.c_str()), testing::webTestDataPath(),
removing c_str here and there

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp
(right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp:78:
WebString::fromUTF8(baseURL.c_str()), testing::webTestDataPath(),
On 2017/01/26 09:32:41, kinuko wrote:
> ditto

Done.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebPluginContainerTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebPluginContainerTest.cpp:99:
WebString::fromUTF8(m_baseURL.c_str()), testing::webTestDataPath(),
On 2017/01/26 09:32:41, kinuko wrote:
> ditto

Done.

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebSearchableFormDataTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebSearchableFormDataTest.cpp:51:
WebString::fromUTF8(baseURL.c_str()), testing::webTestDataPath(),
removing c_str

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebViewTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:239:
WebString::fromUTF8(m_baseURL.c_str()),
removing c_str

[Takashi Toyoshima, 2017-01-26 10:58:13]

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

[commit-bot: I haz the power, 2017-01-26 10:58:32]

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-01-26 11:34:20]

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

[commit-bot: I haz the power, 2017-01-26 11:34:21]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Takashi Toyoshima, 2017-01-26 11:36:02]

I found the reason of errors. I'd upload revised one.

https://codereview.chromium.org/2654933003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:213:
WebString::fromUTF8(m_baseURL), testing::webTestDataPath(),
Oops, just there is a bug, and I can repro it on local.

[Takashi Toyoshima, 2017-01-26 11:38:02]

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

[commit-bot: I haz the power, 2017-01-26 11:38:14]

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-01-26 12:14:11]

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

[commit-bot: I haz the power, 2017-01-26 12:14:12]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Takashi Toyoshima, 2017-01-26 12:48:21]

https://codereview.chromium.org/2654933003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/testing/UnitTestHelpers.cpp (right):

https://codereview.chromium.org/2654933003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/UnitTestHelpers.cpp:95:
.Append(relativePath));
Hum... this should be AppendASCII() to compile on Windows, or probably taking
"const String& relativePath", and convert it via WebStringToFilePath looks a
right approach.

Let me try changing argument type to const String&. That should not need caller
side changes so much.

[Takashi Toyoshima, 2017-01-26 12:48:33]

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

[commit-bot: I haz the power, 2017-01-26 12:48:49]

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-01-26 14:14:43]

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

[commit-bot: I haz the power, 2017-01-26 14:14:44]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Takashi Toyoshima, 2017-01-27 09:05:02]

Patch Set 6 passed the CQ dryrun. Could you take another look?

[kinuko, 2017-01-27 15:50:40]

lgtm

https://codereview.chromium.org/2654933003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/testing/URLTestHelpers.h (right):

https://codereview.chromium.org/2654933003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/testing/URLTestHelpers.h:53: // |basePath| or
|filePath| could be obtained though UnitTestHelpers.
though -> through

What do you mean by 'filePath' could be obtained?

This comment can probably be more explicit, e.g. "webTestDataPath() or
platformTestDataPath() in UnitTestHelpers can be used to get the appropriate
|basePath| for test data directories"

https://codereview.chromium.org/2654933003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/testing/UnitTestHelpers.h (right):

https://codereview.chromium.org/2654933003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/testing/UnitTestHelpers.h:57: // It returns
top web test directory if |relativePath| was not specified.
nit: top web test directory -> the top web test directory

https://codereview.chromium.org/2654933003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/testing/UnitTestHelpers.h:62: // It returns
top platform test directory if |relativePath| was not specified.
top ... -> the top ...

[haraken, 2017-01-27 18:17:48]

LGTM

[Takashi Toyoshima, 2017-02-06 08:14:55]

https://codereview.chromium.org/2654933003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/testing/URLTestHelpers.h (right):

https://codereview.chromium.org/2654933003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/testing/URLTestHelpers.h:53: // |basePath| or
|filePath| could be obtained though UnitTestHelpers.
Done.

|filePath| means the argument of registerMockedURLLoad().

Probably, it would be better to split these comments to common one and
per-function one.

https://codereview.chromium.org/2654933003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/testing/UnitTestHelpers.h (right):

https://codereview.chromium.org/2654933003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/testing/UnitTestHelpers.h:57: // It returns
top web test directory if |relativePath| was not specified.
On 2017/01/27 15:50:40, kinuko wrote:
> nit: top web test directory -> the top web test directory

Done.

https://codereview.chromium.org/2654933003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/testing/UnitTestHelpers.h:62: // It returns
top platform test directory if |relativePath| was not specified.
On 2017/01/27 15:50:40, kinuko wrote:
> top ... -> the top ...

Done.

[Takashi Toyoshima, 2017-02-06 09:13:42]

will apply some minor changes that were found by new "cl format" that forces to
reorder header files.

https://codereview.chromium.org/2654933003/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/CompositorWorkerTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/CompositorWorkerTest.cpp:5: #include
<gtest/gtest.h>
hum.... this should not be a system header!

https://codereview.chromium.org/2654933003/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/ListenerLeakTest.cpp (right):

https://codereview.chromium.org/2654933003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/ListenerLeakTest.cpp:31: #include
<v8/include/v8-profiler.h>
it seems we do not have "v8/include/" prefix usually for v8 headers.

[Takashi Toyoshima, 2017-02-06 09:15:40]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2017-02-06 09:15:40]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2654933003/#ps160001
(title: "header changes")

[commit-bot: I haz the power, 2017-02-06 09:15:53]

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-02-06 11:04:02]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1486372540142360,
"parent_rev": "0772dab70feef2c8a77628cf2476914b39d963b7", "commit_rev":
"8fabb1af2315f0b6fc685118b7c25b726bf3f94e"}

[commit-bot: I haz the power, 2017-02-06 11:04:55]

Description was changed from

==========
platform/testing/{URL|Unit}TestHelpers improvements

URLTestHelpers assumed that the test directory is web/tests/data.
But for blink_platform_tests, we didn't have a dependency to
web/tests/data in build rules, and should use platform/testing/data
instead of web/tests/data.

This patch revises URLTestHelpers so that it works without knowing
where the test directory is, and adds interfaces that provide
test directories for web and platform.

Also this patch does some refactoring changes against caller side
files so that these interface changes should not affect so much.

Some tests that directly had test directory path literals are
fixed to use UnitTestHelpers to obtain fullpaths for test files.

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

to

==========
platform/testing/{URL|Unit}TestHelpers improvements

URLTestHelpers assumed that the test directory is web/tests/data.
But for blink_platform_tests, we didn't have a dependency to
web/tests/data in build rules, and should use platform/testing/data
instead of web/tests/data.

This patch revises URLTestHelpers so that it works without knowing
where the test directory is, and adds interfaces that provide
test directories for web and platform.

Also this patch does some refactoring changes against caller side
files so that these interface changes should not affect so much.

Some tests that directly had test directory path literals are
fixed to use UnitTestHelpers to obtain fullpaths for test files.

BUG=655920

Review-Url: https://codereview.chromium.org/2654933003
Cr-Commit-Position: refs/heads/master@{#448227}
Committed:
https://chromium.googlesource.com/chromium/src/+/8fabb1af2315f0b6fc685118b7c2...
==========

[commit-bot: I haz the power, 2017-02-06 11:04:57]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/8fabb1af2315f0b6fc685118b7c2...