Test SuggestionsServiceImpl without friending anything

components/suggestions/suggestions_service_impl.h

Index: components/suggestions/suggestions_service_impl.h
diff --git a/components/suggestions/suggestions_service_impl.h b/components/suggestions/suggestions_service_impl.h
index 7bd2959d65b3a3d315adc732a59b799f8d543d56..f01cb85474510f81a06c35e231bbd6337c55305f 100644
--- a/components/suggestions/suggestions_service_impl.h
+++ b/components/suggestions/suggestions_service_impl.h
@@ -78,6 +78,16 @@ class SuggestionsServiceImpl : public SuggestionsService,
   bool UndoBlacklistURL(const GURL& url) override;
   void ClearBlacklist() override;
 
+  base::TimeDelta blacklist_delay_for_testing() const {

[Marc Treib, 2017/05/10 09:53:44]

Out of curiosity: Do you feel that "for_testing" methods are better than
friends? I usually tend to prefer friending the test base class, since then you
don't have to clutter up the public interface (but I don't feel very strongly,
so this is fine too).

[fhorschig, 2017/05/10 11:54:44]

In this case, I am (obviously) absolutely for for_testing as the friending was
abused to test small parts of the implementation and not overall behaviors.
Also, the public interface that really is used is SuggestionsService, so in
order to abuse the public for_testing methods, you would have to put in quite 
some effort.
(2nd point in our guidelines: go/zine-testing#heading=h.ch8i4lz9s0f2)

+    return scheduling_delay_;
+  }
+  void set_blacklist_delay_for_testing(base::TimeDelta delay) {
+    scheduling_delay_ = delay;
+  }
+  bool has_pending_request_for_testing() const {
+    return !!pending_request_.get();
+  }
+
   // Determines which URL a blacklist request was for, irrespective of the
   // request's status. Returns false if |request| is not a blacklist request.
   static bool GetBlacklistedUrl(const net::URLFetcher& request, GURL* url);
@@ -86,35 +96,6 @@ class SuggestionsServiceImpl : public SuggestionsService,
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
  private:
-  friend class SuggestionsServiceTest;
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, FetchSuggestionsData);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, IgnoresNoopSyncChange);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest,
-                           IgnoresUninterestingSyncChange);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, DoesNotFetchOnStartup);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest,
-                           FetchSuggestionsDataSyncNotInitializedEnabled);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest,
-                           FetchSuggestionsDataSyncDisabled);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest,
-                           FetchSuggestionsDataNoAccessToken);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest,
-                           IssueRequestIfNoneOngoingError);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest,
-                           IssueRequestIfNoneOngoingResponseNotOK);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, BlacklistURL);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, BlacklistURLRequestFails);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, ClearBlacklist);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, UndoBlacklistURL);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest,
-                           UndoBlacklistURLFailsIfNotInBlacklist);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest,
-                           UndoBlacklistURLFailsIfAlreadyCandidate);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest,
-                           GetBlacklistedUrlBlacklistRequest);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, UpdateBlacklistDelay);
-  FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, CheckDefaultTimeStamps);
-
   // Establishes the different sync states that matter to SuggestionsService.
   enum SyncState {
     // State: Sync service is not initialized, yet not disabled. History sync
@@ -200,10 +181,6 @@ class SuggestionsServiceImpl : public SuggestionsService,
   // Adds extra data to suggestions profile.
   void PopulateExtraData(SuggestionsProfile* suggestions);
 
-  // Test seams.
-  base::TimeDelta blacklist_delay() const { return scheduling_delay_; }
-  void set_blacklist_delay(base::TimeDelta delay) { scheduling_delay_ = delay; }
-
   base::ThreadChecker thread_checker_;
 
   SigninManagerBase* signin_manager_;