Cache GCM token in prefs across browser restarts

There is no guarantee that if chrome is in full browser mode that it is
able to get a GCM token before scheduling a PrefetchBackgroundTask. This
cl caches the prefetch gcm token in PrefService across browser restarts
to help in that scenario.

Bug: 968320
Change-Id: If88ef3d393daa998fae1d7681999921e9142ae2c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1638622
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Auto-Submit: Mohamed Heikal <mheikal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#667344}

chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/OfflineTestUtil.java

@@ -168,6 +168,10 @@ public class OfflineTestUtil {
                 () -> { nativeSetPrefetchingEnabledByServer(enabled); });
     }
 
+    public static void setGCMTokenForTesting(String gcmToken) {
+        TestThreadUtils.runOnUiThreadBlocking(() -> { nativeSetGCMTokenForTesting(gcmToken); });
+    }
+
     private static native void nativeGetRequestsInQueue(Callback<SavePageRequest[]> callback);
     private static native void nativeGetAllPages(
             List<OfflinePageItem> offlinePages, final Callback<List<OfflinePageItem>> callback);
@@ -177,4 +181,5 @@ public class OfflineTestUtil {
     private static native void nativeDumpRequestCoordinatorState(Callback<String> callback);
     private static native void nativeWaitForConnectivityState(boolean connected, Runnable callback);
     private static native void nativeSetPrefetchingEnabledByServer(boolean enabled);
+    private static native void nativeSetGCMTokenForTesting(String gcmToken);
 }

chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchBackgroundTaskTest.java

@@ -192,6 +192,7 @@ public class PrefetchBackgroundTaskTest {
             BackgroundTaskSchedulerFactory.setSchedulerForTesting(mScheduler);
         });
         OfflineTestUtil.setPrefetchingEnabledByServer(true);
+        OfflineTestUtil.setGCMTokenForTesting("dummy_gcm_token");
 
         PrefetchBackgroundTask.alwaysSupportServiceManagerOnlyForTesting();
     }

chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchFeedFlowTest.java

@@ -200,6 +200,7 @@ public class PrefetchFeedFlowTest implements WebServer.RequestHandler {
         });
 
         OfflineTestUtil.setPrefetchingEnabledByServer(true);
+        OfflineTestUtil.setGCMTokenForTesting("dummy_gcm_token");
     }
 
     @After

chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchFlowTest.java

@@ -138,6 +138,7 @@ public class PrefetchFlowTest implements WebServer.RequestHandler {
             PrefetchTestBridge.skipNTPSuggestionsAPIKeyCheck();
         });
         OfflineTestUtil.setPrefetchingEnabledByServer(true);
+        OfflineTestUtil.setGCMTokenForTesting("dummy_gcm_token");
     }
 
     @After

chrome/browser/offline_pages/android/offline_test_util_jni.cc

@@ -266,4 +266,12 @@ void JNI_OfflineTestUtil_SetPrefetchingEnabledByServer(
   }
 }
 
+void JNI_OfflineTestUtil_SetGCMTokenForTesting(
+    JNIEnv* env,
+    const JavaParamRef<jstring>& gcm_token) {
+  prefetch_prefs::SetCachedPrefetchGCMToken(
+      ::android::GetMainProfileKey()->GetPrefs(),
+      base::android::ConvertJavaStringToUTF8(env, gcm_token));
+}
+
 }  // namespace offline_pages

chrome/browser/offline_pages/android/prefetch_background_task_android.cc

