Abstract GoogleURLTracker & google_util Profile dependencies

chrome/browser/google/google_profile_helper.h

Index: chrome/browser/google/google_profile_helper.h
diff --git a/chrome/browser/google/google_profile_helper.h b/chrome/browser/google/google_profile_helper.h
new file mode 100644
index 0000000000000000000000000000000000000000..14b2aa7ce2c025b63a562e83b0df83422296d7db
--- /dev/null
+++ b/chrome/browser/google/google_profile_helper.h
@@ -0,0 +1,33 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Provides Google-related information for a given Profile.

[Peter Kasting, 2014/05/30 20:50:39]

I would expand this file comment to say something along the lines of these
largely being wrappers for caller convenience around google component functions,
which can't depend on Profiles.

+
+#ifndef CHROME_BROWSER_GOOGLE_GOOGLE_PROFILE_HELPER_H__
+#define CHROME_BROWSER_GOOGLE_GOOGLE_PROFILE_HELPER_H__
+
+#include <string>
+
+#include "base/basictypes.h"
+
+class GURL;
+class Profile;
+
+namespace google_profile_helper {

[Peter Kasting, 2014/05/30 20:50:39]

So, how did you end up deciding to put these functions here, but not wrappers
for the "request server check" or "search committed" calls?  It seems a little
inconsistent that some places people should use wrappers, and other places they
should talk to the Google URL tracker object directly.

Also, everywhere that calls those directly, you've made conditional against a
NULL tracker, which implies that's actually possible at those callsites -- but I
don't know whether it _is_ possible at all those callsites (and where it's not,
we shouldn't NULL-check).

[blundell, 2014/06/02 15:42:26]

I created the GetGoogleHomePageURL() function since that has the logic of what
URL to set as the default if the GoogleURLTracker doesn't exist, which I didn't
want to replicate at all the callsites. For the other two, I agree that they're
not necessary; removed now.
 
The GoogleURLTrackerFactory has the service be NULL while testing, so any code
that calls GoogleURLTrackerFactory::GetForProfile() does need to guard against
the service being NULL (if it doesn't blow up, it's just because that code isn't
currently covered by any tests).

+
+// Returns the Google country code string for the given profile. This will
+// return a valid country code even if |profile| is NULL or a testing profile.
+std::string GetGoogleCountryCode(Profile* profile);
+
+// Returns the Google search URL for the given profile. This will return a
+// valid URL even if |profile| is NULL or a testing profile.
+GURL GetGoogleSearchURL(Profile* profile);
+
+// Returns the current Google homepage URL. This will return a valid URL even if
+// |profile| is NULL or a testing profile.
+GURL GetGoogleHomePageURL(Profile* profile);
+
+}  // namespace google_profile_helper
+
+#endif  // CHROME_BROWSER_GOOGLE_GOOGLE_PROFILE_HELPER_H__