Mandoline: let html_viewer handle more contents

Mandoline: let html_viewer handle more contents

- Let html_viewer handle 'text/plain'.
- When a URL fetch results in 4XX/5XX status code, don't drop the response.
  If the mime type is 'text/html' or 'text/plain', the response will be
  rendered by html_viewer.
- AboutFetcher returns 404 for about: URLs other than about:blank.

BUG=None

Committed: https://crrev.com/faf4512218eb08a205e31bde8b5dba84e9b58056
Cr-Commit-Position: refs/heads/master@{#349138}

[yzshen1, 2015-09-15 08:14:43]

yzshen@chromium.org changed reviewers:
+ sky@chromium.org

[yzshen1, 2015-09-15 08:14:43]

Hi, Scott.

Would you please take a look? Thanks!

This change prevents page_cycler.typical_25 from hanging after my load-event
fix.

We still need to handle the general problem that the page enters
permenantly-loading state when the shell drops subframe navigation requests. But
I feel that this fix is useful no matter we fix the general problem or not.

[sky, 2015-09-15 13:15:49]

This is subtle and I suspect will break without test coverage. Can you write
test coverage? Also, how are these handled now in Chrome? Do they show
interstitials, or does blink show the results?

[yzshen1, 2015-09-15 23:47:58]

On 2015/09/15 13:15:49, sky wrote:
> This is subtle and I suspect will break without test coverage. Can you write
> test coverage?
I have added tests.

> Also, how are these handled now in Chrome? Do they show
> interstitials, or does blink show the results?

Using html_viewer to handle 'text/plain' or 4xx/5xx response matches the chrome
behavior.
Returning 4xx for about: URLs other than about:blank is not exactly what chrome
does. I have added TODO and a bug link.

Would you please take another look? Thanks!

[sky, 2015-09-16 01:14:45]

LGTM

https://codereview.chromium.org/1342093002/diff/80001/mojo/fetcher/network_fe...
File mojo/fetcher/network_fetcher_unittest.cc (right):

https://codereview.chromium.org/1342093002/diff/80001/mojo/fetcher/network_fe...
mojo/fetcher/network_fetcher_unittest.cc:95: run_loop_ = &run_loop;
nit: base::AutoReset

https://codereview.chromium.org/1342093002/diff/80001/mojo/fetcher/network_fe...
mojo/fetcher/network_fetcher_unittest.cc:109: base::RunLoop* run_loop_;
document what where this comes from (especially ownership).

https://codereview.chromium.org/1342093002/diff/80001/mojo/fetcher/network_fe...
mojo/fetcher/network_fetcher_unittest.cc:159: TestFetchURL(k200Request, true,
200u);
I would separate each into its own test. That way if something fails it's easy
to see which code failed.

[yzshen1, 2015-09-16 05:24:54]

Thanks, Scott!

https://codereview.chromium.org/1342093002/diff/80001/mojo/fetcher/network_fe...
File mojo/fetcher/network_fetcher_unittest.cc (right):

https://codereview.chromium.org/1342093002/diff/80001/mojo/fetcher/network_fe...
mojo/fetcher/network_fetcher_unittest.cc:95: run_loop_ = &run_loop;
On 2015/09/16 01:14:45, sky wrote:
> nit: base::AutoReset

Done.

https://codereview.chromium.org/1342093002/diff/80001/mojo/fetcher/network_fe...
mojo/fetcher/network_fetcher_unittest.cc:109: base::RunLoop* run_loop_;
On 2015/09/16 01:14:45, sky wrote:
> document what where this comes from (especially ownership).

Done.

https://codereview.chromium.org/1342093002/diff/80001/mojo/fetcher/network_fe...
mojo/fetcher/network_fetcher_unittest.cc:159: TestFetchURL(k200Request, true,
200u);
On 2015/09/16 01:14:45, sky wrote:
> I would separate each into its own test. That way if something fails it's easy
> to see which code failed.

Done.

[yzshen1, 2015-09-16 05:25:46]

The CQ bit was checked by yzshen@chromium.org

[yzshen1, 2015-09-16 05:25:47]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1342093002/#ps100001
(title: " ")

[commit-bot: I haz the power, 2015-09-16 05:26:18]

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

[commit-bot: I haz the power, 2015-09-16 07:54:04]

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

[commit-bot: I haz the power, 2015-09-16 07:54:04]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[yzshen1, 2015-09-16 15:48:54]

The CQ bit was checked by yzshen@chromium.org

[commit-bot: I haz the power, 2015-09-16 15:49:12]

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

[commit-bot: I haz the power, 2015-09-16 16:19:06]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-09-16 16:19:47]

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

[commit-bot: I haz the power, 2015-09-23 12:56:34]

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