Delete deprecated synced notifications code

Delete deprecated synced notifications code

Synced notifications no longer have native code. Delete the remaining code
(which has not been used for serveral milestones).

BUG=439577
R=kalman@chromium.org, petewil@chromium.org, stanisc@chromium.org
TBR=dewittj@chromium.org

Committed: https://chromium.googlesource.com/chromium/src/+/92a506ded2abce9f58c75232da7ab48c83652449

[Nicolas Zea, 2014-12-05 21:21:58]

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

[Nicolas Zea, 2014-12-05 21:21:59]

+Pete, PTAL. I think I covered all the code, but let me know if I missed
something or was being too aggressive.

[Nicolas Zea, 2014-12-05 21:23:33]

zea@chromium.org changed reviewers:
+ stanisc@chromium.org

[Nicolas Zea, 2014-12-05 21:23:33]

+Stan for sync code.

[stanisc, 2014-12-05 23:19:13]

Do we still want to show these 3 deprecated types on about:sync?
What about some unit tests that explicitly use SYNCED_NOTIFICATIONS like
directory_update_handler_unittest.cc?
There is also a reference to SYNCED_NOTIFICATIONS in places like model_type.cc
and user_selectable_sync_type.h. Should we keep those?

https://codereview.chromium.org/780363003/diff/1/sync/protocol/synced_notific...
File sync/protocol/synced_notification_app_info_specifics.proto (right):

https://codereview.chromium.org/780363003/diff/1/sync/protocol/synced_notific...
sync/protocol/synced_notification_app_info_specifics.proto:13: message
SyncedNotificationAppInfoSpecifics {
Do we keep this for backwards compatibility?
Consider adding a comment.

[Nicolas Zea, 2014-12-05 23:25:47]

On 2014/12/05 23:19:13, stanisc wrote:
> Do we still want to show these 3 deprecated types on about:sync?
> What about some unit tests that explicitly use SYNCED_NOTIFICATIONS like
> directory_update_handler_unittest.cc?
> There is also a reference to SYNCED_NOTIFICATIONS in places like model_type.cc
> and user_selectable_sync_type.h. Should we keep those?
> 
>
https://codereview.chromium.org/780363003/diff/1/sync/protocol/synced_notific...
> File sync/protocol/synced_notification_app_info_specifics.proto (right):
> 
>
https://codereview.chromium.org/780363003/diff/1/sync/protocol/synced_notific...
> sync/protocol/synced_notification_app_info_specifics.proto:13: message
> SyncedNotificationAppInfoSpecifics {
> Do we keep this for backwards compatibility?
> Consider adding a comment.

They'll show if they're enabled, but it shouldn't be possible for them to be
enabled anymore. Typically we've kept around the deprecated datatype info in
model_type.cc for the sake of some measure of backwards compatibility (i.e. if
the server starts sending back some response related to the datatype, the client
can recognize which type it is, even if it shouldn't be running). I'll update
the specifics to make that more clear.

[Nicolas Zea, 2014-12-09 00:46:14]

Updated comments, PTAL

[stanisc, 2014-12-09 01:48:16]

lgtm

[Pete Williamson, 2014-12-10 22:10:05]

Looks good overall.  I did have two quick questions.

https://codereview.chromium.org/780363003/diff/20001/sync/internal_api/public...
File sync/internal_api/public/base/model_type.h (right):

https://codereview.chromium.org/780363003/diff/20001/sync/internal_api/public...
sync/internal_api/public/base/model_type.h:83: APP_NOTIFICATIONS,  //
Deprecated.
You know that this is not related to synced notifications, right?  It is OK if
you are deprecating it as part of cleanup, but does not strictly need to be
removed for synced notifications.

https://codereview.chromium.org/780363003/diff/20001/sync/protocol/sync.proto
File sync/protocol/sync.proto (right):

https://codereview.chromium.org/780363003/diff/20001/sync/protocol/sync.proto...
sync/protocol/sync.proto:119: optional SyncedNotificationAppInfoSpecifics
synced_notification_app_info = 235816;
Should this line be removed, or does it need to stay for historical purposes?

[Nicolas Zea, 2014-12-10 22:26:02]

https://codereview.chromium.org/780363003/diff/20001/sync/internal_api/public...
File sync/internal_api/public/base/model_type.h (right):

https://codereview.chromium.org/780363003/diff/20001/sync/internal_api/public...
sync/internal_api/public/base/model_type.h:83: APP_NOTIFICATIONS,  //
Deprecated.
On 2014/12/10 22:10:05, Pete Williamson wrote:
> You know that this is not related to synced notifications, right?  It is OK if
> you are deprecating it as part of cleanup, but does not strictly need to be
> removed for synced notifications.

Yeah, this is an old datatype that is no longer being used, so figured I'd
update it too.

https://codereview.chromium.org/780363003/diff/20001/sync/protocol/sync.proto
File sync/protocol/sync.proto (right):

https://codereview.chromium.org/780363003/diff/20001/sync/protocol/sync.proto...
sync/protocol/sync.proto:119: optional SyncedNotificationAppInfoSpecifics
synced_notification_app_info = 235816;
On 2014/12/10 22:10:05, Pete Williamson wrote:
> Should this line be removed, or does it need to stay for historical purposes?

I'm keeping it around for historical purposes.

[Pete Williamson, 2014-12-10 22:32:53]

lgtm

[Nicolas Zea, 2014-12-10 23:05:45]

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

[Nicolas Zea, 2014-12-10 23:06:09]

+TBR Justin for notifications dead code removal, committing via bot.

[Nicolas Zea, 2014-12-10 23:06:13]

The CQ bit was checked by zea@chromium.org

[commit-bot: I haz the power, 2014-12-10 23:07:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/780363003/20001

[commit-bot: I haz the power, 2014-12-10 23:13:25]

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

[commit-bot: I haz the power, 2014-12-10 23:13:26]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Nicolas Zea, 2014-12-11 01:02:10]

The CQ bit was checked by zea@chromium.org

[commit-bot: I haz the power, 2014-12-11 01:03:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/780363003/40001

[commit-bot: I haz the power, 2014-12-11 01:25:54]

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

[commit-bot: I haz the power, 2014-12-11 01:25:55]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Nicolas Zea, 2014-12-11 01:39:44]

zea@chromium.org changed reviewers:
+ kalman@chromium.org

[Nicolas Zea, 2014-12-11 01:39:46]

+Ben for permissions updates (removing private SyncedNotification api +
permissions). Let me know if I missed anything.

[not at google - send to devlin, 2014-12-11 17:31:34]

You also need to delete:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/common/exte...

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/common/exte...

That should be it, lgtm.

[not at google - send to devlin, 2014-12-11 17:32:38]

Oh, and rename these:

https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...

[Nicolas Zea, 2014-12-11 18:34:03]

Kalman@'s comments addressed. Gogo commitbot.

[Nicolas Zea, 2014-12-11 18:34:30]

The CQ bit was checked by zea@chromium.org

[commit-bot: I haz the power, 2014-12-11 18:35:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/780363003/80001

[commit-bot: I haz the power, 2014-12-11 18:49:22]

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

[commit-bot: I haz the power, 2014-12-11 18:49:24]

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

[commit-bot: I haz the power, 2014-12-11 19:27:15]

Patchset 5 (id:??) landed as
https://crrev.com/92a506ded2abce9f58c75232da7ab48c83652449
Cr-Commit-Position: refs/heads/master@{#307946}

[Nicolas Zea, 2014-12-11 19:27:24]

Committed patchset #5 (id:80001) manually as
92a506ded2abce9f58c75232da7ab48c83652449 (presubmit successful).