Remove TopSites notification in favor of Observers

Remove TopSites notification in favor of Observers

Remove obsolete notification NOTIFICATION_TOP_SITES_LOADED as no code
was registering to receive it.

Port all clients of notification NOTIFICATION_TOP_SITES_CHANGED to
instead be history::TopSitesObserver and remove the notification that
is no longer useful.

ChromeHistoryClient is no longer a history::TopSitesObserver as it
does not have to convert the events into notification. Remove the
functionality and references to Profile and TopSites.

BUG=373329
Committed: https://crrev.com/edf9e01f8765025276e2c7090d49dde7afd24e18
Cr-Commit-Position: refs/heads/master@{#311311}

[sdefresne, 2015-01-10 00:59:11]

sdefresne@chromium.org changed reviewers:
+ droger@chromium.org

[sdefresne, 2015-01-10 00:59:12]

droger: please take a look.

[droger, 2015-01-12 10:06:49]

lgtm

[sdefresne, 2015-01-12 15:37:46]

sdefresne@chromium.org changed reviewers:
+ bauerb@chromium.org, erg@chromium.org, jcivelli@chromium.org,
samarth@chromium.org, sky@chromium.org

[sdefresne, 2015-01-12 15:37:46]

OWNERS please review the following directories/files

bauerb:
- //chrome/browser/android
- //chrome/browser/ui/webui/ntp

samarth:
- //chrome/browser/search

erg:
- //chrome/browser/ui/views/frame

sky:
- //chrome/browser/chrome_notification_types.h
- //chrome/browser/jumlist_win.{cc,h}

jcivelli:
- //chrome/browser/test/base

[Bernhard Bauer, 2015-01-12 15:44:03]

Android and WebUI LGTM with a nit:

https://codereview.chromium.org/845013002/diff/60001/chrome/browser/android/m...
File chrome/browser/android/most_visited_sites.h (right):

https://codereview.chromium.org/845013002/diff/60001/chrome/browser/android/m...
chrome/browser/android/most_visited_sites.h:98: // history::TopSites
implementation.
Nit: TopSitesObserver

[sky, 2015-01-12 17:42:30]

LGTM with the following changes.

https://codereview.chromium.org/845013002/diff/80001/chrome/browser/jumplist_...
File chrome/browser/jumplist_win.cc (right):

https://codereview.chromium.org/845013002/diff/80001/chrome/browser/jumplist_...
chrome/browser/jumplist_win.cc:610: history::TopSites* top_sites =
profile_->GetTopSites();
top_sites is supplied to this function, no need to look it up again. If you want
anything here, DCHECK the supplied top_sites equals that of
profile_->GetTopSites().

https://codereview.chromium.org/845013002/diff/80001/chrome/browser/jumplist_...
chrome/browser/jumplist_win.cc:611: if (top_sites) {
null-check shouldn't be necessary here. If you want anything, a DCHECK. But IMO
that's overkill.

[Jay Civelli, 2015-01-12 18:13:22]

LGTM for chrome/test/base/

[Elliot Glaysher, 2015-01-12 18:18:54]

lgtm for global menu bar

[sdefresne, 2015-01-13 10:23:46]

Thank you for the review.

samarth: ping

https://codereview.chromium.org/845013002/diff/80001/chrome/browser/jumplist_...
File chrome/browser/jumplist_win.cc (right):

https://codereview.chromium.org/845013002/diff/80001/chrome/browser/jumplist_...
chrome/browser/jumplist_win.cc:610: history::TopSites* top_sites =
profile_->GetTopSites();
On 2015/01/12 at 17:42:30, sky wrote:
> top_sites is supplied to this function, no need to look it up again. If you
want anything here, DCHECK the supplied top_sites equals that of
profile_->GetTopSites().

Nice catch!

https://codereview.chromium.org/845013002/diff/80001/chrome/browser/jumplist_...
chrome/browser/jumplist_win.cc:611: if (top_sites) {
On 2015/01/12 at 17:42:30, sky wrote:
> null-check shouldn't be necessary here. If you want anything, a DCHECK. But
IMO that's overkill.

Yeah, if does not make sense here. I didn't review the code after moving it from
the Observe() method. Fixed. Thank you.

[samarth, 2015-01-13 17:58:53]

lgtm

lgtm

[sdefresne, 2015-01-13 18:17:41]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2015-01-13 18:18:13]

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

[commit-bot: I haz the power, 2015-01-13 19:46:10]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-01-13 19:47:57]

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