[Popular Sites] Fix loading latency noticeable on iOS (again)

[Popular Sites] Fix loading latency noticeable on iOS (again)

Cached Popular Sites are now read synchronously (via tiles()) instead
of waiting for the callback. The callback is expected to be almost
immediate, but it turns out that the latency is easily noticeable on
iOS.

Since we have cached and parsed JSON, the list of sites can be built
before returning from StartFetch (now renamed to MaybeStartFetch())
and hence MostVisitedSites is able to expose PopularSites in its
first notification to observers.

The fix has been verified on an iOS device.

BUG=662397
Review-Url: https://codereview.chromium.org/2659633002
Cr-Commit-Position: refs/heads/master@{#446736}
Committed: https://chromium.googlesource.com/chromium/src/+/fd2c7ab348ff8ec60fe8609410da9cc02a769402

[mastiz, 2017-01-26 16:36:42]

Description was changed from

==========
[Popular Sites] Fix loading latency noticeable on iOS (again)

BUG=662397
==========

to

==========
[Popular Sites] Fix loading latency noticeable on iOS (again)

Cached Popular Sites are now read synchronously (via tiles()) instead
of waiting for the callback. The callback is expected to be almost
immediate, but it turns out that the latency is easily noticeable on
iOS.

Since we have cached and parsed JSON, the list of sites can be built
before returning from StartFetch (now renamed to MaybeStartFetch())
and hence MostVisitedSites is able to expose PopularSites in its
first notification to observers.

The fix has been verified on an iOS device.

BUG=662397
==========

[mastiz, 2017-01-26 16:36:57]

[email protected] changed reviewers:
+ [email protected]

[Marc Treib, 2017-01-26 16:42:04]

[email protected] changed reviewers:
+ [email protected]

[Marc Treib, 2017-01-26 16:42:05]

drive-by comments :)

https://codereview.chromium.org/2659633002/diff/1/components/ntp_tiles/popula...
File components/ntp_tiles/popular_sites.h (right):

https://codereview.chromium.org/2659633002/diff/1/components/ntp_tiles/popula...
components/ntp_tiles/popular_sites.h:56: virtual void StartFetch(bool
force_download,
MaybeStartFetch, per the description :)

That said, this seems like a weird API. How would the caller know whether to
wait for the callback or not?

https://codereview.chromium.org/2659633002/diff/1/components/ntp_tiles/popula...
File components/ntp_tiles/popular_sites_impl.cc (right):