@@ -28,6 +28,7 @@ namespace prefetch {
 static jboolean JNI_PrefetchBackgroundTask_StartPrefetchTask(
     JNIEnv* env,
     const JavaParamRef<jobject>& jcaller,
+    // TODO(https://crbug.com/972218): remove unused param
     const JavaParamRef<jstring>& gcm_token) {
   ProfileKey* profile_key = ::android::GetMainProfileKey();
   DCHECK(profile_key);
@@ -37,9 +38,6 @@ static jboolean JNI_PrefetchBackgroundTask_StartPrefetchTask(
   if (!prefetch_service)
     return false;
 
-  prefetch_service->SetCachedGCMToken(
-      base::android::ConvertJavaStringToUTF8(env, gcm_token));
-
   prefetch_service->GetPrefetchDispatcher()->BeginBackgroundTask(
       std::make_unique<PrefetchBackgroundTaskAndroid>(env, jcaller,
                                                       prefetch_service));

chrome/browser/offline_pages/prefetch/prefetch_service_factory.cc

@@ -69,8 +69,8 @@ void OnProfileCreated(PrefetchServiceImpl* service, Profile* profile) {
     base::PostTaskWithTraits(
         FROM_HERE,
         {content::BrowserThread::UI, base::TaskPriority::BEST_EFFORT},
-        base::BindOnce(&PrefetchServiceImpl::GetGCMToken, service->GetWeakPtr(),
-                       base::DoNothing::Once<const std::string&>()));
+        base::BindOnce(&PrefetchServiceImpl::RefreshGCMToken,
+                       service->GetWeakPtr()));
   }
 }
 
@@ -177,7 +177,7 @@ std::unique_ptr<KeyedService> PrefetchServiceFactory::BuildServiceInstanceFor(
       std::move(prefetch_store), std::move(suggested_articles_observer),
       std::move(prefetch_downloader), std::move(prefetch_importer),
       std::move(prefetch_background_task_handler), std::move(thumbnail_fetcher),
-      image_fetcher);
+      image_fetcher, profile_key->GetPrefs());
 
   auto callback = base::BindOnce(&OnProfileCreated, service.get());
   FullBrowserTransitionManager::Get()->RegisterCallbackOnProfileCreation(

chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc

@@ -261,28 +261,20 @@ void OfflineInternalsUIMessageHandler::HandleGetNetworkStatus(
 void OfflineInternalsUIMessageHandler::HandleScheduleNwake(
     const base::ListValue* args) {
   AllowJavascript();
-  CHECK(!args->GetList().empty());
-  base::Value callback_id = args->GetList()[0].Clone();
+  const base::Value* callback_id;
+  CHECK(args->Get(0, &callback_id));
 
   if (prefetch_service_) {
-    prefetch_service_->GetGCMToken(base::BindOnce(
-        &OfflineInternalsUIMessageHandler::ScheduleNwakeWithGCMToken,
-        weak_ptr_factory_.GetWeakPtr(), std::move(callback_id)));
+    prefetch_service_->ForceRefreshSuggestions();
+    prefetch_service_->GetPrefetchBackgroundTaskHandler()->EnsureTaskScheduled(
+        prefetch_service_->GetCachedGCMToken());
+    ResolveJavascriptCallback(*callback_id, base::Value("Scheduled."));
   } else {
-    RejectJavascriptCallback(callback_id,
+    RejectJavascriptCallback(*callback_id,
                              base::Value("No prefetch service available."));
   }
 }
 
-void OfflineInternalsUIMessageHandler::ScheduleNwakeWithGCMToken(
-    base::Value callback_id,
-    const std::string& gcm_token) {
-  prefetch_service_->ForceRefreshSuggestions();
-  prefetch_service_->GetPrefetchBackgroundTaskHandler()->EnsureTaskScheduled(
-      gcm_token);
-  ResolveJavascriptCallback(callback_id, base::Value("Scheduled."));
-}
-
 void OfflineInternalsUIMessageHandler::HandleCancelNwake(
     const base::ListValue* args) {
   AllowJavascript();

chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.h

@@ -85,8 +85,6 @@ class OfflineInternalsUIMessageHandler : public content::WebUIMessageHandler {
 
   // Schedules an NWake signal.
   void HandleScheduleNwake(const base::ListValue* args);
-  void ScheduleNwakeWithGCMToken(base::Value callback_id,
-                                 const std::string& gcm_token);
 
   // Cancels an NWake signal.
   void HandleCancelNwake(const base::ListValue* args);

components/offline_pages/core/prefetch/prefetch_dispatcher_impl.cc

@@ -91,17 +91,11 @@ void PrefetchDispatcherImpl::EnsureTaskScheduled() {
     background_task_->SetReschedule(
         PrefetchBackgroundTaskRescheduleType::RESCHEDULE_WITHOUT_BACKOFF);
   } else {
-    service_->GetGCMToken(
-        base::BindOnce(&PrefetchDispatcherImpl::EnsureTaskScheduledWithGCMToken,
-                       weak_factory_.GetWeakPtr()));
+    service_->GetPrefetchBackgroundTaskHandler()->EnsureTaskScheduled(
+        service_->GetCachedGCMToken());
   }
 }
 
-void PrefetchDispatcherImpl::EnsureTaskScheduledWithGCMToken(
-    const std::string& gcm_token) {
-  service_->GetPrefetchBackgroundTaskHandler()->EnsureTaskScheduled(gcm_token);
-}
-
 void PrefetchDispatcherImpl::AddCandidatePrefetchURLs(
     const std::string& name_space,
     const std::vector<PrefetchURL>& prefetch_urls) {

components/offline_pages/core/prefetch/prefetch_dispatcher_impl.h

@@ -74,8 +74,6 @@ class PrefetchDispatcherImpl : public PrefetchDispatcher,
 
   void DisposeTask();
 
-  void EnsureTaskScheduledWithGCMToken(const std::string& gcm_token);
-
   // Callbacks for network requests.
   void DidGenerateBundleOrGetOperationRequest(
       const std::string& request_name_for_logging,

components/offline_pages/core/prefetch/prefetch_dispatcher_impl_unittest.cc

@@ -305,6 +305,8 @@ class PrefetchDispatcherTest : public PrefetchRequestTestBase {
 
     taco_ = std::make_unique<PrefetchServiceTestTaco>(suggestion_source);
     prefetch_prefs::SetEnabledByServer(taco_->pref_service(), true);
+    prefetch_prefs::SetCachedPrefetchGCMToken(taco_->pref_service(),
+                                              "dummy_gcm_token");
     dispatcher_ = new PrefetchDispatcherImpl(taco_->pref_service());
     network_request_factory_ = new FakePrefetchNetworkRequestFactory(
         shared_url_loader_factory(), taco_->pref_service());

components/offline_pages/core/prefetch/prefetch_download_flow_unittest.cc

@@ -44,6 +44,8 @@ class PrefetchDownloadFlowTest : public PrefetchTaskTestBase {
     prefetch_service_taco_->SetPrefService(std::move(prefs_));
     prefetch_prefs::SetEnabledByServer(prefetch_service_taco_->pref_service(),
                                        true);
+    prefetch_prefs::SetCachedPrefetchGCMToken(
+        prefetch_service_taco_->pref_service(), "dummy_gcm_token");
 
     auto downloader = std::make_unique<PrefetchDownloaderImpl>(
         &download_service_, kTestChannel,
@@ -56,8 +58,6 @@ class PrefetchDownloadFlowTest : public PrefetchTaskTestBase {
     prefetch_service_taco_->SetPrefetchStore(store_util()->ReleaseStore());
     prefetch_service_taco_->SetPrefetchDownloader(std::move(downloader));
     prefetch_service_taco_->CreatePrefetchService();
-    prefetch_service_taco_->prefetch_service()->SetCachedGCMToken(
-        "dummy_gcm_token");
     item_generator()->set_client_namespace(kSuggestedArticlesNamespace);
   }
 

components/offline_pages/core/prefetch/prefetch_prefs.cc

@@ -20,6 +20,7 @@ const char kPrefetchTestingHeaderPref[] =
 const char kEnabledByServer[] = "offline_prefetch.enabled_by_server";
 const char kNextForbiddenCheckTimePref[] = "offline_prefetch.next_gpb_check";
 const base::TimeDelta kForbiddenCheckDelay = base::TimeDelta::FromDays(7);
+const char kPrefetchCachedGCMToken[] = "offline_prefetch.gcm_token";
 
 }  // namespace
 
@@ -34,6 +35,7 @@ void RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterStringPref(kPrefetchTestingHeaderPref, std::string());
   registry->RegisterBooleanPref(kEnabledByServer, false);
   registry->RegisterTimePref(kNextForbiddenCheckTimePref, base::Time());
+  registry->RegisterStringPref(kPrefetchCachedGCMToken, std::string());
 }
 
 void SetPrefetchingEnabledInSettings(PrefService* prefs, bool enabled) {
@@ -120,5 +122,15 @@ void ResetForbiddenStateForTesting(PrefService* prefs) {
   prefs->SetTime(kNextForbiddenCheckTimePref, base::Time());
 }
 
+void SetCachedPrefetchGCMToken(PrefService* prefs, const std::string& value) {
+  DCHECK(prefs);
+  prefs->SetString(kPrefetchCachedGCMToken, value);
+}
+
+std::string GetCachedPrefetchGCMToken(PrefService* prefs) {
+  DCHECK(prefs);
+  return prefs->GetString(kPrefetchCachedGCMToken);
+}
+
 }  // namespace prefetch_prefs
 }  // namespace offline_pages

components/offline_pages/core/prefetch/prefetch_prefs.h

@@ -70,6 +70,11 @@ bool IsEnabledByServerUnknown(PrefService* prefs);
 // run.
 void ResetForbiddenStateForTesting(PrefService* prefs);
 
+// Caches the GCM token across browser restarts so that it can be accessed in
+// reduced mode.
+void SetCachedPrefetchGCMToken(PrefService* prefs, const std::string& value);
+std::string GetCachedPrefetchGCMToken(PrefService* prefs);
+
 }  // namespace prefetch_prefs
 }  // namespace offline_pages
 

components/offline_pages/core/prefetch/prefetch_service.h

@@ -88,13 +88,9 @@ class PrefetchService : public KeyedService {
   // mode.
   virtual PrefetchGCMHandler* GetPrefetchGCMHandler() = 0;
 
-  // Obtains the current GCM token from the PrefetchGCMHandler
-  virtual void GetGCMToken(GCMTokenCallback callback) = 0;
-
-  // Stores and retrieves a cached GCM token to be used if PrefetchGCMHandler is
+  // Retrieves a cached GCM token from prefs to be used if PrefetchGCMHandler is
   // unavailable.
-  virtual void SetCachedGCMToken(const std::string& gcm_token) = 0;
-  virtual const std::string& GetCachedGCMToken() const = 0;
+  virtual std::string GetCachedGCMToken() const = 0;
 
   virtual void SetEnabledByServer(PrefService* pref_service, bool enabled) = 0;
 

components/offline_pages/core/prefetch/prefetch_service_impl.cc

@@ -39,7 +39,8 @@ PrefetchServiceImpl::PrefetchServiceImpl(
     std::unique_ptr<PrefetchBackgroundTaskHandler>
         prefetch_background_task_handler,
     std::unique_ptr<ThumbnailFetcher> thumbnail_fetcher,
-    image_fetcher::ImageFetcher* image_fetcher)
+    image_fetcher::ImageFetcher* image_fetcher,
+    PrefService* prefs)
     : offline_metrics_collector_(std::move(offline_metrics_collector)),
       prefetch_dispatcher_(std::move(dispatcher)),
       network_request_factory_(std::move(network_request_factory)),
@@ -49,6 +50,7 @@ PrefetchServiceImpl::PrefetchServiceImpl(
       prefetch_importer_(std::move(prefetch_importer)),
       prefetch_background_task_handler_(
           std::move(prefetch_background_task_handler)),
+      prefs_(prefs),
       suggested_articles_observer_(std::move(suggested_articles_observer)),
       thumbnail_fetcher_(std::move(thumbnail_fetcher)),
       image_fetcher_(image_fetcher),
@@ -76,37 +78,24 @@ void PrefetchServiceImpl::ForceRefreshSuggestions() {
   }
 }
 
-void PrefetchServiceImpl::SetCachedGCMToken(const std::string& gcm_token) {
-  // This method is passed a cached token that was stored in the job scheduler,
-  // to be used until the PrefetchGCMHandler is created. In some cases, the
-  // PrefetchGCMHandler could have been already created and a fresher token
-  // requested before this function is called. Make sure to not override a
-  // fresher token with a stale one.
-  if (gcm_token_.empty())
-    gcm_token_ = gcm_token;
+std::string PrefetchServiceImpl::GetCachedGCMToken() const {
+  return prefetch_prefs::GetCachedPrefetchGCMToken(prefs_);
 }
 
-const std::string& PrefetchServiceImpl::GetCachedGCMToken() const {
-  DCHECK(!gcm_token_.empty()) << "No cached token is set, you should call "
-                                 "PrefetchService::GetGCMToken instead";
-  return gcm_token_;
-}
-
-void PrefetchServiceImpl::GetGCMToken(GCMTokenCallback callback) {
+void PrefetchServiceImpl::RefreshGCMToken() {
   DCHECK(prefetch_gcm_handler_);
   prefetch_gcm_handler_->GetGCMToken(base::AdaptCallbackForRepeating(
-      base::BindOnce(&PrefetchServiceImpl::OnGCMTokenReceived, GetWeakPtr(),
-                     std::move(callback))));
+      base::BindOnce(&PrefetchServiceImpl::OnGCMTokenReceived, GetWeakPtr())));
 }
 
 void PrefetchServiceImpl::OnGCMTokenReceived(
-    GCMTokenCallback callback,
     const std::string& gcm_token,
     instance_id::InstanceID::Result result) {
   // TODO(dimich): Add UMA reporting on instance_id::InstanceID::Result.
-  // Keep the cached token fresh
-  gcm_token_ = gcm_token;
-  std::move(callback).Run(gcm_token);
+  if (result == instance_id::InstanceID::Result::SUCCESS) {
+    // Keep the cached token fresh
+    prefetch_prefs::SetCachedPrefetchGCMToken(prefs_, gcm_token);
+  }
 }
 
 void PrefetchServiceImpl::SetContentSuggestionsService(

components/offline_pages/core/prefetch/prefetch_service_impl.h

@@ -33,7 +33,8 @@ class PrefetchServiceImpl : public PrefetchService {
       std::unique_ptr<PrefetchImporter> prefetch_importer,
       std::unique_ptr<PrefetchBackgroundTaskHandler> background_task_handler,
       std::unique_ptr<ThumbnailFetcher> thumbnail_fetcher,
-      image_fetcher::ImageFetcher* image_fetcher_);
+      image_fetcher::ImageFetcher* image_fetcher_,
+      PrefService* prefs);
 
   ~PrefetchServiceImpl() override;
 
@@ -46,9 +47,7 @@ class PrefetchServiceImpl : public PrefetchService {
   void NewSuggestionsAvailable() override;
   void RemoveSuggestion(GURL url) override;
   PrefetchGCMHandler* GetPrefetchGCMHandler() override;
-  void SetCachedGCMToken(const std::string& gcm_token) override;
-  const std::string& GetCachedGCMToken() const override;
-  void GetGCMToken(GCMTokenCallback callback) override;
+  std::string GetCachedGCMToken() const override;
   void SetEnabledByServer(PrefService* pref_service, bool enabled) override;
 
   // Internal usage only functions.
@@ -64,6 +63,7 @@ class PrefetchServiceImpl : public PrefetchService {
   PrefetchBackgroundTaskHandler* GetPrefetchBackgroundTaskHandler() override;
 
   void SetPrefetchGCMHandler(std::unique_ptr<PrefetchGCMHandler> handler);
+  void RefreshGCMToken();
 
   // Thumbnail fetchers. With Feed, GetImageFetcher() is available
   // and GetThumbnailFetcher() is null.
@@ -78,12 +78,10 @@ class PrefetchServiceImpl : public PrefetchService {
   void Shutdown() override;
 
  private:
-  void OnGCMTokenReceived(GCMTokenCallback callback,
-                          const std::string& gcm_token,
+  void OnGCMTokenReceived(const std::string& gcm_token,
                           instance_id::InstanceID::Result result);
 
   OfflineEventLogger logger_;
-  std::string gcm_token_;
   std::unique_ptr<PrefetchGCMHandler> prefetch_gcm_handler_;
 
   std::unique_ptr<OfflineMetricsCollector> offline_metrics_collector_;
@@ -95,6 +93,7 @@ class PrefetchServiceImpl : public PrefetchService {
   std::unique_ptr<PrefetchImporter> prefetch_importer_;
   std::unique_ptr<PrefetchBackgroundTaskHandler>
       prefetch_background_task_handler_;
+  PrefService* prefs_;
 
   // Zine/Feed: only non-null when using Zine.
   std::unique_ptr<SuggestedArticlesObserver> suggested_articles_observer_;

components/offline_pages/core/prefetch/prefetch_service_test_taco.cc

@@ -200,7 +200,8 @@ void PrefetchServiceTestTaco::CreatePrefetchService() {
       std::move(prefetch_store_), std::move(suggested_articles_observer_),
       std::move(prefetch_downloader_), std::move(prefetch_importer_),
       std::move(prefetch_background_task_handler_),
-      std::move(thumbnail_fetcher_), thumbnail_image_fetcher_.get());
+      std::move(thumbnail_fetcher_), thumbnail_image_fetcher_.get(),
+      pref_service_.get());
   service->SetPrefetchGCMHandler(std::move(gcm_handler_));
   prefetch_service_ = std::move(service);
 }

components/offline_pages/core/prefetch/stub_prefetch_service.cc

@@ -18,14 +18,10 @@ void StubPrefetchService::NewSuggestionsAvailable() {}
 
 void StubPrefetchService::RemoveSuggestion(GURL url) {}
 
-void StubPrefetchService::SetCachedGCMToken(const std::string& gcm_token) {}
-
-void StubPrefetchService::GetGCMToken(GCMTokenCallback callback) {}
-
 void StubPrefetchService::ForceRefreshSuggestions() {}
 
-const std::string& StubPrefetchService::GetCachedGCMToken() const {
-  return gcm_token_;
+std::string StubPrefetchService::GetCachedGCMToken() const {
+  return "";
 }
 
 PrefetchGCMHandler* StubPrefetchService::GetPrefetchGCMHandler() {

components/offline_pages/core/prefetch/stub_prefetch_service.h

@@ -20,9 +20,7 @@ class StubPrefetchService : public PrefetchService {
       SuggestionsProvider* suggestions_provider) override;
   void NewSuggestionsAvailable() override;
   void RemoveSuggestion(GURL url) override;
-  void SetCachedGCMToken(const std::string& gcm_token) override;
-  const std::string& GetCachedGCMToken() const override;
-  void GetGCMToken(GCMTokenCallback callback) override;
+  std::string GetCachedGCMToken() const override;
   void ForceRefreshSuggestions() override;
   PrefetchGCMHandler* GetPrefetchGCMHandler() override;
   OfflineEventLogger* GetLogger() override;
@@ -39,9 +37,6 @@ class StubPrefetchService : public PrefetchService {
   void SetEnabledByServer(PrefService* pref_service, bool enabled) override;
 
   SuggestedArticlesObserver* GetSuggestedArticlesObserverForTesting() override;
-
- private:
-  std::string gcm_token_;
 };
 
 }  // namespace offline_pages