[Offline pages] Refactoring and code fix in BackgroundOffinerTask

[Offline pages] Refactoring and code fix in BackgroundOffinerTask

This patch deals with 2 issues:
* We need to simplify the logic in BackgroundOffinerTask to reuse it
  when converting background work to JobService for O,
* It turns out that the waiter provided by ChromeBackgroundService
  was not released by the BackgroundOffinerTask in cases when device
  conditions were not met,
* Relevant test are updated: wait is added to make sure lock is
  released in all cases,
* Additionally tests are refactored to apply to new interface:
  boolean result of startBackgroundRequests was never used, hence it was
  removed and replaced with proper lock release.

BUG=690322, 683802
Review-Url: https://codereview.chromium.org/2685873003
Cr-Commit-Position: refs/heads/master@{#449502}
Committed: https://chromium.googlesource.com/chromium/src/+/4f0be6ea9ee6b8f44a56673a26996a767fd4539c

[fgorski, 2017-02-09 06:05:50]

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

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

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

[fgorski, 2017-02-09 06:06:45]

fgorski@chromium.org changed reviewers:
+ petewil@chromium.org

[fgorski, 2017-02-09 06:06:46]

PTAL. This is first step to make it work on O.

[commit-bot: I haz the power, 2017-02-09 06:41:40]

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

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

Dry run: This issue passed the CQ dry run.

[Pete Williamson, 2017-02-09 21:23:43]

lgtm with comment change

https://codereview.chromium.org/2685873003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundOfflinerTask.java
(right):

https://codereview.chromium.org/2685873003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundOfflinerTask.java:86:
// TODO(petewil): Nothing is holding the Wake Lock.  We need some solution to
While we are in here, let's update this comment.  We are actually holding the
wake lock by not returning.

[Pete Williamson, 2017-02-09 21:23:43]

lgtm with comment change

https://codereview.chromium.org/2685873003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundOfflinerTask.java
(right):

https://codereview.chromium.org/2685873003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundOfflinerTask.java:86:
// TODO(petewil): Nothing is holding the Wake Lock.  We need some solution to
While we are in here, let's update this comment.  We are actually holding the
wake lock by not returning.

[fgorski, 2017-02-09 23:56:20]

Updated

https://codereview.chromium.org/2685873003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundOfflinerTask.java
(right):

https://codereview.chromium.org/2685873003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundOfflinerTask.java:86:
// TODO(petewil): Nothing is holding the Wake Lock.  We need some solution to
On 2017/02/09 21:23:43, Pete Williamson wrote:
> While we are in here, let's update this comment.  We are actually holding the
> wake lock by not returning.

Done.

[fgorski, 2017-02-09 23:56:24]

The CQ bit was checked by fgorski@chromium.org

[fgorski, 2017-02-09 23:56:25]

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/2685873003/#ps20001
(title: "Addressing CR feedback: comment updated")

[commit-bot: I haz the power, 2017-02-09 23:56:52]

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-10 01:22:06]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486684584728280,
"parent_rev": "21f5e38c079d7bed76f88b5ca25b0bb142d45aff", "commit_rev":
"4f0be6ea9ee6b8f44a56673a26996a767fd4539c"}

[commit-bot: I haz the power, 2017-02-10 01:23:20]

Description was changed from

==========
[Offline pages] Refactoring and code fix in BackgroundOffinerTask

This patch deals with 2 issues:
* We need to simplify the logic in BackgroundOffinerTask to reuse it
  when converting background work to JobService for O,
* It turns out that the waiter provided by ChromeBackgroundService
  was not released by the BackgroundOffinerTask in cases when device
  conditions were not met,
* Relevant test are updated: wait is added to make sure lock is
  released in all cases,
* Additionally tests are refactored to apply to new interface:
  boolean result of startBackgroundRequests was never used, hence it was
  removed and replaced with proper lock release.

BUG=690322,683802
==========

to

==========
[Offline pages] Refactoring and code fix in BackgroundOffinerTask

This patch deals with 2 issues:
* We need to simplify the logic in BackgroundOffinerTask to reuse it
  when converting background work to JobService for O,
* It turns out that the waiter provided by ChromeBackgroundService
  was not released by the BackgroundOffinerTask in cases when device
  conditions were not met,
* Relevant test are updated: wait is added to make sure lock is
  released in all cases,
* Additionally tests are refactored to apply to new interface:
  boolean result of startBackgroundRequests was never used, hence it was
  removed and replaced with proper lock release.

BUG=690322,683802

Review-Url: https://codereview.chromium.org/2685873003
Cr-Commit-Position: refs/heads/master@{#449502}
Committed:
https://chromium.googlesource.com/chromium/src/+/4f0be6ea9ee6b8f44a56673a2699...
==========

[commit-bot: I haz the power, 2017-02-10 01:23:21]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/4f0be6ea9ee6b8f44a56673a2699...