Merge the GoogleServicesNotificationController to SyncNotificationController

Merge the GoogleServicesNotificationController to SyncNotificationController

The SyncNotificationController is the only consumer of the GSNC, so we
can simplify by merging the two classes together. In addition, remove
displayAndroidMasterSyncDisabledNotification() which had no callers.

The sync notifications do not appear to be tested. I filed
https://crbug.com/679750 to track adding some.

BUG=

Review-Url: https://codereview.chromium.org/2621023002
Cr-Commit-Position: refs/heads/master@{#442992}
Committed: https://chromium.googlesource.com/chromium/src/+/2e36b96ae1199d1027734fb870f6c76c0630f410

[Peter Beverloo, 2017-01-10 15:21:08]

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

[commit-bot: I haz the power, 2017-01-10 15:21:26]

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

[Peter Beverloo, 2017-01-10 15:24:57]

Description was changed from

==========
Use a notification group for sync-related notifications

Additionally, merge the functionality of
GoogleServicesNotificationController in to SyncNotificationController
and delete the displayAndroidMasterSyncDisabledNotification() method,
which has no callers.

BUG=674927
==========

to

==========
Use a notification group for sync-related notifications

Additionally, merge the functionality of
GoogleServicesNotificationController in to SyncNotificationController
and delete the displayAndroidMasterSyncDisabledNotification() method,
which has no callers.

The sync notifications do not appear to be tested. I filed
https://crbug.com/679750 to track adding some.

BUG=674927
==========

[awdf, 2017-01-10 15:53:59]

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

[awdf, 2017-01-10 15:53:59]

lgtm % splitting into two commits

https://codereview.chromium.org/2621023002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/sync/SyncNotificationController.java
(right):

https://codereview.chromium.org/2621023002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/sync/SyncNotificationController.java:55:
public void syncStateChanged() {
nit: This method is getting over-long. Consider splitting lines 91-109 into a
separate 'showSyncNotification' method or similar.

https://codereview.chromium.org/2621023002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/sync/SyncNotificationController.java:104:
.setGroup(NotificationConstants.GROUP_SYNC);
It would be great if this line could be a separate commit to keep refactoring
separate from functional change.

https://codereview.chromium.org/2621023002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/sync/SyncNotificationController.java:104:
.setGroup(NotificationConstants.GROUP_SYNC);
nit: Add a comment saying why we're not generating a summary notification for
the group.

[commit-bot: I haz the power, 2017-01-10 16:20:34]

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

[commit-bot: I haz the power, 2017-01-10 16:20:34]

Dry run: This issue passed the CQ dry run.

[Peter Beverloo, 2017-01-11 18:47:36]

Patchset #2 (id:20001) has been deleted

[Peter Beverloo, 2017-01-11 18:49:27]

Description was changed from

==========
Use a notification group for sync-related notifications

Additionally, merge the functionality of
GoogleServicesNotificationController in to SyncNotificationController
and delete the displayAndroidMasterSyncDisabledNotification() method,
which has no callers.

The sync notifications do not appear to be tested. I filed
https://crbug.com/679750 to track adding some.

BUG=674927
==========

to

==========
Merge the GoogleServicesNotificationController to SyncNotificationController

The SyncNotificationController is the only consumer of the GSNC, so we
can simplify by merging the two classes together. In addition, remove
displayAndroidMasterSyncDisabledNotification() which had no callers.

The sync notifications do not appear to be tested. I filed
https://crbug.com/679750 to track adding some.

BUG=
==========

[Peter Beverloo, 2017-01-11 18:50:41]

peter@chromium.org changed reviewers:
+ maxbogue@chromium.org, nyquist@chromium.org

[Peter Beverloo, 2017-01-11 18:50:41]

Thanks Anita! I'll send another CL to add the group.

+maxbogue and/or nyquist for review

https://codereview.chromium.org/2621023002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/sync/SyncNotificationController.java
(right):

https://codereview.chromium.org/2621023002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/sync/SyncNotificationController.java:55:
public void syncStateChanged() {
On 2017/01/10 15:53:59, awdf wrote:
> nit: This method is getting over-long. Consider splitting lines 91-109 into a
> separate 'showSyncNotification' method or similar.

Done.

https://codereview.chromium.org/2621023002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/sync/SyncNotificationController.java:104:
.setGroup(NotificationConstants.GROUP_SYNC);
On 2017/01/10 15:53:59, awdf wrote:
> nit: Add a comment saying why we're not generating a summary notification for
> the group.

Acknowledged.

https://codereview.chromium.org/2621023002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/sync/SyncNotificationController.java:104:
.setGroup(NotificationConstants.GROUP_SYNC);
On 2017/01/10 15:53:59, awdf wrote:
> It would be great if this line could be a separate commit to keep refactoring
> separate from functional change.

Done.

[Peter Beverloo, 2017-01-11 18:54:45]

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

[commit-bot: I haz the power, 2017-01-11 18:55:31]

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

[nyquist, 2017-01-11 18:56:35]

lgtm

[commit-bot: I haz the power, 2017-01-11 20:01:53]

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

[commit-bot: I haz the power, 2017-01-11 20:01:54]

Dry run: This issue passed the CQ dry run.

[Peter Beverloo, 2017-01-11 20:16:08]

The CQ bit was checked by peter@chromium.org

[Peter Beverloo, 2017-01-11 20:16:09]

The patchset sent to the CQ was uploaded after l-g-t-m from awdf@chromium.org
Link to the patchset: https://codereview.chromium.org/2621023002/#ps40001
(title: "Use a notification group for sync-related notifications")

[commit-bot: I haz the power, 2017-01-11 20:16:32]

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-11 20:51:09]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484165768604930,
"parent_rev": "b38f05eb1dcc47b3d4fbe79f4a5deeaca07f0ef3", "commit_rev":
"2e36b96ae1199d1027734fb870f6c76c0630f410"}

[commit-bot: I haz the power, 2017-01-11 20:51:44]

Description was changed from

==========
Merge the GoogleServicesNotificationController to SyncNotificationController

The SyncNotificationController is the only consumer of the GSNC, so we
can simplify by merging the two classes together. In addition, remove
displayAndroidMasterSyncDisabledNotification() which had no callers.

The sync notifications do not appear to be tested. I filed
https://crbug.com/679750 to track adding some.

BUG=
==========

to

==========
Merge the GoogleServicesNotificationController to SyncNotificationController

The SyncNotificationController is the only consumer of the GSNC, so we
can simplify by merging the two classes together. In addition, remove
displayAndroidMasterSyncDisabledNotification() which had no callers.

The sync notifications do not appear to be tested. I filed
https://crbug.com/679750 to track adding some.

BUG=

Review-Url: https://codereview.chromium.org/2621023002
Cr-Commit-Position: refs/heads/master@{#442992}
Committed:
https://chromium.googlesource.com/chromium/src/+/2e36b96ae1199d1027734fb870f6...
==========

[commit-bot: I haz the power, 2017-01-11 20:51:51]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/2e36b96ae1199d1027734fb870f6...