Revert "Remove webkit_glue::BuildUserAgent(), change the contract in webkit_glue"

TBR=jam@chromium.org
BUG=90442
TEST=none

Review URL: http://codereview.chromium.org/8002003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@102353 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/browser_init.cc

@@ -78,6 +78,7 @@
 #include "net/base/net_util.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "webkit/glue/webkit_glue.h"
 
 #if defined(OS_MACOSX)
 #include "base/mac/mac_util.h"
@@ -689,6 +690,11 @@ bool BrowserInit::LaunchWithProfile::Launch(
     }
   }
 
+  if (command_line_.HasSwitch(switches::kUserAgent)) {
+    webkit_glue::SetUserAgent(command_line_.GetSwitchValueASCII(
+        switches::kUserAgent));
+  }
+
   // Open the required browser windows and tabs. First, see if
   // we're being run as an application window. If so, the user
   // opened an app shortcut.  Don't restore tabs or open initial

chrome/common/chrome_content_client.cc

@@ -300,18 +300,12 @@ bool ChromeContentClient::CanHandleWhileSwappedOut(
   return false;
 }
 
-std::string ChromeContentClient::GetUserAgent(bool* overriding) const {
-  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kUserAgent)) {
-    *overriding = true;
-    return CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-        switches::kUserAgent);
-  } else {
-    *overriding = false;
-    chrome::VersionInfo version_info;
-    std::string product("Chrome/");
-    product += version_info.is_valid() ? version_info.Version() : "0.0.0.0";
-    return webkit_glue::BuildUserAgentFromProduct(product);
-  }
+std::string ChromeContentClient::GetUserAgent(bool mimic_windows) const {
+  chrome::VersionInfo version_info;
+  std::string product("Chrome/");
+  product += version_info.is_valid() ? version_info.Version() : "0.0.0.0";
+
+  return webkit_glue::BuildUserAgentHelper(mimic_windows, product);
 }
 
 string16 ChromeContentClient::GetLocalizedString(int message_id) const {

chrome/common/chrome_content_client.h

@@ -6,7 +6,6 @@
 #define CHROME_COMMON_CHROME_CONTENT_CLIENT_H_
 #pragma once
 
-#include "base/compiler_specific.h"
 #include "content/common/content_client.h"
 
 namespace chrome {
@@ -17,19 +16,18 @@ class ChromeContentClient : public content::ContentClient {
   static const char* const kNaClPluginName;
   static const char* const kNaClOldPluginName;
 
-  virtual void SetActiveURL(const GURL& url) OVERRIDE;
-  virtual void SetGpuInfo(const GPUInfo& gpu_info) OVERRIDE;
-  virtual void AddPepperPlugins(
-      std::vector<PepperPluginInfo>* plugins) OVERRIDE;
-  virtual bool CanSendWhileSwappedOut(const IPC::Message* msg) OVERRIDE;
-  virtual bool CanHandleWhileSwappedOut(const IPC::Message& msg) OVERRIDE;
-  virtual std::string GetUserAgent(bool* overriding) const OVERRIDE;
-  virtual string16 GetLocalizedString(int message_id) const OVERRIDE;
-  virtual base::StringPiece GetDataResource(int resource_id) const OVERRIDE;
+  virtual void SetActiveURL(const GURL& url);
+  virtual void SetGpuInfo(const GPUInfo& gpu_info);
+  virtual void AddPepperPlugins(std::vector<PepperPluginInfo>* plugins);
+  virtual bool CanSendWhileSwappedOut(const IPC::Message* msg);
+  virtual bool CanHandleWhileSwappedOut(const IPC::Message& msg);
+  virtual std::string GetUserAgent(bool mimic_windows) const;
+  virtual string16 GetLocalizedString(int message_id) const;
+  virtual base::StringPiece GetDataResource(int resource_id) const;
 
 #if defined(OS_WIN)
   virtual bool SandboxPlugin(CommandLine* command_line,
-                             sandbox::TargetPolicy* policy) OVERRIDE;
+                             sandbox::TargetPolicy* policy);
 #endif
 };
 

chrome_frame/html_utils.cc

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 //
@@ -385,7 +385,7 @@ const char* GetChromeUserAgent() {
       product += version_info.is_valid() ? version_info.Version()
                                          : "0.0.0.0";
 
-      ua = webkit_glue::BuildUserAgentFromProduct(product);
+      ua = webkit_glue::BuildUserAgentHelper(false, product);
 
       DCHECK(ua.length() < arraysize(g_chrome_user_agent));
       lstrcpynA(g_chrome_user_agent, ua.c_str(),

chrome_frame/renderer_glue.cc

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -12,3 +12,15 @@ class GURL;
 bool IsPluginProcess() {
   return false;
 }
+
+namespace webkit_glue {
+
+std::string BuildUserAgent(bool mimic_windows) {
+  chrome::VersionInfo version_info;
+  std::string product("Chrome/");
+  product += version_info.is_valid() ? version_info.Version()
+                                     : "0.0.0.0";
+  return webkit_glue::BuildUserAgentHelper(mimic_windows, product);
+}
+
+}  // end namespace webkit_glue

chrome_frame/test/html_util_unittests.cc

@@ -20,12 +20,11 @@
 #include "net/base/net_util.h"
 
 #include "chrome/browser/automation/url_request_automation_job.h"
-#include "chrome/common/chrome_version_info.h"
 #include "chrome_frame/chrome_frame_automation.h"
 #include "chrome_frame/chrome_frame_delegate.h"
 #include "chrome_frame/html_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#include "webkit/glue/user_agent.h"
+#include "webkit/glue/webkit_glue.h"
 
 const char kChromeFrameUserAgent[] = "chromeframe";
 
@@ -404,15 +403,11 @@ TEST_F(HtmlUtilUnittest, GetDefaultUserAgentHeaderWithCFTag) {
 }
 
 TEST_F(HtmlUtilUnittest, GetChromeUserAgent) {
-  // This code is duplicated from chrome_content_client.cc to avoid
-  // introducing a link-time dependency on chrome_common.
-  chrome::VersionInfo version_info;
-  std::string product("Chrome/");
-  product += version_info.is_valid() ? version_info.Version() : "0.0.0.0";
-  std::string chrome_ua(webkit_glue::BuildUserAgentFromProduct(product));
-
+  std::string chrome_ua;
+  chrome_ua = webkit_glue::BuildUserAgent(false);
+  EXPECT_FALSE(chrome_ua.empty());
   const char* ua = http_utils::GetChromeUserAgent();
-  EXPECT_EQ(ua, chrome_ua);
+  EXPECT_EQ(0, chrome_ua.compare(ua));
 }
 
 TEST_F(HtmlUtilUnittest, GetDefaultUserAgent) {

content/common/child_thread.cc

@@ -41,6 +41,12 @@ void ChildThread::Init() {
   check_with_browser_before_shutdown_ = false;
   on_channel_error_called_ = false;
   message_loop_ = MessageLoop::current();
+  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kUserAgent)) {
+    webkit_glue::SetUserAgent(
+        CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+            switches::kUserAgent));
+  }
+
   channel_.reset(new IPC::SyncChannel(channel_name_,
       IPC::Channel::MODE_CLIENT, this,
       ChildProcess::current()->io_message_loop_proxy(), true,

content/common/content_client.cc

@@ -5,7 +5,6 @@
 #include "content/common/content_client.h"
 
 #include "base/string_piece.h"
-#include "webkit/glue/webkit_glue.h"
 
 namespace content {
 
@@ -13,19 +12,6 @@ static ContentClient* g_client;
 
 void SetContentClient(ContentClient* client) {
   g_client = client;
-
-  // TODO(dpranke): Doing real work (calling webkit_glue::SetUserAgent)
-  // inside what looks like a function that initializes a global is a
-  // bit odd, but we need to make sure this is done before
-  // webkit_glue::GetUserAgent() is called (so that the UA doesn't change).
-  //
-  // It would be cleaner if we could rename this to something like a
-  // content::Initialize(). Maybe we can merge this into ContentMain() somehow?
-  if (client) {
-    bool custom = false;
-    std::string ua = client->GetUserAgent(&custom);
-    webkit_glue::SetUserAgent(ua, custom);
-  }
 }
 
 ContentClient* GetContentClient() {

content/common/content_client.h

@@ -76,10 +76,10 @@ class CONTENT_EXPORT ContentClient {
   // behalf of a swapped out renderer.
   virtual bool CanHandleWhileSwappedOut(const IPC::Message& msg) = 0;
 
-  // Returns the user agent and a flag indicating whether the returned
-  // string should always be used (if false, callers may override the
-  // value as needed to work around various user agent sniffing bugs).
-  virtual std::string GetUserAgent(bool *overriding) const = 0;
+  // Returns the user agent. If mimic_windows is true then the embedder can
+  // return a fake Windows user agent. This is a workaround for broken
+  // websites.
+  virtual std::string GetUserAgent(bool mimic_windows) const = 0;
 
   // Returns a string resource given its id.
   virtual string16 GetLocalizedString(int message_id) const = 0;

content/renderer/renderer_glue.cc

@@ -247,4 +247,8 @@ base::StringPiece GetDataResource(int resource_id) {
   return content::GetContentClient()->GetDataResource(resource_id);
 }
 
+std::string BuildUserAgent(bool mimic_windows) {
+  return content::GetContentClient()->GetUserAgent(mimic_windows);
+}
+
 }  // namespace webkit_glue

content/shell/shell_content_client.cc

@@ -30,9 +30,8 @@ bool ShellContentClient::CanHandleWhileSwappedOut(const IPC::Message& msg) {
   return false;
 }
 
-std::string ShellContentClient::GetUserAgent(bool* overriding) const {
-  *overriding = false;
-  return std::string("Chrome/15.16.17.18");
+std::string ShellContentClient::GetUserAgent(bool mimic_windows) const {
+  return webkit_glue::BuildUserAgentHelper(mimic_windows, "Chrome/15.16.17.18");
 }
 
 string16 ShellContentClient::GetLocalizedString(int message_id) const {

content/shell/shell_content_client.h

@@ -21,7 +21,7 @@ class ShellContentClient : public ContentClient {
       std::vector<PepperPluginInfo>* plugins) OVERRIDE;
   virtual bool CanSendWhileSwappedOut(const IPC::Message* msg) OVERRIDE;
   virtual bool CanHandleWhileSwappedOut(const IPC::Message& msg) OVERRIDE;
-  virtual std::string GetUserAgent(bool* overriding) const OVERRIDE;
+  virtual std::string GetUserAgent(bool mimic_windows) const OVERRIDE;
   virtual string16 GetLocalizedString(int message_id) const OVERRIDE;
   virtual base::StringPiece GetDataResource(int resource_id) const OVERRIDE;
 

content/test/test_content_client.cc

@@ -31,9 +31,8 @@ bool TestContentClient::CanHandleWhileSwappedOut(const IPC::Message& msg) {
   return true;
 }
 
-std::string TestContentClient::GetUserAgent(bool* overriding) const {
-  *overriding = false;
-  return std::string("TestContentClient");
+std::string TestContentClient::GetUserAgent(bool mimic_windows) const {
+  return std::string();
 }
 
 string16 TestContentClient::GetLocalizedString(int message_id) const {

content/test/test_content_client.h

@@ -21,7 +21,7 @@ class TestContentClient : public content::ContentClient {
       std::vector<PepperPluginInfo>* plugins) OVERRIDE;
   virtual bool CanSendWhileSwappedOut(const IPC::Message* msg) OVERRIDE;
   virtual bool CanHandleWhileSwappedOut(const IPC::Message& msg) OVERRIDE;
-  virtual std::string GetUserAgent(bool* overriding) const OVERRIDE;
+  virtual std::string GetUserAgent(bool mimic_windows) const OVERRIDE;
   virtual string16 GetLocalizedString(int message_id) const OVERRIDE;
   virtual base::StringPiece GetDataResource(int resource_id) const OVERRIDE;
 #if defined(OS_WIN)

webkit/glue/user_agent.cc

@@ -113,7 +113,8 @@ int GetWebKitMinorVersion() {
   return WEBKIT_VERSION_MINOR;
 }
 
-std::string BuildUserAgentFromProduct(const std::string& product) {
+std::string BuildUserAgentHelper(bool mimic_windows,
+                                 const std::string& product) {
   const char kUserAgentPlatform[] =
 #if defined(OS_WIN)
       "";
@@ -127,6 +128,7 @@ std::string BuildUserAgentFromProduct(const std::string& product) {
 
   std::string user_agent;
 
+  // Replace Safari's Version/X string with the product name/version passed in.
   // This is done to expose our product name in a manner that is maximally
   // compatible with Safari, we hope!!
 
@@ -135,7 +137,7 @@ std::string BuildUserAgentFromProduct(const std::string& product) {
       &user_agent,
       "Mozilla/5.0 (%s%s) AppleWebKit/%d.%d"
       " (KHTML, like Gecko) %s Safari/%d.%d",
-      kUserAgentPlatform,
+      mimic_windows ? "Windows " : kUserAgentPlatform,
       webkit_glue::BuildOSCpuInfo().c_str(),
       WEBKIT_VERSION_MAJOR,
       WEBKIT_VERSION_MINOR,

webkit/glue/user_agent.h

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -21,9 +21,10 @@ std::string GetWebKitVersion();
 int GetWebKitMajorVersion();
 int GetWebKitMinorVersion();
 
-// Helper function to generate a full user agent string from a short
-// product name.
-std::string BuildUserAgentFromProduct(const std::string& product);
+// Helper function to generate the user agent.
+// - If mimic_windows is true, produce a fake Windows Chrome string..
+std::string BuildUserAgentHelper(bool mimic_windows,
+                                 const std::string& product);
 }  // namespace webkit_glue
 
 #endif  // WEBKIT_GLUE_USER_AGENT_H_

webkit/glue/webkit_glue.cc

@@ -343,11 +343,13 @@ class UserAgentState {
   UserAgentState();
   ~UserAgentState();
 
-  void Set(const std::string& user_agent, bool overriding);
+  void Set(const std::string& user_agent);
   const std::string& Get(const GURL& url) const;
 
  private:
   mutable std::string user_agent_;
+  // The UA string when we're pretending to be Windows Chrome.
+  mutable std::string mimic_windows_user_agent_;
   // The UA string when we're pretending to be Mac Safari.
   mutable std::string mimic_mac_safari_user_agent_;
 
@@ -367,18 +369,11 @@ UserAgentState::UserAgentState()
 UserAgentState::~UserAgentState() {
 }
 
-void UserAgentState::Set(const std::string& user_agent, bool overriding) {
+void UserAgentState::Set(const std::string& user_agent) {
   base::AutoLock auto_lock(lock_);
-  if (user_agent == user_agent_) {
-    // We allow the user agent to be set multiple times as long as it
-    // is set to the same value, in order to simplify unit testing
-    // given g_user_agent is a global.
-    return;
-  }
-  DCHECK(!user_agent.empty());
   DCHECK(!user_agent_requested_) << "Setting the user agent after someone has "
       "already requested it can result in unexpected behavior.";
-  user_agent_is_overridden_ = overriding;
+  user_agent_is_overridden_ = true;
   user_agent_ = user_agent;
 }
 
@@ -386,10 +381,21 @@ const std::string& UserAgentState::Get(const GURL& url) const {
   base::AutoLock auto_lock(lock_);
   user_agent_requested_ = true;
 
-  DCHECK(!user_agent_.empty());
+  if (user_agent_.empty())
+    user_agent_ = BuildUserAgent(false);
 
   // Workarounds for sites that use misguided UA sniffing.
   if (!user_agent_is_overridden_) {
+#if defined(OS_POSIX) && !defined(OS_MACOSX)
+    if (MatchPattern(url.host(), "*.mail.yahoo.com")) {
+      // mail.yahoo.com is ok with Windows Chrome but not Linux Chrome.
+      // http://bugs.chromium.org/11136
+      // TODO(evanm): remove this if Yahoo fixes their sniffing.
+      if (mimic_windows_user_agent_.empty())
+        mimic_windows_user_agent_ = BuildUserAgent(true);
+      return mimic_windows_user_agent_;
+    }
+#endif
 #if defined(OS_MACOSX)
     if (url.host() == "www.microsoft.com" &&
         StartsWithASCII(url.path(), "/getsilverlight", false)) {
@@ -399,7 +405,7 @@ const std::string& UserAgentState::Get(const GURL& url) const {
       // Should be removed if the sniffing is removed: http://crbug.com/88211
       if (mimic_mac_safari_user_agent_.empty()) {
         mimic_mac_safari_user_agent_ =
-            BuildUserAgentFromProduct("Version/5.0.4 Safari/533.20.27");
+            BuildUserAgentHelper(false, "Version/5.0.4 Safari/533.20.27");
       }
       return mimic_mac_safari_user_agent_;
     }
@@ -413,8 +419,8 @@ base::LazyInstance<UserAgentState> g_user_agent(base::LINKER_INITIALIZED);
 
 }  // namespace
 
-void SetUserAgent(const std::string& user_agent, bool overriding) {
-  g_user_agent.Get().Set(user_agent, overriding);
+void SetUserAgent(const std::string& new_user_agent) {
+  g_user_agent.Get().Set(new_user_agent);
 }
 
 const std::string& GetUserAgent(const GURL& url) {

webkit/glue/webkit_glue.h

@@ -88,14 +88,14 @@ string16 DumpFrameScrollPosition(WebKit::WebFrame* web_frame, bool recursive);
 string16 DumpHistoryState(const std::string& history_state, int indent,
                           bool is_current);
 
-// Sets the user agent.  Pass true for overriding if this is a custom
-// user agent instead of the default one (in order to turn off any browser
-// sniffing workarounds). This must be called before GetUserAgent() can
-// be called.
-void SetUserAgent(const std::string& user_agent, bool overriding);
-
-// Returns the user agent to use for the given URL. SetUserAgent() must
-// be called prior to calling this function.
+// Called to override the default user agent with a custom one.  Call this
+// before anyone actually asks for the user agent in order to prevent
+// inconsistent behavior.
+void SetUserAgent(const std::string& new_user_agent);
+
+// Returns the user agent to use for the given URL, which is usually the
+// default user agent but may be overriden by a call to SetUserAgent() (which
+// should be done at startup).
 const std::string& GetUserAgent(const GURL& url);
 
 // Creates serialized state for the specified URL. This is a variant of
@@ -147,6 +147,10 @@ WebKit::WebCanvas* ToWebCanvas(skia::PlatformCanvas*);
 // used to get memory usage statistics.
 int GetGlyphPageCount();
 
+// Construct the User-Agent header, filling in |result|.
+// - If mimic_windows is true, produce a fake Windows Chrome string.
+std::string BuildUserAgent(bool mimic_windows);
+
 //---- END FUNCTIONS IMPLEMENTED BY WEBKIT/GLUE -------------------------------
 
 

webkit/support/webkit_support.cc

@@ -264,7 +264,6 @@ static void SetUpTestEnvironmentImpl(bool unit_test_mode) {
     // because on Linux, we need base::AtExitManager.
     icu_util::Initialize();
   }
-  webkit_glue::SetUserAgent("DumpRenderTree", false);
 }
 
 void SetUpTestEnvironment() {

webkit/support/webkit_support_glue.cc

@@ -48,6 +48,11 @@ bool IsProtocolSupportedForMedia(const GURL& url) {
   return false;
 }
 
+std::string BuildUserAgent(bool mimic_windows) {
+  return webkit_glue::BuildUserAgentHelper(mimic_windows,
+                                           "DumpRenderTree/0.0.0.0");
+}
+
 bool GetPluginFinderURL(std::string* plugin_finder_url) {
   return false;
 }

webkit/tools/test_shell/test_shell.cc

@@ -146,7 +146,6 @@ TestShell::TestShell()
     filter->AddHostnameHandler("test-shell-resource", "inspector",
                                &URLRequestTestShellFileJob::InspectorFactory);
     url_util::AddStandardScheme("test-shell-resource");
-    webkit_glue::SetUserAgent("TestShell", false);
 }
 
 TestShell::~TestShell() {
@@ -636,6 +635,10 @@ bool IsProtocolSupportedForMedia(const GURL& url) {
   return false;
 }
 
+std::string BuildUserAgent(bool mimic_windows) {
+  return webkit_glue::BuildUserAgentHelper(mimic_windows, "Chrome/0.0.0.0");
+}
+
 #if defined(OS_LINUX)
 int MatchFontWithFallback(const std::string& face, bool bold,
                           bool italic, int charset) {