.:

[GTK] Reenable -fvisibility=hidden https://bugs.webkit.org/show_bug.cgi?id=181916 Patch by Michael Catanzaro <mcatanzaro@gnome.org> on 2021-03-09 Reviewed by Don Olmstead. In non-DEVELOPER_MODE builds, we rely on a linker version script to hide symbols that we don't want to export. Building with hidden visibility might seem redundant with this, but actually building with hidden visibility has advantages anyway. See https://gcc.gnu.org/wiki/Visibility. Note that I'm not confident GTK port can safely use -fvisibility-inlines-hidden, since it's split between two shared objects. Also, because GTK is split into two shared objects, GTK needs to build bmalloc and WTF as CMake OBJECT libraries, which is effectively the same as using -Wl,--whole-archive to prevent symbols from being prematurely stripped away. P.S. Major credit to Don Olmstead, who did most of the work to make this possible, which has already landed in previous patches. * Source/cmake/OptionsGTK.cmake: Source/WebCore: [WPE][GTK] Reenable -fvisibility=hidden (and -fvisibility-inlines-hidden for WPE) https://bugs.webkit.org/show_bug.cgi?id=181916 Patch by Michael Catanzaro <mcatanzaro@gnome.org> on 2021-03-09 Reviewed by Don Olmstead. We need to export the destructor of EventTarget. * PlatformGTK.cmake: * dom/EventTarget.cpp: * dom/EventTarget.h: Tools: [GTK] Reenable -fvisibility=hidden https://bugs.webkit.org/show_bug.cgi?id=181916 Patch by Michael Catanzaro <mcatanzaro@gnome.org> on 2021-03-09 Reviewed by Don Olmstead. * TestWebKitAPI/PlatformGTK.cmake: * TestWebKitAPI/glib/TestExpectations.json: Canonical link: https://commits.webkit.org/235087@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@274166 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Mar 9, 2021 · 505edb6 · 505edb6
1 parent 3af29ce
commit 505edb6
Show file tree

Hide file tree

Showing 9 changed files with 56 additions and 11 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -1,3 +1,25 @@
+2021-03-09  Michael Catanzaro  <mcatanzaro@gnome.org>
+
+        [GTK] Reenable -fvisibility=hidden
+        https://bugs.webkit.org/show_bug.cgi?id=181916
+
+        Reviewed by Don Olmstead.
+
+        In non-DEVELOPER_MODE builds, we rely on a linker version script to hide symbols that we
+        don't want to export. Building with hidden visibility might seem redundant with this, but
+        actually building with hidden visibility has advantages anyway. See
+        https://gcc.gnu.org/wiki/Visibility.
+
+        Note that I'm not confident GTK port can safely use -fvisibility-inlines-hidden, since it's
+        split between two shared objects. Also, because GTK is split into two shared objects, GTK
+        needs to build bmalloc and WTF as CMake OBJECT libraries, which is effectively the same as
+        using -Wl,--whole-archive to prevent symbols from being prematurely stripped away.
+
+        P.S. Major credit to Don Olmstead, who did most of the work to make this possible, which has
+        already landed in previous patches.
+
+        * Source/cmake/OptionsGTK.cmake:
+
 2021-03-05  Michael Catanzaro  <mcatanzaro@gnome.org>
 
         [GTK] Remove ADD_WHOLE_ARCHIVE_TO_LIBRARIES

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,16 @@
+2021-03-09  Michael Catanzaro  <mcatanzaro@gnome.org>
+
+        [WPE][GTK] Reenable -fvisibility=hidden (and -fvisibility-inlines-hidden for WPE)
+        https://bugs.webkit.org/show_bug.cgi?id=181916
+
+        Reviewed by Don Olmstead.
+
+        We need to export the destructor of EventTarget.
+
+        * PlatformGTK.cmake:
+        * dom/EventTarget.cpp:
+        * dom/EventTarget.h:
+
 2021-03-09  Antoine Quint  <graouts@webkit.org>
 
         [Web Animations] setKeyframes does not preserve animation's current offset

diff --git a/Source/WebCore/PlatformGTK.cmake b/Source/WebCore/PlatformGTK.cmake
@@ -8,13 +8,6 @@ include(platform/TextureMapper.cmake)
 
 set(WebCore_OUTPUT_NAME WebCoreGTK)
 
