[Offline pages] Update background loader offliner to return error when

[Offline pages] Update background loader offliner to return error when
renderer crashes or when the internal contents is destroyed

BUG=665242
Committed: https://crrev.com/b13fb4e397a21917ac67a38dc7d2312e2221aac8
Cr-Commit-Position: refs/heads/master@{#441168}

[chili, 2016-12-21 18:04:03]

chili@chromium.org changed reviewers:
+ dougarnett@chromium.org, petewil@chromium.org

[chili, 2016-12-21 18:04:03]

Small update to possible "failed" loading scenarios.  Unfortunately I have not
yet found a good way to trigger these scenarios from unit tests.  Will keep
looking, but may defer unit tests until later

[Pete Williamson, 2016-12-21 20:49:29]

https://codereview.chromium.org/2596913002/diff/1/chrome/browser/android/offl...
File chrome/browser/android/offline_pages/background_loader_offliner.cc (right):

https://codereview.chromium.org/2596913002/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/background_loader_offliner.cc:100: void
BackgroundLoaderOffliner::RenderProcessGone(
What would cause a call to RenderProcessGone?  I didn't see it get called
anywhere.

[Pete Williamson, 2016-12-21 20:59:00]

The CQ bit was checked by petewil@chromium.org

[Pete Williamson, 2016-12-21 20:59:20]

The CQ bit was unchecked by petewil@chromium.org

[chili, 2016-12-21 23:40:40]

On 2016/12/21 20:49:29, Pete Williamson wrote:
>
https://codereview.chromium.org/2596913002/diff/1/chrome/browser/android/offl...
> File chrome/browser/android/offline_pages/background_loader_offliner.cc
(right):
> 
>
https://codereview.chromium.org/2596913002/diff/1/chrome/browser/android/offl...
> chrome/browser/android/offline_pages/background_loader_offliner.cc:100: void
> BackgroundLoaderOffliner::RenderProcessGone(
> What would cause a call to RenderProcessGone?  I didn't see it get called
> anywhere.

RenderProcessGone is initiated by the
RenderProcessHostObserver::RenderProcessExited().  The render process may be
killed because out of memory, something causing the render process to crash,
chrome getting shut down, or some other reason.  This is overriding one of the
webcontents observer methods

[Pete Williamson, 2016-12-21 23:50:32]

lgtm

[dougarnett, 2016-12-22 01:37:53]

As for testing, why not call RenderProcessGone() or WebContentsDestroyed() on
the BackgroundLoaderOffliner to simulate a failure?

https://codereview.chromium.org/2596913002/diff/1/chrome/browser/android/offl...
File chrome/browser/android/offline_pages/background_loader_offliner.cc (right):

https://codereview.chromium.org/2596913002/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/background_loader_offliner.cc:110: case
TERMINATION_STATUS_PROCESS_WAS_KILLED:
Can you share any insight on this case? Cleanly killed so try something else?

[chili, 2016-12-31 23:54:04]

Sorry for delay. Took a few tries to get the tests right

PTAL

[dougarnett, 2017-01-03 17:06:08]

lgtm

Thanks for adding the tests

[chili, 2017-01-03 18:06:41]

The CQ bit was checked by chili@chromium.org

[chili, 2017-01-03 18:06:42]

The patchset sent to the CQ was uploaded after l-g-t-m from petewil@chromium.org
Link to the patchset: https://codereview.chromium.org/2596913002/#ps20001
(title: "tests and code review")

[commit-bot: I haz the power, 2017-01-03 18:07:04]

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-03 18:36:24]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1483466801646130,
"parent_rev": "f8588e00f08a152adaa0335281b9aac6bb039a82", "commit_rev":
"8d01ba69ba16a7ffa2cde9da6fb18571cd80a9a6"}

[commit-bot: I haz the power, 2017-01-03 18:36:55]

Description was changed from

==========
[Offline pages] Update background loader offliner to return error when
renderer crashes or when the internal contents is destroyed

BUG=665242
==========

to

==========
[Offline pages] Update background loader offliner to return error when
renderer crashes or when the internal contents is destroyed

BUG=665242

Review-Url: https://codereview.chromium.org/2596913002
==========

[commit-bot: I haz the power, 2017-01-03 18:36:56]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2017-01-03 18:40:19]

Description was changed from

==========
[Offline pages] Update background loader offliner to return error when
renderer crashes or when the internal contents is destroyed

BUG=665242

Review-Url: https://codereview.chromium.org/2596913002
==========

to

==========
[Offline pages] Update background loader offliner to return error when
renderer crashes or when the internal contents is destroyed

BUG=665242

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

[commit-bot: I haz the power, 2017-01-03 18:40:20]

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

[Pete Williamson, 2017-01-03 20:51:14]

lgtm