RemoteContentSuggestions: Stores the time of the last successful background fetch in a pref

components/ntp_snippets/remote/remote_suggestions_provider_unittest.cc

Index: components/ntp_snippets/remote/remote_suggestions_provider_unittest.cc
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider_unittest.cc b/components/ntp_snippets/remote/remote_suggestions_provider_unittest.cc
index b593ee4958589942d58af50c81b03cf56d745b34..2bcc45e091bb14ae5fd49a941db69d3620720aa2 100644
--- a/components/ntp_snippets/remote/remote_suggestions_provider_unittest.cc
+++ b/components/ntp_snippets/remote/remote_suggestions_provider_unittest.cc
@@ -20,6 +20,7 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/histogram_tester.h"
+#include "base/test/simple_test_tick_clock.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "components/image_fetcher/image_decoder.h"
@@ -1593,4 +1594,40 @@ TEST_F(RemoteSuggestionsProviderTest,
               SizeIs(service->GetMaxSnippetCountForTesting() + 1));
 }
 
+TEST_F(RemoteSuggestionsProviderTest, StoreLastSuccessfullBackgroundFetchTime) {
+  // On initialization of the RemoteSuggestionsProvider a background fetch is
+  // triggered since the snippets DB is empty. Therefore the service must not be
+  // initialized until the test clock is set.
+  auto service = MakeSnippetsServiceWithoutInitialization();
+
+  base::SimpleTestTickClock* simple_test_tick_clock =
+      new base::SimpleTestTickClock();

[Marc Treib, 2016/12/07 14:42:13]

I dislike raw pointers :)  How about:
- Make a unique_ptr
- Get a raw ptr copy
- Pass the unique_ptr to the provider
?

[markusheintz_, 2016/12/08 09:47:41]

Done

+  std::unique_ptr<base::TickClock> tick_clock(simple_test_tick_clock);
+  service->SetTickClockForTesting(std::move(tick_clock));
+
+  base::TimeTicks test_now = base::TimeTicks::Now();
+  simple_test_tick_clock->SetNowTicks(test_now);

[Marc Treib, 2016/12/07 14:42:13]

Should this happen before passing the clock to the provider? (So that it always
has a well-defined value)

[markusheintz_, 2016/12/08 09:47:41]

SGTM done

+
+  // Test that the preference is correclty initialized with the default value 0.

[Marc Treib, 2016/12/07 14:42:13]

s/correclty/correctly/

[markusheintz_, 2016/12/08 09:47:41]

Done.

+  EXPECT_EQ(
+      0, pref_service()->GetInt64(prefs::kLastSuccessfullBackgroundFetchTime));
+
+  // Initialize the service. Setting |set_empty_response| to true will trigger a
+  // background fetch on initialization since the snippets DB is still empty.

[Marc Treib, 2016/12/07 14:42:13]

This isn't really accurate - the background fetch attempt will happen anyway,
independent of |set_empty_response|.

[markusheintz_, 2016/12/08 09:47:41]

This is not what I experienced when I wrote this. I'll remove the comment and
investigate after the CL is done.

+  WaitForSnippetsServiceInitialization(service.get(),
+                                       /*set_empty_response=*/true);
+
+  EXPECT_EQ(
+      test_now.ToInternalValue(),
+      pref_service()->GetInt64(prefs::kLastSuccessfullBackgroundFetchTime));
+
+  simple_test_tick_clock->Advance(TimeDelta::FromHours(1));
+  service->FetchSnippetsInTheBackground();
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(
+      (test_now + TimeDelta::FromHours(1)).ToInternalValue(),

[Marc Treib, 2016/12/07 14:42:13]

This could just use simple_test_tick_clock->NowTicks(), then the |test_now|
variable wouldn't be necessary, right?

+      pref_service()->GetInt64(prefs::kLastSuccessfullBackgroundFetchTime));
+}
+
 }  // namespace ntp_snippets