Make WebURLLoaderMockFactoryImpl::createURLLoader accept nullptr

Make WebURLLoaderMockFactoryImpl::createURLLoader accept nullptr

Now a valid instance of WebURLLoader is required to create a
WebURLLoaderMock instance through createURLLoader() so that
it can have defaultLoader, a WebURLLoader for fallbacks.

But, in most cases, it can work without defaultLoader since
many tests are designed to use mocked URLs that would be
served from files directly without using defaultLoader.

In blink_platform_tests, default Platform::current() does not
return a valid instance for createURLLoader(). So allowing
nullptr would be useful.

BUG=655920
Review-Url: https://codereview.chromium.org/2657793002
Cr-Commit-Position: refs/heads/master@{#446316}
Committed: https://chromium.googlesource.com/chromium/src/+/0fa47a4664f18f8f4a2d28408ff4754eb600aaf4

[Takashi Toyoshima, 2017-01-25 06:57:36]

Patchset #2 (id:20001) has been deleted

[Takashi Toyoshima, 2017-01-25 07:00:48]

Description was changed from

==========
Make WebURLLoaderMock accept nullptr for defaultLoader

Now a valid instance of WebURLLoader is required to create a
WebURLLoaderMock instance so that it can have a WebURLLoader
that is used for fallbacks.

But, in most cases, it can work without defaultLoader since
many tests are designed to use mocked URLs that would be
served from files directly without using defaultLoader.

In blink_platform_tests, default Platform::current() does not
return a valid instance for createURLLoader(). So allowing
nullptr would be useful.

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

to

==========
Make WebURLLoaderMockFactoryImpl::createURLLoader accept nullptr

Now a valid instance of WebURLLoader is required to create a
WebURLLoaderMock instance through createURLLoader() so that
it can have defaultLoader, a WebURLLoader for fallbacks.

But, in most cases, it can work without defaultLoader since
many tests are designed to use mocked URLs that would be
served from files directly without using defaultLoader.

In blink_platform_tests, default Platform::current() does not
return a valid instance for createURLLoader(). So allowing
nullptr would be useful.

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

[Takashi Toyoshima, 2017-01-25 07:00:49]

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

[Takashi Toyoshima, 2017-01-25 07:00:57]

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

[commit-bot: I haz the power, 2017-01-25 07:01:09]

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-25 07:01:36]

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

[Takashi Toyoshima, 2017-01-25 07:01:44]

ptal

[yhirano, 2017-01-25 08:16:04]

We talked about moving |url_loader_factory_| out of 
TestBlinkWebUnitTestSupport. With that change we may be able to remove
|defaultLoader| parameter. What do you think?

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

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

[commit-bot: I haz the power, 2017-01-25 09:12:15]

Dry run: This issue passed the CQ dry run.

[Takashi Toyoshima, 2017-01-25 10:34:36]

I tried it today, but I noticed that it makes many webkit_unit_tests fail.
So, even if we remove it from TestBlinkWebUnitTestSupport, we need to have a
TestingPlatformSupport in blink side that provides a WebURLLoaderMock with a
defaultLoader. That means we would still need defaultLoader after that cleanup
change.

[kinuko, 2017-01-25 12:42:13]

ok, lgtm

https://codereview.chromium.org/2657793002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/testing/WebURLLoaderMock.cpp (right):

https://codereview.chromium.org/2657793002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/WebURLLoaderMock.cpp:17: void
IsFallbackLoaderAvailable(const WebURL& url,
I'd name this 'assertFallbackLoaderAvailability()'

[Takashi Toyoshima, 2017-01-26 11:54:24]

https://codereview.chromium.org/2657793002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/testing/WebURLLoaderMock.cpp (right):

https://codereview.chromium.org/2657793002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/testing/WebURLLoaderMock.cpp:17: void
IsFallbackLoaderAvailable(const WebURL& url,
On 2017/01/25 12:42:13, kinuko wrote:
> I'd name this 'assertFallbackLoaderAvailability()'

Done.

[Takashi Toyoshima, 2017-01-26 11:59:11]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2017-01-26 11:59:12]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org
Link to the patchset: https://codereview.chromium.org/2657793002/#ps80001
(title: "cl format")

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

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 13:32:30]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1485431951663280,
"parent_rev": "cc5bb40aed5443930544af6292de928234edb6b2", "commit_rev":
"0fa47a4664f18f8f4a2d28408ff4754eb600aaf4"}

[commit-bot: I haz the power, 2017-01-26 13:33:04]

Description was changed from

==========
Make WebURLLoaderMockFactoryImpl::createURLLoader accept nullptr

Now a valid instance of WebURLLoader is required to create a
WebURLLoaderMock instance through createURLLoader() so that
it can have defaultLoader, a WebURLLoader for fallbacks.

But, in most cases, it can work without defaultLoader since
many tests are designed to use mocked URLs that would be
served from files directly without using defaultLoader.

In blink_platform_tests, default Platform::current() does not
return a valid instance for createURLLoader(). So allowing
nullptr would be useful.

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

to

==========
Make WebURLLoaderMockFactoryImpl::createURLLoader accept nullptr

Now a valid instance of WebURLLoader is required to create a
WebURLLoaderMock instance through createURLLoader() so that
it can have defaultLoader, a WebURLLoader for fallbacks.

But, in most cases, it can work without defaultLoader since
many tests are designed to use mocked URLs that would be
served from files directly without using defaultLoader.

In blink_platform_tests, default Platform::current() does not
return a valid instance for createURLLoader(). So allowing
nullptr would be useful.

BUG=655920

Review-Url: https://codereview.chromium.org/2657793002
Cr-Commit-Position: refs/heads/master@{#446316}
Committed:
https://chromium.googlesource.com/chromium/src/+/0fa47a4664f18f8f4a2d28408ff4...
==========

[commit-bot: I haz the power, 2017-01-26 13:33:05]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/0fa47a4664f18f8f4a2d28408ff4...

[yhirano, 2017-01-27 04:41:41]

Please fix WebURLLoaderMockFactory::createURLLoader comment.