Remove NOTIFICATION_BROWSING_DATA_REMOVED

Remove NOTIFICATION_BROWSING_DATA_REMOVED

Replace the notification system with a static callback list in
BrowsingDataRemover.

BUG=chromium:268984
TEST=BrowsingDataRemoverTest and ExtensionBrowsingDataTest pass.

Signed-off-by: Simon Que <sque@chromium.org>;

Committed: https://crrev.com/df2e50a279c9cdc44d50ccb0e70b3e9bf289016d
Cr-Commit-Position: refs/heads/master@{#313137}

[Simon Que, 2015-01-24 02:20:28]

sque@chromium.org changed reviewers:
+ bauerb@chromium.org, caitkp@chromium.org, markusheintz@chromium.org,
michaeln@chromium.org, mkwst@chromium.org

[Bernhard Bauer, 2015-01-26 09:51:44]

FTR, it's customary to pick one reviewer from the OWNERS file, to avoid having
several people do the same work.

https://codereview.chromium.org/874613004/diff/1/chrome/browser/browsing_data...
File chrome/browser/browsing_data/browsing_data_remover.h (right):

https://codereview.chromium.org/874613004/diff/1/chrome/browser/browsing_data...
chrome/browser/browsing_data/browsing_data_remover.h:156: using CallbackList =
base::CallbackList<void(const NotificationDetails&)>;
You could move this typedef to the private section, as you don't need it from
another class.

https://codereview.chromium.org/874613004/diff/1/chrome/browser/browsing_data...
chrome/browser/browsing_data/browsing_data_remover.h:405: static CallbackList*
on_browsing_data_removed_callbacks() {
This is slightly more complex than a simple accessor, so it should be in
CamelCase and with the definition in the .cc file.

Actually, if it's private and static anyway, you could move it to an anonymous
namespace in the .cc file, together with |on_browsing_data_removed_callbacks_|.

[Simon Que, 2015-01-26 18:21:56]

https://codereview.chromium.org/874613004/diff/1/chrome/browser/browsing_data...
File chrome/browser/browsing_data/browsing_data_remover.h (right):

https://codereview.chromium.org/874613004/diff/1/chrome/browser/browsing_data...
chrome/browser/browsing_data/browsing_data_remover.h:156: using CallbackList =
base::CallbackList<void(const NotificationDetails&)>;
On 2015/01/26 09:51:43, Bernhard Bauer wrote:
> You could move this typedef to the private section, as you don't need it from
> another class.

Done.

https://codereview.chromium.org/874613004/diff/1/chrome/browser/browsing_data...
chrome/browser/browsing_data/browsing_data_remover.h:405: static CallbackList*
on_browsing_data_removed_callbacks() {
On 2015/01/26 09:51:43, Bernhard Bauer wrote:
> This is slightly more complex than a simple accessor, so it should be in
> CamelCase and with the definition in the .cc file.
> 
> Actually, if it's private and static anyway, you could move it to an anonymous
> namespace in the .cc file, together with
|on_browsing_data_removed_callbacks_|.

Done.

https://codereview.chromium.org/874613004/diff/1/chrome/browser/browsing_data...
chrome/browser/browsing_data/browsing_data_remover.h:405: static CallbackList*
on_browsing_data_removed_callbacks() {
On 2015/01/26 09:51:43, Bernhard Bauer wrote:
> This is slightly more complex than a simple accessor, so it should be in
> CamelCase and with the definition in the .cc file.
> 
> Actually, if it's private and static anyway, you could move it to an anonymous
> namespace in the .cc file, together with
|on_browsing_data_removed_callbacks_|.

Done.

[Bernhard Bauer, 2015-01-26 18:24:22]

LGTM with a naming nit:

https://codereview.chromium.org/874613004/diff/20001/chrome/browser/browsing_...
File chrome/browser/browsing_data/browsing_data_remover.cc (right):

https://codereview.chromium.org/874613004/diff/20001/chrome/browser/browsing_...
chrome/browser/browsing_data/browsing_data_remover.cc:110: CallbackList*
on_browsing_data_removed_callbacks_ = nullptr;
Name this |g_on_browsing_data_removed_callbacks| now that it's a global variable
instead of a member?

[Simon Que, 2015-01-26 18:43:00]

https://codereview.chromium.org/874613004/diff/20001/chrome/browser/browsing_...
File chrome/browser/browsing_data/browsing_data_remover.cc (right):

https://codereview.chromium.org/874613004/diff/20001/chrome/browser/browsing_...
chrome/browser/browsing_data/browsing_data_remover.cc:110: CallbackList*
on_browsing_data_removed_callbacks_ = nullptr;
On 2015/01/26 18:24:22, Bernhard Bauer wrote:
> Name this |g_on_browsing_data_removed_callbacks| now that it's a global
variable
> instead of a member?

Done.

[Simon Que, 2015-01-26 18:43:38]

The CQ bit was checked by sque@chromium.org

[commit-bot: I haz the power, 2015-01-26 18:44:07]

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

[Simon Que, 2015-01-26 18:49:42]

sque@chromium.org changed reviewers:
+ sky@chromium.org
- markusheintz@chromium.org, michaeln@chromium.org, mkwst@chromium.org

[Simon Que, 2015-01-26 18:49:43]

Adding sky to review chrome_notification_types.h

[commit-bot: I haz the power, 2015-01-26 18:52:40]

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

[commit-bot: I haz the power, 2015-01-26 18:52:40]

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

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

[sky, 2015-01-26 20:07:43]

LGTM

[Simon Que, 2015-01-26 21:14:04]

The CQ bit was checked by sque@chromium.org

[commit-bot: I haz the power, 2015-01-26 21:14:18]

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

[commit-bot: I haz the power, 2015-01-26 21:14:46]

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

[commit-bot: I haz the power, 2015-01-26 21:14:47]

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

[Simon Que, 2015-01-26 21:18:19]

The CQ bit was checked by sque@chromium.org

[commit-bot: I haz the power, 2015-01-26 21:19:15]

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

[commit-bot: I haz the power, 2015-01-26 21:20:33]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-01-26 21:21:35]

Patchset 3 (id:??) landed as
https://crrev.com/df2e50a279c9cdc44d50ccb0e70b3e9bf289016d
Cr-Commit-Position: refs/heads/master@{#313137}