[Android O] Upstreaming notification channels code

[Android O] Upstreaming notification channels code

This is part one which moves where we create the notification builder
from AppHooksImpl to NotificationBuilderFactory.

The AppHooks method will be removed in a further patch.

BUG=702626
Review-Url: https://codereview.chromium.org/2765003002
Cr-Commit-Position: refs/heads/master@{#459060}
Committed: https://chromium.googlesource.com/chromium/src/+/a2876b06705e043a0781ca08019234e396140010

[awdf, 2017-03-21 18:03:23]

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

[commit-bot: I haz the power, 2017-03-21 18:04:23]

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-03-21 20:01:14]

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

[commit-bot: I haz the power, 2017-03-21 20:01:15]

Dry run: This issue passed the CQ dry run.

[awdf, 2017-03-22 11:22:16]

Description was changed from

==========
[Android O] Upstreaming notification channels code

This is part one which moves where we create the notification builder
from AppHooksImpl to NotificationBuilderFactory.

The AppHooks method will be removed in a further patch.

BUG=702626
==========

to

==========
[Android O] Upstreaming notification channels code

This is part one which moves where we create the notification builder
from AppHooksImpl to NotificationBuilderFactory.

The AppHooks method will be removed in a further patch.

BUG=702626
==========

[awdf, 2017-03-22 11:22:16]

awdf@chromium.org changed reviewers:
+ peter@chromium.org

[awdf, 2017-03-22 14:59:10]

peter - please review

[Peter Beverloo, 2017-03-22 15:08:56]

lgtm, would be good to file a bug for removing this altogether once we target 26

https://codereview.chromium.org/2765003002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderFactory.java
(right):

https://codereview.chromium.org/2765003002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderFactory.java:34:
*/
nit: spacing in the comments is a bit off (lines 30, 32 and 33)

https://codereview.chromium.org/2765003002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderFactory.java:43:
: new NotificationBuilder(context);
Don't we need to `import` those?

[awdf, 2017-03-22 17:24:51]

https://codereview.chromium.org/2765003002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderFactory.java
(right):

https://codereview.chromium.org/2765003002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderFactory.java:34:
*/
On 2017/03/22 15:08:56, Peter Beverloo wrote:
> nit: spacing in the comments is a bit off (lines 30, 32 and 33)

Done.

https://codereview.chromium.org/2765003002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderFactory.java:43:
: new NotificationBuilder(context);
On 2017/03/22 15:08:56, Peter Beverloo wrote:
> Don't we need to `import` those?

Nope since they're in the same package (they are our modded versions not
Android's).

[awdf, 2017-03-22 17:25:46]

The CQ bit was checked by awdf@chromium.org

[awdf, 2017-03-22 17:25:46]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org
Link to the patchset: https://codereview.chromium.org/2765003002/#ps40001
(title: "Add bug to TODO and fix spacing")

[commit-bot: I haz the power, 2017-03-22 17:26:05]

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

[awdf, 2017-03-22 17:31:45]

awdf@chromium.org changed reviewers:
+ mlamouri@chromium.org

[awdf, 2017-03-22 17:32:19]

awdf@chromium.org changed reviewers:
+ nyquist@chromium.org

[commit-bot: I haz the power, 2017-03-22 17:33:22]

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

[commit-bot: I haz the power, 2017-03-22 17:33:24]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[awdf, 2017-03-22 17:34:18]

awdf@chromium.org changed reviewers:
- mlamouri@chromium.org

[awdf, 2017-03-22 17:36:24]

+nyquist for owners review

[nyquist, 2017-03-22 23:16:53]

lgtm

https://codereview.chromium.org/2765003002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderForO.java
(right):

https://codereview.chromium.org/2765003002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderForO.java:24:
private static final String TAG = "NotifBuilderInternal";
Nit: This doesn't seem to match anymore.

https://codereview.chromium.org/2765003002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderForO.java:52:
// Create channel group
Nit: End sentence with period throughout here.

[awdf, 2017-03-23 11:53:05]

Thanks for the review! I will proceed to start landing these patches in
sequence.

https://codereview.chromium.org/2765003002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderForO.java
(right):

https://codereview.chromium.org/2765003002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderForO.java:24:
private static final String TAG = "NotifBuilderInternal";
On 2017/03/22 at 23:16:53, nyquist wrote:
> Nit: This doesn't seem to match anymore.

Good spot - Done.

[awdf, 2017-03-23 11:53:17]

The CQ bit was checked by awdf@chromium.org

[awdf, 2017-03-23 11:53:17]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org,
nyquist@chromium.org
Link to the patchset: https://codereview.chromium.org/2765003002/#ps60001
(title: "Rebase + update tag")

[commit-bot: I haz the power, 2017-03-23 11:53:36]

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-03-23 12:54:29]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490269997119430,
"parent_rev": "779245bdf7569d3b392a7f0bab3837dcb0722c48", "commit_rev":
"a2876b06705e043a0781ca08019234e396140010"}

[commit-bot: I haz the power, 2017-03-23 12:55:11]

Description was changed from

==========
[Android O] Upstreaming notification channels code

This is part one which moves where we create the notification builder
from AppHooksImpl to NotificationBuilderFactory.

The AppHooks method will be removed in a further patch.

BUG=702626
==========

to

==========
[Android O] Upstreaming notification channels code

This is part one which moves where we create the notification builder
from AppHooksImpl to NotificationBuilderFactory.

The AppHooks method will be removed in a further patch.

BUG=702626

Review-Url: https://codereview.chromium.org/2765003002
Cr-Commit-Position: refs/heads/master@{#459060}
Committed:
https://chromium.googlesource.com/chromium/src/+/a2876b06705e043a0781ca080192...
==========

[commit-bot: I haz the power, 2017-03-23 12:55:12]

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