FrameLoaderClientGtk hardcodes data, including platform to Linux i686

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Ed Schouten]

The FrameLoaderClientGtk class hardcodes the string:

Mozilla/5.0 (X11; U; Linux i686; en-US) AppleWebKit/420+ (KHTML, like Gecko)

This is obviously wrong in a lot of cases. This makes the browser show up wrong in webserver logs in a lot of cases.

[Ed Schouten]

Created attachment 16574 [details]
Use uname(3) to read the sysname + machine

This unhardcodes already two of the things, namely the operating system and platform.

[Mark Rowe (bdash)]

Other portions of the user agent string that are incorrect include the WebKit version number, locale, and potentially the "X11".

[Adam Roben (:aroben)]

Comment on attachment 16574 [details]
Use uname(3) to read the sysname + machine

The error handling of the call to uname seems a bit dodgy -- do we really want to have a user-agent string that doesn't have any system/architecture at all?

This definitely seems like a step in the right direction, but I think we can make this fix even more complete pretty easily:

1. As Mark said, the WebKit version number should match the one used on OS X, since that's what most websites will be checking for (<http://trac.webkit.org/projects/webkit/wiki/DetectingWebKit>).
2. As Mark said, the "X11" part of the user-agent string may not always be correct -- what about Gtk+ on Win32?
3. Also in the Win32 vein, I don't think Win32 has a uname function, so I don't think this code will compile on Gtk+/Win32. But maybe we don't care about that?

r- for now so we can figure out the above issues.

[Ed Schouten]

Another thing we should pay attention to:
According to kalikiana on IRC, we can figure out the locale by using g_get_language_names().

Adam,

