2011-02-03 Mark Mentovai <mark@chromium.org>
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Feb 2011 00:46:32 +0000 (00:46 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Feb 2011 00:46:32 +0000 (00:46 +0000)
        Reviewed by Dimitri Glazkov.

        Chromium GYP build fix.

        When various settings were moved to webcore_prerequisites in r66364,
        things that should have been direct_dependent_settings were not marked
        as such. GYP 'defines', for example, make no sense on a 'none'-type
        target such as webcore_prerequisites. It appears that it was intended
        for these settings to be pushed to direct dependents, which would make
        direct_dependent_settings correct.

        Losing the ChromiumWebCoreObjC defines on the Mac, for example, caused
        http://crbug.com/71537, which at best causes Mac console log spew, and
        at worst may result in Chromium's copy of WebCore using system
        definitions of certain Objective-C classes at runtime, or vice-versa.

        The build now includes a postbuild step to prevent
        http://crbug.com/71537 from regressing again. The build will fail upon
        regression.

        https://bugs.webkit.org/show_bug.cgi?id=53630

        * WebCore.gyp/WebCore.gyp: Move things in webcore_prerequisites into
          direct_dependent_settings as needed, add the check_objc_rename
          postbuild step.
        * WebCore.gyp/mac/check_objc_rename.sh: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@77562 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/WebCore.gyp/WebCore.gyp
Source/WebCore/WebCore.gyp/mac/check_objc_rename.sh [new file with mode: 0644]

index 263418f8d55687cb57baf1285dfa62dce4958d11..9223cc1713476115d4d912052b6dc400af331b83 100644 (file)
@@ -1,3 +1,32 @@
+2011-02-03  Mark Mentovai  <mark@chromium.org>
+
+        Reviewed by Dimitri Glazkov.
+
+        Chromium GYP build fix.
+
+        When various settings were moved to webcore_prerequisites in r66364,
+        things that should have been direct_dependent_settings were not marked
+        as such. GYP 'defines', for example, make no sense on a 'none'-type
+        target such as webcore_prerequisites. It appears that it was intended
+        for these settings to be pushed to direct dependents, which would make
+        direct_dependent_settings correct.
+
+        Losing the ChromiumWebCoreObjC defines on the Mac, for example, caused
+        http://crbug.com/71537, which at best causes Mac console log spew, and
+        at worst may result in Chromium's copy of WebCore using system
+        definitions of certain Objective-C classes at runtime, or vice-versa.
+
+        The build now includes a postbuild step to prevent
+        http://crbug.com/71537 from regressing again. The build will fail upon
+        regression.
+
+        https://bugs.webkit.org/show_bug.cgi?id=53630
+
+        * WebCore.gyp/WebCore.gyp: Move things in webcore_prerequisites into
+          direct_dependent_settings as needed, add the check_objc_rename
+          postbuild step.
+        * WebCore.gyp/mac/check_objc_rename.sh: Added.
+
 2011-02-03  Adam Barth  <abarth@webkit.org>
 
         Reviewed by Eric Seidel.
index 70df557765016e4c268c02fea8489617ef094577..6051bfdd2bd63ac3177b0d6144185a45547f3543 100644 (file)
           ],
           'conditions': [
             ['inside_chromium_build==1 and OS=="win" and component=="shared_library"', {
-              'defines': [
-                'USING_V8_SHARED',
-              ],
+              'direct_dependent_settings': {
+                'defines': [
+                  'USING_V8_SHARED',
+                ],
+              },
             }],
           ],
         }],
             '<(chromium_src_dir)/build/linux/system.gyp:fontconfig',
             '<(chromium_src_dir)/build/linux/system.gyp:gtk',
           ],