-# FIXME: https://bugs.webkit.org/show_bug.cgi?id=181916
-# Remove these lines when turning on hidden visibility
-list(APPEND WebCore_PRIVATE_LIBRARIES WebKit::WTF)
-if (NOT USE_SYSTEM_MALLOC)
-    list(APPEND WebCore_PRIVATE_LIBRARIES WebKit::bmalloc)
-endif ()
-
 list(APPEND WebCore_UNIFIED_SOURCE_LIST_FILES
     "SourcesGTK.txt"
 

diff --git a/Source/WebCore/dom/EventTarget.cpp b/Source/WebCore/dom/EventTarget.cpp
@@ -66,6 +66,8 @@ Ref<EventTarget> EventTarget::create(ScriptExecutionContext& context)
     return EventTargetConcrete::create(context);
 }
 
+EventTarget::~EventTarget() = default;
+
 bool EventTarget::isNode() const
 {
     return false;

diff --git a/Source/WebCore/dom/EventTarget.h b/Source/WebCore/dom/EventTarget.h
@@ -103,7 +103,7 @@ class EventTarget : public ScriptWrappable, public CanMakeWeakPtr<EventTarget> {
     const EventTargetData* eventTargetData() const;
 
 protected:
-    virtual ~EventTarget() = default;
+    WEBCORE_EXPORT virtual ~EventTarget();
 
     virtual EventTargetData* eventTargetData() = 0;
     virtual EventTargetData* eventTargetDataConcurrently() = 0;

diff --git a/Source/cmake/OptionsGTK.cmake b/Source/cmake/OptionsGTK.cmake
@@ -18,6 +18,11 @@ endif ()
 
 CALCULATE_LIBRARY_VERSIONS_FROM_LIBTOOL_TRIPLE(JAVASCRIPTCORE 37 0 19)
 
+set(CMAKE_C_VISIBILITY_PRESET hidden)
+set(CMAKE_CXX_VISIBILITY_PRESET hidden)
+set(bmalloc_LIBRARY_TYPE OBJECT)
+set(WTF_LIBRARY_TYPE OBJECT)
+
 # These are shared variables, but we special case their definition so that we can use the
 # CMAKE_INSTALL_* variables that are populated by the GNUInstallDirs macro.
 set(LIB_INSTALL_DIR "${CMAKE_INSTALL_FULL_LIBDIR}" CACHE PATH "Absolute path to library installation directory")

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
@@ -1,3 +1,13 @@
+2021-03-09  Michael Catanzaro  <mcatanzaro@gnome.org>
+
+        [GTK] Reenable -fvisibility=hidden
+        https://bugs.webkit.org/show_bug.cgi?id=181916
+
+        Reviewed by Don Olmstead.
+
+        * TestWebKitAPI/PlatformGTK.cmake:
+        * TestWebKitAPI/glib/TestExpectations.json:
+
 2021-03-09  Mark Lam  <mark.lam@apple.com>
 
         Use --verifyGC=true on some JSC stress test configurations.

diff --git a/Tools/TestWebKitAPI/PlatformGTK.cmake b/Tools/TestWebKitAPI/PlatformGTK.cmake
@@ -28,9 +28,6 @@ list(APPEND TestWTF_LIBRARIES
     GTK::GTK
 )
 
-# FIXME: Remove when turning on hidden visibility https://bugs.webkit.org/show_bug.cgi?id=181916
-list(APPEND TestJavaScriptCore_LIBRARIES WTF)
-
 # TestWebCore
 list(APPEND TestWebCore_SOURCES
     ${test_main_SOURCES}

diff --git a/Tools/TestWebKitAPI/glib/TestExpectations.json b/Tools/TestWebKitAPI/glib/TestExpectations.json
@@ -294,6 +294,9 @@
         "subtests": {
             "/jsc/vm": {
                 "expected": {"all": {"slow": true}}
+            },
+            "/jsc/weak-value": {
+                "expected": {"all": {"status": ["FAIL", "PASS"], "bug": "webkit.org/b/222972"}}
             }
         }
     },