[Offline Pages] Add an observer for requests in the CCT namespace.

[Offline Pages] Add an observer for requests in the CCT namespace.

This will allow us to track when requests finish.  Adds a hook so that
internal code can have different behavior.  See bug for details.

BUG=720782
Review-Url: https://codereview.chromium.org/2888273003
Cr-Commit-Position: refs/heads/master@{#473994}
Committed: https://chromium.googlesource.com/chromium/src/+/471379a5598eea23b5514740f17ce66b4a76bb68

[dewittj, 2017-05-18 20:39:14]

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

[dewittj, 2017-05-18 20:40:33]

dewittj@chromium.org changed reviewers:
+ chili@chromium.org

[dewittj, 2017-05-18 20:40:34]

+chili - PTAL, thanks!

[commit-bot: I haz the power, 2017-05-18 20:40:36]

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-05-18 21:55:41]

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

[commit-bot: I haz the power, 2017-05-18 21:55:42]

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_...)

[chili, 2017-05-23 01:04:00]

Sorry for taking so long to look :)

lgtm with minor comment.

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

https://codereview.chromium.org/2888273003/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/cct_request_observer.cc:54: env,
as_jint((int)status),
what's the difference between static_cast<int> and simply casting as int?

[dewittj, 2017-05-23 15:57:14]

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

[commit-bot: I haz the power, 2017-05-23 15:57:46]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dewittj, 2017-05-23 16:03:04]

dewittj@chromium.org changed reviewers:
+ dtrainor@chromium.org

[dewittj, 2017-05-23 16:03:05]

+dtrainor for AppHooks

Thanks!

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

https://codereview.chromium.org/2888273003/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/cct_request_observer.cc:54: env,
as_jint((int)status),
On 2017/05/23 01:04:00, chili wrote:
> what's the difference between static_cast<int> and simply casting as int?

You are right that I should have used static_cast.  I think int is sometimes
problematic because it could mean any of static_cast, const_cast, or
reinterpret_cast.  So we are typically explicit in Chromium.

[David Trainor- moved to gerrit, 2017-05-23 16:53:14]

lgtm

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

https://codereview.chromium.org/2888273003/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/cct_request_observer.cc:54: env,
as_jint((int)status),
On 2017/05/23 16:03:05, dewittj wrote:
> On 2017/05/23 01:04:00, chili wrote:
> > what's the difference between static_cast<int> and simply casting as int?
> 
> You are right that I should have used static_cast.  I think int is sometimes
> problematic because it could mean any of static_cast, const_cast, or
> reinterpret_cast.  So we are typically explicit in Chromium.

Yeah IIUC static_cast<> is a more restrictive form of C-style cast and can fail
at compile time.  C-style casts can potentially do bad things at runtime.

https://codereview.chromium.org/2888273003/diff/20001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/AppHooks.java (right):

https://codereview.chromium.org/2888273003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/AppHooks.java:298: public
Callback<CCTRequestStatus> getOfflinePagesCCTRequestDoneCallback() {
Just going to suggest adding a javadoc, even though the name is damn descriptive
:D!

[dewittj, 2017-05-23 17:02:26]

The CQ bit was checked by dewittj@chromium.org

[dewittj, 2017-05-23 17:02:27]

The patchset sent to the CQ was uploaded after l-g-t-m from chili@chromium.org,
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2888273003/#ps60001
(title: "comments.")

[dewittj, 2017-05-23 17:02:50]

done. Thanks!

https://codereview.chromium.org/2888273003/diff/20001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/AppHooks.java (right):

https://codereview.chromium.org/2888273003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/AppHooks.java:298: public
Callback<CCTRequestStatus> getOfflinePagesCCTRequestDoneCallback() {
On 2017/05/23 16:53:14, David Trainor-ping if over 24h wrote:
> Just going to suggest adding a javadoc, even though the name is damn
descriptive
> :D!

Done.

[commit-bot: I haz the power, 2017-05-23 17:03:14]

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-05-23 18:35:27]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1495558946985630,
"parent_rev": "ab17ae0f775e0ebcbd437ec23064eb0e8bb7bd7b", "commit_rev":
"471379a5598eea23b5514740f17ce66b4a76bb68"}

[commit-bot: I haz the power, 2017-05-23 18:35:42]

Description was changed from

==========
[Offline Pages] Add an observer for requests in the CCT namespace.

This will allow us to track when requests finish.  Adds a hook so that
internal code can have different behavior.  See bug for details.

BUG=720782
==========

to

==========
[Offline Pages] Add an observer for requests in the CCT namespace.

This will allow us to track when requests finish.  Adds a hook so that
internal code can have different behavior.  See bug for details.

BUG=720782

Review-Url: https://codereview.chromium.org/2888273003
Cr-Commit-Position: refs/heads/master@{#473994}
Committed:
https://chromium.googlesource.com/chromium/src/+/471379a5598eea23b5514740f17c...
==========

[commit-bot: I haz the power, 2017-05-23 18:35:43]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/471379a5598eea23b5514740f17c...