PopularSites: don't wait for a SuggestionsService network request

PopularSites: don't wait for a SuggestionsService network request

BUG=607615
Committed: https://crrev.com/c1bd1f1093db2d9aada0cb4dd02c95c80f48ea55
Cr-Commit-Position: refs/heads/master@{#390648}

[Marc Treib, 2016-04-29 13:58:32]

treib@chromium.org changed reviewers:
+ bauerb@chromium.org

[Marc Treib, 2016-04-29 13:58:33]

PTAL!

https://codereview.chromium.org/1934583002/diff/1/chrome/browser/android/ntp/...
File chrome/browser/android/ntp/most_visited_sites.cc (left):

https://codereview.chromium.org/1934583002/diff/1/chrome/browser/android/ntp/...
chrome/browser/android/ntp/most_visited_sites.cc:365: return;
This was the actual problem - we'd wait on the network request.
In fact, there's no reason to even send a network request at this point, so I've
just removed it.

https://codereview.chromium.org/1934583002/diff/1/chrome/browser/android/ntp/...
chrome/browser/android/ntp/most_visited_sites.cc:761: if (!observer_)
|observer_| can't be null here, since we only request the PopularSites once an
observer is set.

https://codereview.chromium.org/1934583002/diff/1/chrome/browser/android/ntp/...
File chrome/browser/android/ntp/most_visited_sites.cc (right):

https://codereview.chromium.org/1934583002/diff/1/chrome/browser/android/ntp/...
chrome/browser/android/ntp/most_visited_sites.cc:186:
mv_source_(SUGGESTIONS_SERVICE), weak_ptr_factory_(this) {
Unrelated, but this used to be uninitialized. Optimistically assuming
SUGGESTIONS_SERVICE (as opposed to TOP_SITES) shouldn't hurt, and is at least
better than random :)

[Bernhard Bauer, 2016-04-29 14:15:31]

lgtm

https://codereview.chromium.org/1934583002/diff/1/chrome/browser/android/ntp/...
File chrome/browser/android/ntp/most_visited_sites.h (right):

https://codereview.chromium.org/1934583002/diff/1/chrome/browser/android/ntp/...
chrome/browser/android/ntp/most_visited_sites.h:93: // must not be null.
From a general API POV, this seems a bit problematic, because if the observer
should be destroyed before this object is, it couldn't be unregistered. We can
deal with that if and when it should arise though.

[Marc Treib, 2016-04-29 14:18:57]

Thanks!

https://codereview.chromium.org/1934583002/diff/1/chrome/browser/android/ntp/...
File chrome/browser/android/ntp/most_visited_sites.h (right):

https://codereview.chromium.org/1934583002/diff/1/chrome/browser/android/ntp/...
chrome/browser/android/ntp/most_visited_sites.h:93: // must not be null.
On 2016/04/29 14:15:31, Bernhard Bauer wrote:
> From a general API POV, this seems a bit problematic, because if the observer
> should be destroyed before this object is, it couldn't be unregistered. We can
> deal with that if and when it should arise though.

True, but I'm really just documenting the pre-existing assumption here.

[Marc Treib, 2016-04-29 14:19:01]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2016-04-29 14:19:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1934583002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1934583002/1

[commit-bot: I haz the power, 2016-04-29 15:06:14]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-04-30 17:25:49]

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

[commit-bot: I haz the power, 2016-04-30 18:00:38]

Description was changed from

==========
PopularSites: don't wait for a SuggestionsService network request

BUG=607615
==========

to

==========
PopularSites: don't wait for a SuggestionsService network request

BUG=607615

Committed: https://crrev.com/c1bd1f1093db2d9aada0cb4dd02c95c80f48ea55
Cr-Commit-Position: refs/heads/master@{#390648}
==========