-          'cflags': [
-            # WebCore does not work with strict aliasing enabled.
-            # https://bugs.webkit.org/show_bug.cgi?id=25864
-            '-fno-strict-aliasing',
-          ],
+          'direct_dependent_settings': {
+            'cflags': [
+              # WebCore does not work with strict aliasing enabled.
+              # https://bugs.webkit.org/show_bug.cgi?id=25864
+              '-fno-strict-aliasing',
+            ],
+          },
         }],
         ['OS=="linux"', {
-          'defines': [
-            # Mozilla on Linux effectively uses uname -sm, but when running
-            # 32-bit x86 code on an x86_64 processor, it uses
-            # "Linux i686 (x86_64)".  Matching that would require making a
-            # run-time determination.
-            'WEBCORE_NAVIGATOR_PLATFORM="Linux i686"',
-          ],
+          'direct_dependent_settings': {
+            'defines': [
+              # Mozilla on Linux effectively uses uname -sm, but when running
+              # 32-bit x86 code on an x86_64 processor, it uses
+              # "Linux i686 (x86_64)".  Matching that would require making a
+              # run-time determination.
+              'WEBCORE_NAVIGATOR_PLATFORM="Linux i686"',
+            ],
+          },
         }],
         ['OS=="mac"', {
           'dependencies': [
           'export_dependent_settings': [
             'webkit_system_interface',
           ],
-          'defines': [
-            # Match Safari and Mozilla on Mac x86.
-            'WEBCORE_NAVIGATOR_PLATFORM="MacIntel"',
-
-            # Chromium's version of WebCore includes the following Objective-C
-            # classes. The system-provided WebCore framework may also provide
-            # these classes. Because of the nature of Objective-C binding
-            # (dynamically at runtime), it's possible for the Chromium-provided
-            # versions to interfere with the system-provided versions.  This may
-            # happen when a system framework attempts to use WebCore.framework,
-            # such as when converting an HTML-flavored string to an
-            # NSAttributedString.  The solution is to force Objective-C class
-            # names that would conflict to use alternate names.
+          'direct_dependent_settings': {
+            'defines': [
+              # Match Safari and Mozilla on Mac x86.
+              'WEBCORE_NAVIGATOR_PLATFORM="MacIntel"',
 
-            # FIXME: This list will hopefully shrink but may also grow.
-            # Periodically run:
-            # nm libwebcore.a | grep -E '[atsATS] ([+-]\[|\.objc_class_name)'
-            # and make sure that everything listed there has the alternate
-            # ChromiumWebCoreObjC name, and that nothing extraneous is listed
-            # here. If all Objective-C can be eliminated from Chromium's WebCore
-            # library, these defines should be removed entirely.
-            'ScrollbarPrefsObserver=ChromiumWebCoreObjCScrollbarPrefsObserver',
-            'WebCoreRenderThemeNotificationObserver=ChromiumWebCoreObjCWebCoreRenderThemeNotificationObserver',
-            'WebFontCache=ChromiumWebCoreObjCWebFontCache',
-          ],
-          'include_dirs': [
-            '../../../WebKitLibraries',
-          ],
+              # Chromium's version of WebCore includes the following Objective-C
+              # classes. The system-provided WebCore framework may also provide
+              # these classes. Because of the nature of Objective-C binding
+              # (dynamically at runtime), it's possible for the
+              # Chromium-provided versions to interfere with the system-provided
+              # versions.  This may happen when a system framework attempts to
+              # use WebCore.framework, such as when converting an HTML-flavored
+              # string to an NSAttributedString.  The solution is to force
+              # Objective-C class names that would conflict to use alternate
+              # names.
+              #
+              # This list will hopefully shrink but may also grow.  Its
+              # performance is monitored by the "Check Objective-C Rename"
+              # postbuild step, and any suspicious-looking symbols not handled
+              # here or whitelisted in that step will cause a build failure.
+              #
+              # If this is unhandled, the console will receive log messages
+              # such as:
+              # com.google.Chrome[] objc[]: Class ScrollbarPrefsObserver is implemented in both .../Google Chrome.app/Contents/Versions/.../Google Chrome Helper.app/Contents/MacOS/../../../Google Chrome Framework.framework/Google Chrome Framework and /System/Library/Frameworks/WebKit.framework/Versions/A/Frameworks/WebCore.framework/Versions/A/WebCore. One of the two will be used. Which one is undefined.
+              'ScrollbarPrefsObserver=ChromiumWebCoreObjCScrollbarPrefsObserver',
+              'WebCoreRenderThemeNotificationObserver=ChromiumWebCoreObjCWebCoreRenderThemeNotificationObserver',
+              'WebFontCache=ChromiumWebCoreObjCWebFontCache',
+            ],
+            'include_dirs': [
+              '../../../WebKitLibraries',
+            ],
+            'postbuilds': [
+              {
+                # This step ensures that any Objective-C names that aren't
+                # redefined to be "safe" above will cause a build failure.
+                'postbuild_name': 'Check Objective-C Rename',
+                'variables': {
+                  'class_whitelist_regex':
+                      'ChromiumWebCoreObjC|TCMVisibleView|RTCMFlippedView',
+                  'category_whitelist_regex':
+                      'TCMInterposing',
+                },
+                'action': [
+                  'mac/check_objc_rename.sh',
+                  '<(class_whitelist_regex)',
+                  '<(category_whitelist_regex)',
+                ],
+              },
+            ],
+          },
         }],
         ['OS=="win"', {
           'dependencies': [
           'export_dependent_settings': [
             '<(chromium_src_dir)/build/win/system.gyp:cygwin'
           ],
-          'defines': [
-            # Match Safari and Mozilla on Windows.
-            'WEBCORE_NAVIGATOR_PLATFORM="Win32"',
-            '__PRETTY_FUNCTION__=__FUNCTION__',
-          ],
-          # This is needed because Event.h in this directory is blocked
-          # by a system header on windows.
-          'include_dirs++': ['../dom'],
+          'direct_dependent_settings': {
+            'defines': [
+              # Match Safari and Mozilla on Windows.
+              'WEBCORE_NAVIGATOR_PLATFORM="Win32"',
+              '__PRETTY_FUNCTION__=__FUNCTION__',
+            ],
+            # This is needed because Event.h in this directory is blocked
+            # by a system header on windows.
+            'include_dirs++': ['../dom'],
+          },
         }],
         ['(OS=="linux" or OS=="win") and "WTF_USE_WEBAUDIO_MKL=1" in feature_defines', {
           # This directory needs to be on the include path for multiple sub-targets of webcore.
diff --git a/Source/WebCore/WebCore.gyp/mac/check_objc_rename.sh b/Source/WebCore/WebCore.gyp/mac/check_objc_rename.sh
new file mode 100644 (file)
index 0000000..ed3c119
--- /dev/null
@@ -0,0 +1,79 @@
+#!/bin/bash
+
+#
+# Copyright (C) 2011 Google Inc. All rights reserved.
+# 
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+# 
+#     * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#     * 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.
+#     * Neither the name of Google Inc. 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 THE COPYRIGHT HOLDERS AND 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 THE COPYRIGHT
+# OWNER 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.
+#
+
+# This script checks a WebCore static library for potential Objective-C
+# class name collisions with the system's copy of the WebCore framework.
+# See the postbuild action that calls it from ../WebCore.gyp for details.
+
+set -e
+set -o pipefail
+
+if [[ $# -ne 2 ]]; then
+  echo "usage: ${0} class_whitelist_pattern category_whitelist_pattern" >& 2
+  exit 1
+fi
+
+lib="${BUILT_PRODUCTS_DIR}/${FULL_PRODUCT_NAME}"
+nm_pattern='[atsATS] ([+-]\[|\.objc_class_name_)'
+
+class_whitelist_pattern="${1}"
+category_whitelist_pattern="${2}"
+
+# Send nm's stderr in the pipeline to /dev/null to avoid spewing
+# "nm: no name list" messages. This means that if the pipelined nm fails, there
+# won't be any output, so if the entire assignment fails, run nm again to get
+# some output.
+violators=$(nm -p "${lib}" 2> /dev/null | \
+    (grep -E "${nm_pattern}" || true) | \
+    (grep -Ev "${nm_pattern}(${class_whitelist_pattern})" || true) | \
+    (grep -Ev "\((${category_whitelist_pattern})\)" || true)) || nm -p "${lib}"
+
+if [[ -z "${violators}" ]]; then
+  # An empty list means that everything's clean.
+  exit 0
+fi
+
+cat << __EOF__ >&2
+These Objective-C symbols may clash with those provided by the system's own
+WebCore framework:
+${violators}
+
+These symbols were found in:
+${lib}
+
+This should be corrected by adding the appropriate definitions to
+$(dirname ${0})/../WebCore.gyp
+or by updating the whitelist in
+${0}
+__EOF__
+
+exit 1