https://codereview.chromium.org/2659633002/diff/1/components/ntp_tiles/popula...
components/ntp_tiles/popular_sites_impl.cc:324: PopularSites::SitesVector
PopularSitesImpl::ParseSiteList(
nit: Add
// static

[mastiz, 2017-01-27 08:57:19]

PTAL treib@ (since sfiera@ is OOO today).

https://codereview.chromium.org/2659633002/diff/1/components/ntp_tiles/popula...
File components/ntp_tiles/popular_sites.h (right):

https://codereview.chromium.org/2659633002/diff/1/components/ntp_tiles/popula...
components/ntp_tiles/popular_sites.h:56: virtual void StartFetch(bool
force_download,
On 2017/01/26 16:42:05, Marc Treib wrote:
> MaybeStartFetch, per the description :)
> 
> That said, this seems like a weird API. How would the caller know whether to
> wait for the callback or not?

Renamed, sfiera@ and I reached the same conclusion offline.

Wrt to the weirdness in the API: as it happens, no client code is actually
interested in knowing whether the callback is going to be called. Hence,
returning a boolean would be dead code. Extended the comment to state this.

https://codereview.chromium.org/2659633002/diff/1/components/ntp_tiles/popula...
File components/ntp_tiles/popular_sites_impl.cc (right):

https://codereview.chromium.org/2659633002/diff/1/components/ntp_tiles/popula...
components/ntp_tiles/popular_sites_impl.cc:324: PopularSites::SitesVector
PopularSitesImpl::ParseSiteList(
On 2017/01/26 16:42:05, Marc Treib wrote:
> nit: Add
> // static

Done.

[mastiz, 2017-01-27 08:57:25]

The CQ bit was checked by [email protected] to run a CQ dry run

[commit-bot: I haz the power, 2017-01-27 08:57:47]

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

[Marc Treib, 2017-01-27 09:25:21]

Looks like you accidentally uploaded some sort of temp file?

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/mo...
File components/ntp_tiles/most_visited_sites.cc (right):

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/mo...
components/ntp_tiles/most_visited_sites.cc:92: false,
base::Bind(&MostVisitedSites::OnPopularSitesAvailable,
Could you also rename OnPopularSitesAvailable, since that name isn't really
accurate (anymore). Maybe OnPopularSitesUpdated?

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/mo...
File components/ntp_tiles/most_visited_sites_unittest.cc (right):

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/mo...
components/ntp_tiles/most_visited_sites_unittest.cc:362: if
(IsPopularSitesEnabledViaFinch()) {
Not for this CL, but as a general rule, is it preferred to have two tests and
explicitly set the experiment state? IMO that'd be better.

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
File components/ntp_tiles/popular_sites.h (right):

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
components/ntp_tiles/popular_sites.h:42: using FinishedCallback =
base::Callback<void(bool /* success */)>;
Hm, arguably the |success| param also doesn't really make sense anymore, it you
might not get called back anyway... WDYT?

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
File components/ntp_tiles/popular_sites_impl.cc (right):

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
components/ntp_tiles/popular_sites_impl.cc:18: #include
"base/threading/thread_task_runner_handle.h"
Can be removed now.
I think the two above also aren't used.

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
File components/ntp_tiles/popular_sites_impl_unittest.cc (right):

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
components/ntp_tiles/popular_sites_impl_unittest.cc:141: base::MessageLoopForUI
ui_loop_;
Is this still needed? I think it was only there for the ThreadTaskRunner thing.

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
components/ntp_tiles/popular_sites_impl_unittest.cc:339: 
Can we add a test for the new case where the callback isn't called?

[commit-bot: I haz the power, 2017-01-27 09:28:16]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2017-01-27 09:28:17]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mastiz, 2017-01-27 13:26:37]

PTAL!

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/mo...
File components/ntp_tiles/most_visited_sites.cc (right):

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/mo...
components/ntp_tiles/most_visited_sites.cc:92: false,
base::Bind(&MostVisitedSites::OnPopularSitesAvailable,
On 2017/01/27 09:25:21, Marc Treib wrote:
> Could you also rename OnPopularSitesAvailable, since that name isn't really
> accurate (anymore). Maybe OnPopularSitesUpdated?

Renamed to OnPopularSitesDownloaded(), which is more accurate IMO and consistent
with the existing log trace.

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/mo...
File components/ntp_tiles/most_visited_sites_unittest.cc (right):

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/mo...
components/ntp_tiles/most_visited_sites_unittest.cc:362: if
(IsPopularSitesEnabledViaFinch()) {
On 2017/01/27 09:25:21, Marc Treib wrote:
> Not for this CL, but as a general rule, is it preferred to have two tests and
> explicitly set the experiment state? IMO that'd be better.

This is a comment for https://codereview.chromium.org/2658573005, under review,
feel free to jump in. My argument is that there's already many tests, and a few
fixtures, and listing all combinations would be worse than a few conditional
expectations.

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
File components/ntp_tiles/popular_sites.h (right):

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
components/ntp_tiles/popular_sites.h:42: using FinishedCallback =
base::Callback<void(bool /* success */)>;
On 2017/01/27 09:25:21, Marc Treib wrote:
> Hm, arguably the |success| param also doesn't really make sense anymore, it
you
> might not get called back anyway... WDYT?

I think it still makes sense. If an actual download happened, you are interested
in knowing whether it failed or not (this is specially obvious for the
force_download==true case).

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
File components/ntp_tiles/popular_sites_impl.cc (right):

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
components/ntp_tiles/popular_sites_impl.cc:18: #include
"base/threading/thread_task_runner_handle.h"
On 2017/01/27 09:25:21, Marc Treib wrote:
> Can be removed now.
> I think the two above also aren't used.

Done.

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
File components/ntp_tiles/popular_sites_impl_unittest.cc (right):

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
components/ntp_tiles/popular_sites_impl_unittest.cc:141: base::MessageLoopForUI
ui_loop_;
On 2017/01/27 09:25:21, Marc Treib wrote:
> Is this still needed? I think it was only there for the ThreadTaskRunner
thing.

I thought it would be needed by FakeURLFetcher(
https://cs.chromium.org/chromium/src/net/url_request/test_url_fetcher_factory...),
but running tests shows it's actually SequencedWorkerPoolOwner who complains.

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
components/ntp_tiles/popular_sites_impl_unittest.cc:339: 
On 2017/01/27 09:25:21, Marc Treib wrote:
> Can we add a test for the new case where the callback isn't called?

See UsesCachedJson, which btw was failing (I apparently forgot to run PS tests).
Fixed them all now, with major changes, PTAL.

[Marc Treib, 2017-01-27 14:04:26]

lgtm

Thanks!

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/mo...
File components/ntp_tiles/most_visited_sites_unittest.cc (right):

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/mo...
components/ntp_tiles/most_visited_sites_unittest.cc:362: if
(IsPopularSitesEnabledViaFinch()) {
On 2017/01/27 13:26:36, mastiz wrote:
> On 2017/01/27 09:25:21, Marc Treib wrote:
> > Not for this CL, but as a general rule, is it preferred to have two tests
and
> > explicitly set the experiment state? IMO that'd be better.
> 
> This is a comment for https://codereview.chromium.org/2658573005, under
review,
> feel free to jump in. My argument is that there's already many tests, and a
few
> fixtures, and listing all combinations would be worse than a few conditional
> expectations.

Aah, I was missing that the enabled/disabled state is actually injected and both
combinations are tested. I thought this was checking the actual variations
state.
Comment withdrawn. :)

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
File components/ntp_tiles/popular_sites.h (right):

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
components/ntp_tiles/popular_sites.h:42: using FinishedCallback =
base::Callback<void(bool /* success */)>;
On 2017/01/27 13:26:36, mastiz wrote:
> On 2017/01/27 09:25:21, Marc Treib wrote:
> > Hm, arguably the |success| param also doesn't really make sense anymore, it
> you
> > might not get called back anyway... WDYT?
> 
> I think it still makes sense. If an actual download happened, you are
interested
> in knowing whether it failed or not (this is specially obvious for the
> force_download==true case).

Acknowledged.

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
File components/ntp_tiles/popular_sites_impl_unittest.cc (right):

https://codereview.chromium.org/2659633002/diff/80001/components/ntp_tiles/po...
components/ntp_tiles/popular_sites_impl_unittest.cc:141: base::MessageLoopForUI
ui_loop_;
On 2017/01/27 13:26:36, mastiz wrote:
> On 2017/01/27 09:25:21, Marc Treib wrote:
> > Is this still needed? I think it was only there for the ThreadTaskRunner
> thing.
> 
> I thought it would be needed by FakeURLFetcher(
>
https://cs.chromium.org/chromium/src/net/url_request/test_url_fetcher_factory...),
> but running tests shows it's actually SequencedWorkerPoolOwner who complains.

Acknowledged.

https://codereview.chromium.org/2659633002/diff/100001/components/ntp_tiles/p...
File components/ntp_tiles/popular_sites_impl_unittest.cc (right):

https://codereview.chromium.org/2659633002/diff/100001/components/ntp_tiles/p...
components/ntp_tiles/popular_sites_impl_unittest.cc:132:
base::RunLoop().RunUntilIdle();
Hah, so this is a case where it's not even possible to wait for the callback.
(Though arguably that's because the API is weird...)

https://codereview.chromium.org/2659633002/diff/100001/components/ntp_tiles/p...
components/ntp_tiles/popular_sites_impl_unittest.cc:236:
Eq(base::Optional<bool>()));
nit: Can this be base::nullopt? I'd find that easier to read.
(Same for the other instances below)

[mastiz, 2017-01-27 15:02:29]

https://codereview.chromium.org/2659633002/diff/100001/components/ntp_tiles/p...
File components/ntp_tiles/popular_sites_impl_unittest.cc (right):

https://codereview.chromium.org/2659633002/diff/100001/components/ntp_tiles/p...
components/ntp_tiles/popular_sites_impl_unittest.cc:132:
base::RunLoop().RunUntilIdle();
On 2017/01/27 14:04:26, Marc Treib wrote:
> Hah, so this is a case where it's not even possible to wait for the callback.
> (Though arguably that's because the API is weird...)

I'm not very proud of this API, but I expect it to be replaced soon (with the
new requirement that we shouldn't actually update Popular Sites changes, once we
have a baked-in version).

https://codereview.chromium.org/2659633002/diff/100001/components/ntp_tiles/p...
components/ntp_tiles/popular_sites_impl_unittest.cc:236:
Eq(base::Optional<bool>()));
On 2017/01/27 14:04:26, Marc Treib wrote:
> nit: Can this be base::nullopt? I'd find that easier to read.
> (Same for the other instances below)

Done.

[mastiz, 2017-01-27 15:05:21]

[email protected] changed reviewers:
+ [email protected]

[mastiz, 2017-01-27 15:05:22]

Adding bauerb@ since he reviewed the earlier patch and pointed out that
RunUntilIdle() should be avoided.

Here we face the situation that the callback might or might not be called. This
is admittedly a weird API, but the production code happens to be fine with it
(and hence, it's questionable to influence the API for testing purposes only).

[mastiz, 2017-01-27 15:17:05]

FYI: I reverted one small part of the patch to avoid one occurrence of
RunUntilIdle().

[Bernhard Bauer, 2017-01-27 16:27:31]

https://codereview.chromium.org/2659633002/diff/160001/components/ntp_tiles/p...
File components/ntp_tiles/popular_sites.h (right):

https://codereview.chromium.org/2659633002/diff/160001/components/ntp_tiles/p...
components/ntp_tiles/popular_sites.h:61: virtual void MaybeStartFetch(bool
force_download,
Honestly, I would just return a boolean that says whether it immediately
completed. Yes, you might only care about that in a test, but it's a minor
change and it makes it easier to accurately observe what the method is doing.

https://codereview.chromium.org/2659633002/diff/160001/components/ntp_tiles/p...
File components/ntp_tiles/popular_sites_impl.h (right):

https://codereview.chromium.org/2659633002/diff/160001/components/ntp_tiles/p...
components/ntp_tiles/popular_sites_impl.h:86: static SitesVector
ParseSiteList(const base::ListValue& list);
Can this move to an anonymous namespace in the .cc file?

[mastiz, 2017-01-27 16:38:54]

The CQ bit was checked by [email protected] to run a CQ dry run

[commit-bot: I haz the power, 2017-01-27 16:39:19]

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

[mastiz, 2017-01-27 16:40:29]

The CQ bit was checked by [email protected] to run a CQ dry run

[mastiz, 2017-01-27 16:40:29]

I'll land after a grace period for the sake of having the fix asap: if there are
further comments, I'll address them in a follow-up patch, thanks for the
reviews!

https://codereview.chromium.org/2659633002/diff/160001/components/ntp_tiles/p...
File components/ntp_tiles/popular_sites.h (right):

https://codereview.chromium.org/2659633002/diff/160001/components/ntp_tiles/p...
components/ntp_tiles/popular_sites.h:61: virtual void MaybeStartFetch(bool
force_download,
On 2017/01/27 16:27:30, Bernhard Bauer wrote:
> Honestly, I would just return a boolean that says whether it immediately
> completed. Yes, you might only care about that in a test, but it's a minor
> change and it makes it easier to accurately observe what the method is doing.

Done.

https://codereview.chromium.org/2659633002/diff/160001/components/ntp_tiles/p...
File components/ntp_tiles/popular_sites_impl.h (right):

https://codereview.chromium.org/2659633002/diff/160001/components/ntp_tiles/p...
components/ntp_tiles/popular_sites_impl.h:86: static SitesVector
ParseSiteList(const base::ListValue& list);
On 2017/01/27 16:27:30, Bernhard Bauer wrote:
> Can this move to an anonymous namespace in the .cc file?

Done.

[commit-bot: I haz the power, 2017-01-27 16:40:35]

Dry run: 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-27 17:24:56]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2017-01-27 17:24:58]

Dry run: This issue passed the CQ dry run.

[mastiz, 2017-01-27 19:26:26]

The CQ bit was checked by [email protected]

[mastiz, 2017-01-27 19:26:27]

The patchset sent to the CQ was uploaded after l-g-t-m from [email protected]
Link to the patchset: https://codereview.chromium.org/2659633002/#ps150010
(title: "Address comments.")

[commit-bot: I haz the power, 2017-01-27 19:26:56]

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-27 19:35:03]

CQ is committing da patch.

Bot data: {"patchset_id": 150010, "attempt_start_ts": 1485545186990050,
"parent_rev": "05fed324c57380cc5faf4e937fc6a57ef6360afc", "commit_rev":
"fd2c7ab348ff8ec60fe8609410da9cc02a769402"}

[commit-bot: I haz the power, 2017-01-27 19:35:38]

Description was changed from

==========
[Popular Sites] Fix loading latency noticeable on iOS (again)

Cached Popular Sites are now read synchronously (via tiles()) instead
of waiting for the callback. The callback is expected to be almost
immediate, but it turns out that the latency is easily noticeable on
iOS.

Since we have cached and parsed JSON, the list of sites can be built
before returning from StartFetch (now renamed to MaybeStartFetch())
and hence MostVisitedSites is able to expose PopularSites in its
first notification to observers.

The fix has been verified on an iOS device.

BUG=662397
==========

to

==========
[Popular Sites] Fix loading latency noticeable on iOS (again)

Cached Popular Sites are now read synchronously (via tiles()) instead
of waiting for the callback. The callback is expected to be almost
immediate, but it turns out that the latency is easily noticeable on
iOS.

Since we have cached and parsed JSON, the list of sites can be built
before returning from StartFetch (now renamed to MaybeStartFetch())
and hence MostVisitedSites is able to expose PopularSites in its
first notification to observers.

The fix has been verified on an iOS device.

BUG=662397

Review-Url: https://codereview.chromium.org/2659633002
Cr-Commit-Position: refs/heads/master@{#446736}
Committed:
https://chromium.googlesource.com/chromium/src/+/fd2c7ab348ff8ec60fe8609410da...
==========

[commit-bot: I haz the power, 2017-01-27 19:35:40]

Committed patchset #11 (id:150010) as
https://chromium.googlesource.com/chromium/src/+/fd2c7ab348ff8ec60fe8609410da...