1. I'll see if there is some kind of #define in the source tree, representing the WebKit version number.
2. Maybe it would be better if we'd say `GTK+' instead of `X11; U'. That would be applicable to all systems.
3. It would just chicken out during compilation. I think we'll see a second patch if someone's really interested in using it on Windows.

[Ed Schouten]

Created attachment 16590 [details]
Use GTK+ instead of X11; U. Also unhardcode the language.

I'm not sure how to check for the version number in the source code. It seems the XCode project sets FULL_VERSION, but it's not set in the Makefile.

[Adam Roben (:aroben)]

Comment on attachment 16590 [details]
Use GTK+ instead of X11; U. Also unhardcode the language.

+    m_userAgent = (WebCore::String)"Mozilla/5.0 (GTK+; " + un.sysname + " " + un.machine + "; " + g_get_language_names()[0] + ") AppleWebKit/420+ (KHTML, like Gecko)";

I think it would be better to use String::format() here instead of building this string by appending.

We should also add a FIXME about getting the version number correct.

[Mark Rowe (bdash)]

*** Bug 15591 has been marked as a duplicate of this bug. ***

[Mark Rowe (bdash)]

The most recent patch on bug 15591, which I just duped against this bug, is probably a better basis for work going forwards.

[Mark Rowe (bdash)]

A couple of comments on the patch...  The values returned from uname are not what you would want when running on the Mac.  They return "Darwin" and "i386" when the user agent should have "Intel Mac OS X" for consistency with other browsers.  The WebKit version number really needs to be kept up to date.  Perhaps it could be based on the most recent version number used on Windows, which is stored in a plain text file at WebKit/win/WebKit.vcproj/VERSION.

Ideally, IMO, the user agent string should be as similar as possible to the native Mac and Windows ports when running on those platforms.  For other platforms, I would suggest comparing looking at precisely which fields Firefox changes and following their lead.

[Christian Dywan]

Created attachment 16793 [details]
Based on #15591, better Mac Kernel Value, overriding possible

This patch is based on #15591, but stores the agent and application name in variables and allows for custom values. The Mac kernel value is now either 'PPC Mac OS X' or 'Intel Mac OS X'. The WebKit version is 523+ for now.

The 'Kernel' value should possibly come from a new function which could be shared among ports, which would also provide Windows version.

[Christian Dywan]

Created attachment 16804 [details]
Share systemName, include Windows version

This patch adds systemName to WebCore so that code can be shared with other ports. The Windows version is a copy of osVersion from WebKitWin.

[Alp Toker]

Christian: Can you mark this patch for review if you think it's ready? I'm starting to like the current state of it, and this is a bug that needs to be addressed.

The g_get_prgname() idea is quite elegant :-)

There is still the "523+" but that can be dealt with in a follow up patch.

So, I'd like one of the core developers with UserAgent experience to review this patch, especially with a view to sharing code with the Win port and others.

[Holger Freyther]

(In reply to comment #11)
> Created an attachment (id=16804) [edit]
> Share systemName, include Windows version
> 
> This patch adds systemName to WebCore so that code can be shared with other
> ports. The Windows version is a copy of osVersion from WebKitWin.
>

I would like to see this code move to WebKitPage. E.g. the macintosh API allows to override the UserAgent on a per URL basis and to allow this easily, the code should move into the page.

[Ed Schouten]

Sorry if I complain complain a little, but I think that assuming "Linux i686" is a little bit wrong. Why not just add "Unknown" or something to the string?

[Alp Toker]

Comment on attachment 16804 [details]
Share systemName, include Windows version

>Index: WebCore/ChangeLog
>===================================================================
>--- WebCore/ChangeLog	(revision 26888)
>+++ WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,19 @@
>[phone]  Christian Dywan  <[e-mail]>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        WARNING: NO TEST CASES ADDED OR CHANGED
>+
>+        Implement systemName for use in the user agent.
>+
>+        * WebCore.pro:
>+        * platform/SystemName.cpp: Added.
>+        (WebCore::systemName):
>+        * platform/SystemName.h: Added.
>+        * platform/win/SystemNameWin.cpp: Added.
>+        (WebCore::windowsVersion):
>+        * platform/win/SystemNameWin.h: Added.
>+
> 2007-10-22  Nikolas Zimmermann  <[e-mail]>
> 
>         Reviewed by Simon.
>Index: WebCore/WebCore.pro
>===================================================================
>--- WebCore/WebCore.pro	(revision 26888)
>+++ WebCore/WebCore.pro	(working copy)
>@@ -669,6 +669,7 @@ SOURCES += \
>     platform/SharedBuffer.cpp \
>     platform/String.cpp \
>     platform/StringImpl.cpp \
>+    platform/SystemName.cpp \
>     platform/TextCodec.cpp \
>     platform/TextCodecLatin1.cpp \
>     platform/TextCodecUTF16.cpp \
>@@ -955,6 +956,9 @@ gtk-port {
>         ../WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp \
>         ../WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp \
>         ../WebKit/gtk/WebCoreSupport/InspectorClientGtk.cpp
>+
>+    unix: 
>+    else: SOURCES += platform/win/SystemNameWin.cpp
> }
> 
> # ENABLE_DATABASE probably cannot be disabled without breaking things
>Index: WebCore/platform/SystemName.cpp
>===================================================================
>--- WebCore/platform/SystemName.cpp	(revision 0)
>+++ WebCore/platform/SystemName.cpp	(revision 0)
>@@ -0,0 +1,63 @@
>+/*
>+ * Copyright (C) 2007 Christian Dywan
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
>+ */
>+
>+#include "config.h"
>+#include "SystemName.h"
>+
>+#include "CString.h"
>+#include "PlatformString.h"
>+
>+#if PLATFORM(UNIX)
>+#include <sys/utsname.h>
>+#elif defined(PLATFORM(WIN))
>+#include <win/SystemNameWin.h>
>+#endif
>+
>+namespace WebCore {
>+
>+String systemName(void)
>+{
>+    #if PLATFORM(MAC)
>+        #if PLATFORM(X86)
>+        return "Intel Mac OS X";
>+        #else
>+        return "PPC Mac OS X";
>+        #endif
>+    #elif PLATFORM(UNIX)
>+    struct utsname name;
>+    if(uname(&name) != -1){
>+        return String::format("%s %s", name.sysname, name.machine);
>+    }
>+    else{
>+        return "Linux i686";
>+    }

Coding style: no braces needed here. I'm pretty sure "Linux i686" is not a good default value here. Better to either return some kind of "Unknown" or otherwise pass up an error.

>+    #elif PLATFORM(WIN)
>+    return windowsVersion();
>+    #else
>+    return "Unknown";
>+    #endif
>+}
>+
>+}
>Index: WebCore/platform/SystemName.h
>===================================================================
>--- WebCore/platform/SystemName.h	(revision 0)
>+++ WebCore/platform/SystemName.h	(revision 0)
>@@ -0,0 +1,37 @@
>+/*
>+ * Copyright (C) 2007 Christian Dywan
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
>+ */
>+
>+#ifndef SystemName_h
>+#define SystemName_h
>+
>+namespace WebCore {
>+
>+    class String;
>+
>+    String systemName();
>+
>+}
>+
>+#endif // SystemName_h
>Index: WebCore/platform/win/SystemNameWin.cpp
>===================================================================
>--- WebCore/platform/win/SystemNameWin.cpp	(revision 0)
>+++ WebCore/platform/win/SystemNameWin.cpp	(revision 0)
>@@ -0,0 +1,66 @@
>+/*
>+ * Copyright (C) 2006, 2007 Apple Inc. All rights reserved.
>+ * Copyright (C) 2007 Christian Dywan
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ *
>+ * 1.  Redistributions of source code must retain the above copyright
>+ *     notice, this list of conditions and the following disclaimer. 
>+ * 2.  Redistributions in binary form must reproduce the above copyright
>+ *     notice, this list of conditions and the following disclaimer in the
>+ *     documentation and/or other materials provided with the distribution. 
>+ * 3.  Neither the name of Apple Computer, Inc. ("Apple") nor the names of
>+ *     its contributors may be used to endorse or promote products derived
>+ *     from this software without specific prior written permission. 
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
>+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
>+ * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
>+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
>+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
>+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
>+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
>+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#include "config.h"
>+#include "SystemNameWin.h"
>+
>+#include "CString.h"
>+#include "PlatformString.h"
>+
>+#include <windowsx.h>
>+
>+namespace WebCore {
>+
>+// Copied from osVersion, WebKit/win/WebView.cpp, line 1593
>+String windowsVersion()
>+{
>+    String osVersion;
>+    OSVERSIONINFO versionInfo = {0};
>+    versionInfo.dwOSVersionInfoSize = sizeof(versionInfo);
>+    GetVersionEx(&versionInfo);
>+
>+    if (versionInfo.dwPlatformId == VER_PLATFORM_WIN32_WINDOWS) {
>+        if (versionInfo.dwMajorVersion == 4) {
>+            if (versionInfo.dwMinorVersion == 0)
>+                osVersion = "Windows 95";
>+            else if (versionInfo.dwMinorVersion == 10)
>+                osVersion = "Windows 98";
>+            else if (versionInfo.dwMinorVersion == 90)
>+                osVersion = "Windows 98; Win 9x 4.90";
>+        }
>+    } else if (versionInfo.dwPlatformId == VER_PLATFORM_WIN32_NT)
>+        osVersion = String::format("Windows NT %d.%d", versionInfo.dwMajorVersion, versionInfo.dwMinorVersion);
>+
>+    if (!osVersion.length())
>+        osVersion = String::format("Windows %d.%d", versionInfo.dwMajorVersion, versionInfo.dwMinorVersion);
>+
>+    return osVersion;
>+}
>+
>+} // namespace WebCore

Someone familiar with these issues on Windows should review this part.

>Index: WebCore/platform/win/SystemNameWin.h
>===================================================================
>--- WebCore/platform/win/SystemNameWin.h	(revision 0)
>+++ WebCore/platform/win/SystemNameWin.h	(revision 0)
>@@ -0,0 +1,35 @@
>+/*
>+ * Copyright (C) 2007 Christian Dywan
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
>+ */
>+
>+#ifndef SystemNameWin_h
>+#define SystemNameWin_h
>+
>+namespace WebCore {
>+
>+String windowsVersion(void);
>+
>+}
>+
>+#endif // SystemNameWin_h
>Index: WebKit/gtk/ChangeLog
>===================================================================
>--- WebKit/gtk/ChangeLog	(revision 26888)
>+++ WebKit/gtk/ChangeLog	(working copy)
>@@ -1,3 +1,16 @@
>[phone]  Christian Dywan  <[e-mail]>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Computer a proper user agent.
>+
>+        * ChangeLog:
>+        * WebCoreSupport/FrameLoaderClientGtk.cpp:
>+        (WebKit::composeUserAgent):
>+        (WebKit::FrameLoaderClient::FrameLoaderClient):
>+        (WebKit::FrameLoaderClient::userAgent):
>+        * WebCoreSupport/FrameLoaderClientGtk.h:
>+
> 2007-10-22  Alp Toker  <[e-mail]>
> 
>         Reviewed by Mark Rowe.
>Index: WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp
>===================================================================
>--- WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp	(revision 26888)
>+++ WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp	(working copy)
>@@ -40,10 +40,12 @@
> #include "HTMLFrameElement.h"
> #include "HTMLFrameOwnerElement.h"
> #include "HTMLNames.h"
>+#include "Language.h"
> #include "MIMETypeRegistry.h"
> #include "NotImplemented.h"
> #include "PlatformString.h"
> #include "ResourceRequest.h"
>+#include "SystemName.h"
> #include "CString.h"
> #include "ProgressTracker.h"
> #include "webkitgtkpage.h"
>@@ -55,16 +57,54 @@ using namespace WebCore;
> 
> namespace WebKit {
> 
>+static String composeUserAgent(String applicationName)
>+{
>+    String operatingSystem;
>+
>+    #ifdef GDK_WINDOWING_X11
>+    operatingSystem = "X11";
>+    #elif defined(GTK_WINDOWING_WIN32)
>+    operatingSystem = "Windows";
>+    #elif defined(GTK_WINDOWING_QUARTZ)
>+    operatingSystem = "Macintosh";
>+    #elif defined(GTK_WINDOWING_DIRECTFB)
>+    operatingSystem = "DirectFB";
>+    #else
>+    operatingSystem = "Unknown";
>+    #endif

Perhaps there should be a notImplemented() in the "Unknown" case.

>+
>+    // FIXME: The WebKit version is hardcoded
>+
>+    return String::format("Mozilla/5.0 (%s; U; %s; %s) AppleWebKit/%s (KHTML, like Gecko) %s"
>+        , operatingSystem.utf8().data()
>+        , systemName().utf8().data()
>+        , defaultLanguage().utf8().data()
>+        , "523+"
>+        , applicationName.utf8().data());
>+}

We can fix the hard coded version in a later patch, this is fine for now. bdash has some ideas here.

>+
> FrameLoaderClient::FrameLoaderClient(WebKitFrame* frame)
>     : m_frame(frame)
>     , m_firstData(false)
> {
>     ASSERT(m_frame);
>+
>+    m_applicationNameStandard = g_get_prgname();
> }
> 
> String FrameLoaderClient::userAgent(const KURL&)
> {
>-    return "Mozilla/5.0 (X11; U; Linux i686; en-US) AppleWebKit/420+ (KHTML, like Gecko)";
>+    if(m_userAgentOverridden)
>+        return m_userAgentCustom;
>+
>+    if(!m_userAgentStandard.length()){
>+        if(m_applicationNameOverridden)
>+            m_userAgentStandard = composeUserAgent(m_applicationNameCustom);
>+        else
>+            m_userAgentStandard = composeUserAgent(m_applicationNameStandard);
>+    }
>+
>+    return m_userAgentStandard;
> }
> 
> WTF::PassRefPtr<WebCore::DocumentLoader> FrameLoaderClient::createDocumentLoader(const WebCore::ResourceRequest& request, const SubstituteData& substituteData)
>Index: WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.h
>===================================================================
>--- WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.h	(revision 26888)
>+++ WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.h	(working copy)
>@@ -180,6 +180,12 @@ namespace WebKit {
>     private:
>         WebKitFrame* m_frame;
>         WebCore::ResourceResponse m_response;
>+        WebCore::String m_userAgentStandard;
>+        WebCore::String m_userAgentCustom;
>+        WebCore::String m_applicationNameStandard;
>+        WebCore::String m_applicationNameCustom;
>+        bool m_userAgentOverridden;
>+        bool m_applicationNameOverridden;
>         bool m_firstData;
>     };
> 


Can you consider splitting your patch into two parts?

I can review the GTK+ part straight away, and we can add the platform/SystemName.h part afterwards, since that part needs wider review particularly as ports might be able to share it.

[Alp Toker]

if(!m_userAgentStandard.length())

isEmpty() will do here.

[Christian Dywan]

Created attachment 17044 [details]
Gtk only part, imrpoved based on comments

[Christian Dywan]

Created attachment 17046 [details]
Gtk only part, Updated if and #if style

[Christian Dywan]

Created attachment 17051 [details]
No more string abuse but clean concatenation

[Alp Toker]

Comment on attachment 17051 [details]
No more string abuse but clean concatenation

This patch returns an empty string for the UA on Release builds due to uninitialised variables. I think it would be best to remove the "custom" variables for now since they're not yet used.

[Alp Toker]

Created attachment 17053 [details]
Reduction of the patch

This is a reduced version of the patch which removes the incomplete customisation support in previous versions and fixes a few coding style issues.

Christian: If you like my changes, please mark this patch for review.

We can add customisation and Windows support later but I think this is important to get included ASAP.

[Alp Toker]

Comment on attachment 17053 [details]
Reduction of the patch

On behalf of Christian. Please review.

[Maciej Stachowiak]

Comment on attachment 17053 [details]
Reduction of the patch

I would recommend PLATFORM(DARWIN), not PLATFORM(MAC) for the agentOS function
similarly, PLATFORM(WIN_OS) for the Windows check
Otherwise r=me

[Alp Toker]

Landed in r27460.