Bug 162798 – Remove ClipRects's custom refcounting.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[alan]

and use RefCounted instead. More secure.

[alan]

Created attachment 290365 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 290365 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=290365&action=review

> Source/WebCore/rendering/RenderLayer.cpp:208
> +    bool m_fixed = false;

Why isn't this { false }

[alan]

Created attachment 290369 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 290369 [details]
Patch

Clearing flags on attachment: 290369

Committed r206661: <http://trac.webkit.org/changeset/206661>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[David Kilzer (:ddkilzer)]

(In reply to comment #4)
> Comment on attachment 290369 [details]
> Patch
> 
> Clearing flags on attachment: 290369
> 
> Committed r206661: <http://trac.webkit.org/changeset/206661>

This caused EFL build to fail:

<https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2?numbuilds=25>

FAILED: /usr/bin/c++   -DBUILDING_EFL__=1 -DBUILDING_WITH_CMAKE=1 -DBUILDING_WebCore -DDATA_DIR=\"/usr/local/share/ewebkit2-1\" -DDEFAULT_THEME_DIR=\"/home/joonghunpark1/WebKit-BuildSlave/efl-linux-64-release-wk2/build/WebKitBuild/Release/WebCore/platform/efl/DefaultTheme\" -DHAVE_CONFIG_H=1 -DTEST_HYPHENATAION_PATH=\"/home/joonghunpark1/WebKit-BuildSlave/efl-linux-64-release-wk2/build/WebKitBuild/DependenciesEFL/Root/webkitgtk-test-dicts\" -DUNINSTALLED_AUDIO_RESOURCES_DIR=\"/home/joonghunpark1/WebKit-BuildSlave/efl-linux-64-release-wk2/build/Source/WebCore/platform/audio/resources\" -DUSER_AGENT_EFL_MAJOR_VERSION=\"602\" -DUSER_AGENT_EFL_MINOR_VERSION=\"1\" -DUSE_GSTREAMER_MPEGTS -DUSE_WEBAUDIO_GSTREAMER=1 -DWEBKIT_EXEC_PATH=\"/home/joonghunpark1/WebKit-BuildSlave/efl-linux-64-release-wk2/build/WebKitBuild/Release/bin\" -DWEB_INSPECTOR_DIR=\"/home/joonghunpark1/WebKit-BuildSlave/efl-linux-64-release-wk2/build/WebKitBuild/Release/share/ewebkit2-1/inspector\" -std=c++1y -O3 -DNDEBUG -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-rtti -I../../Source/WebCore -I../../Source/WebCore/Modules/airplay -I../../Source/WebCore/Modules/applepay -I../../Source/WebCore/Modules/battery -I../../Source/WebCore/Modules/encryptedmedia -I../../Source/WebCore/Modules/fetch -I../../Source/WebCore/Modules/geolocation -I../../Source/WebCore/Modules/indexeddb -I../../Source/WebCore/Modules/indexeddb/client -I../../Source/WebCore/Modules/indexeddb/server -I../../Source/WebCore/Modules/indexeddb/shared -I../../Source/WebCore/Modules/indieui -I../../Source/WebCore/Modules/mediacontrols -I../../Source/WebCore/Modules/mediasession -I../../Source/WebCore/Modules/mediasource -I../../Source/WebCore/Modules/mediastream -I../../Source/WebCore/Modules/navigatorcontentutils -I../../Source/WebCore/Modules/notifications -I../../Source/WebCore/Modules/plugins -I../../Source/WebCore/Modules/proximity -I../../Source/WebCore/Modules/quota -I../../Source/WebCore/Modules/speech -I../../Source/WebCore/Modules/streams -I../../Source/WebCore/Modules/vibration -I../../Source/WebCore/Modules/webaudio -I../../Source/WebCore/Modules/webdatabase -I../../Source/WebCore/Modules/websockets -I../../Source/WebCore/accessibility -I../../Source/WebCore/animation -I../../Source/WebCore/bindings -I../../Source/WebCore/bindings/generic -I../../Source/WebCore/bindings/js -I../../Source/WebCore/bridge -I../../Source/WebCore/bridge/c -I../../Source/WebCore/bridge/jsc -I../../Source/WebCore/contentextensions -I../../Source/WebCore/crypto -I../../Source/WebCore/crypto/algorithms -I../../Source/WebCore/crypto/keys -I../../Source/WebCore/crypto/parameters -I../../Source/WebCore/css -I../../Source/WebCore/css/parser -I../../Source/WebCore/cssjit -I../../Source/WebCore/dom -I../../Source/WebCore/dom/default -I../../Source/WebCore/editing -I../../Source/WebCore/fileapi -I../../Source/WebCore/history -I../../Source/WebCore/html -I../../Source/WebCore/html/canvas -I../../Source/WebCore/html/forms -I../../Source/WebCore/html/parser -I../../Source/WebCore/html/shadow -I../../Source/WebCore/html/track -I../../Source/WebCore/inspector -I../../Source/WebCore/loader -I../../Source/WebCore/loader/appcache -I../../Source/WebCore/loader/archive -I../../Source/WebCore/loader/archive/mhtml -I../../Source/WebCore/loader/cache -I../../Source/WebCore/loader/icon -I../../Source/WebCore/mathml -I../../Source/WebCore/page -I../../Source/WebCore/page/animation -I../../Source/WebCore/page/csp -I../../Source/WebCore/page/scrolling -I../../Source/WebCore/platform -I../../Source/WebCore/platform/animation -I../../Source/WebCore/platform/audio -I../../Source/WebCore/platform/crypto -I../../Source/WebCore/platform/gamepad -I../../Source/WebCore/platform/gamepad/deprecated -I../../Source/WebCore/platform/gamepad/linux -I../../Source/WebCore/platform/graphics -I../../Source/WebCore/platform/graphics/cpu/arm -I../../Source/WebCore/platform/graphics/cpu/arm/filters -I../../Source/WebCore/platform/graphics/displaylists -I../../Source/WebCore/platform/graphics/filters -I../../Source/WebCore/platform/graphics/harfbuzz -I../../Source/WebCore/platform/graphics/harfbuzz/ng -I../../Source/WebCore/platform/graphics/opentype -I../../Source/WebCore/platform/graphics/transforms -I../../Source/WebCore/platform/mediastream -I../../Source/WebCore/platform/mock -I../../Source/WebCore/platform/mock/mediasource -I../../Source/WebCore/platform/network -I../../Source/WebCore/platform/sql -I../../Source/WebCore/platform/text -I../../Source/WebCore/platform/text/icu -I../../Source/WebCore/plugins -I../../Source/WebCore/rendering -I../../Source/WebCore/rendering/line -I../../Source/WebCore/rendering/mathml -I../../Source/WebCore/rendering/shapes -I../../Source/WebCore/rendering/style -I../../Source/WebCore/rendering/svg -I../../Source/WebCore/replay -I../../Source/WebCore/storage -I../../Source/WebCore/style -I../../Source/WebCore/svg -I../../Source/WebCore/svg/animation -I../../Source/WebCore/svg/graphics -I../../Source/WebCore/svg/graphics/filters -I../../Source/WebCore/svg/properties -I../../Source/WebCore/websockets -I../../Source/WebCore/workers -I../../Source/WebCore/xml -I../../Source/WebCore/xml/parser -IDerivedSources -IDerivedSources/ForwardingHeaders -IDerivedSources/WebCore -I../../Source -I. -I../../Source/WebCore/Modules/gamepad/deprecated -I../../Source/WebCore/platform/graphics/gpu -I../../Source/WebCore/platform/mediastream/openwebrtc -I../../Source/WebCore/platform/graphics/gstreamer -I../../Source/WebCore/platform/audio/gstreamer -I../../Source/WebCore/platform/image-decoders -I../../Source/WebCore/platform/image-decoders/bmp -I../../Source/WebCore/platform/image-decoders/gif -I../../Source/WebCore/platform/image-decoders/ico -I../../Source/WebCore/platform/image-decoders/jpeg -I../../Source/WebCore/platform/image-decoders/png -I../../Source/WebCore/platform/image-decoders/webp -I../../Source/WebCore/platform/linux -I../../Source/WebCore/platform/graphics/texmap -I../../Source/WebCore/page/scrolling/coordinatedgraphics -I../../Source/WebCore/platform/graphics/texmap/coordinated -IDerivedSources/JavaScriptCore -IDerivedSources/JavaScriptCore/inspector -I../../Source/JavaScriptCore -I../../Source/JavaScriptCore/ForwardingHeaders -I../../Source/JavaScriptCore/API -I../../Source/JavaScriptCore/assembler -I../../Source/JavaScriptCore/bytecode -I../../Source/JavaScriptCore/bytecompiler -I../../Source/JavaScriptCore/dfg -I../../Source/JavaScriptCore/disassembler -I../../Source/JavaScriptCore/heap -I../../Source/JavaScriptCore/debugger -I../../Source/JavaScriptCore/interpreter -I../../Source/JavaScriptCore/jit -I../../Source/JavaScriptCore/llint -I../../Source/JavaScriptCore/parser -I../../Source/JavaScriptCore/profiler -I../../Source/JavaScriptCore/runtime -I../../Source/JavaScriptCore/yarr -I../../Source/ThirdParty/ANGLE -I../../Source/ThirdParty/ANGLE/include/KHR -I../../Source/WebCore/editing/atk -I../../Source/WebCore/page/efl -I../../Source/WebCore/platform/cairo -I../../Source/WebCore/platform/efl -I../../Source/WebCore/platform/gamepad/efl -I../../Source/WebCore/platform/geoclue -I../../Source/WebCore/platform/graphics/cairo -I../../Source/WebCore/platform/graphics/efl -I../../Source/WebCore/platform/graphics/freetype -I../../Source/WebCore/platform/graphics/opengl -I../../Source/WebCore/platform/graphics/surfaces -I../../Source/WebCore/platform/graphics/surfaces/efl -I../../Source/WebCore/platform/graphics/surfaces/glx -I../../Source/WebCore/platform/graphics/x11 -I../../Source/WebCore/platform/network/soup -I../../Source/WebCore/platform/text/efl -I../../Source/WebCore/plugins/efl -I../../Source/WTF -I../../Source/WTF/wtf/efl -I../../Source/WebCore/accessibility/atk -isystem ../DependenciesEFL/Root/include/gstreamer-1.0 -isystem ../DependenciesEFL/Root/lib/gstreamer-1.0/include -isystem ../DependenciesEFL/Root/include/glib-2.0 -isystem ../DependenciesEFL/Root/lib/glib-2.0/include -isystem ../DependenciesEFL/Root/include/cairo -isystem ../DependenciesEFL/Root/include/efl-1 -isystem ../DependenciesEFL/Root/include/ecore-1 -isystem ../DependenciesEFL/Root/include/eo-1 -isystem ../DependenciesEFL/Root/include/eina-1 -isystem ../DependenciesEFL/Root/include/eina-1/eina -isystem ../DependenciesEFL/Root/include/ecore-evas-1 -isystem ../DependenciesEFL/Root/include/ecore-input-evas-1 -isystem ../DependenciesEFL/Root/include/evas-1 -isystem ../DependenciesEFL/Root/include/evas-1/canvas -isystem ../DependenciesEFL/Root/include/freetype2 -isystem ../DependenciesEFL/Root/include/libxml2 -isystem ../DependenciesEFL/Root/include/ecore-input-1 -isystem ../DependenciesEFL/Root/include/eeze-1 -isystem ../DependenciesEFL/Root/include/ecore-file-1 -isystem ../DependenciesEFL/Root/include/ecore-con-1 -isystem ../DependenciesEFL/Root/include/eet-1 -isystem ../DependenciesEFL/Root/include/emile-1 -isystem /usr/include/libpng12 -isystem /usr/include/luajit-2.0 -isystem ../DependenciesEFL/Root/include/ecore-x-1 -isystem ../DependenciesEFL/Root/include/edje-1 -isystem ../DependenciesEFL/Root/include/eio-1 -isystem ../DependenciesEFL/Root/include/efreet-1 -isystem ../DependenciesEFL/Root/include/ecore-ipc-1 -isystem ../DependenciesEFL/Root/include/embryo-1 -isystem ../DependenciesEFL/Root/include/ecore-imf-evas-1 -isystem ../DependenciesEFL/Root/include/ecore-imf-1 -isystem ../DependenciesEFL/Root/include/eldbus-1 -isystem /usr/include/dbus-1.0 -isystem /usr/lib/x86_64-linux-gnu/dbus-1.0/include -isystem ../DependenciesEFL/Root/include/freetype2/freetype -isystem ../DependenciesEFL/Root/include/gio-unix-2.0 -isystem ../DependenciesEFL/Root/include/libsoup-2.4 -isystem ../DependenciesEFL/Root/include/harfbuzz -isystem /usr/include/enchant -isystem ../DependenciesEFL/Root/include/atk-1.0 -isystem /usr/include/espeak -I../../Source/WebCore/testing    -Werror -Wall -Wextra -Wcast-align -Wformat-security -Wmissing-format-attribute -Wpointer-arith -Wundef -Wwrite-strings -fPIC -MMD -MT Source/WebCore/CMakeFiles/WebCore.dir/rendering/RenderLayer.cpp.o -MF Source/WebCore/CMakeFiles/WebCore.dir/rendering/RenderLayer.cpp.o.d -o Source/WebCore/CMakeFiles/WebCore.dir/rendering/RenderLayer.cpp.o -c ../../Source/WebCore/rendering/RenderLayer.cpp
../../Source/WebCore/rendering/RenderLayer.cpp: In copy constructor ‘WebCore::ClipRects::ClipRects(const WebCore::ClipRects&)’:
../../Source/WebCore/rendering/RenderLayer.cpp:200:5: error: base class ‘class WTF::RefCounted<WebCore::ClipRects>’ should be explicitly initialized in the copy constructor [-Werror=extra]
     ClipRects(const ClipRects& other)
     ^
cc1plus: all warnings being treated as errors
ninja: build stopped: subcommand failed.

[David Kilzer (:ddkilzer)]

This seems like the fix:

diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
index d98ebfe..5039236 100644
--- a/Source/WebCore/rendering/RenderLayer.cpp
+++ b/Source/WebCore/rendering/RenderLayer.cpp
@@ -198,7 +198,8 @@ private:
     }
 
     ClipRects(const ClipRects& other)
-        : m_fixed(other.fixed())
+        : RefCounted<ClipRects>()
+        , m_fixed(other.fixed())
         , m_overflowClipRect(other.overflowClipRect())
         , m_fixedClipRect(other.fixedClipRect())
         , m_posClipRect(other.posClipRect())

[David Kilzer (:ddkilzer)]

(In reply to comment #7)
> This seems like the fix:
> 
> diff --git a/Source/WebCore/rendering/RenderLayer.cpp
> b/Source/WebCore/rendering/RenderLayer.cpp
> index d98ebfe..5039236 100644
> --- a/Source/WebCore/rendering/RenderLayer.cpp
> +++ b/Source/WebCore/rendering/RenderLayer.cpp
> @@ -198,7 +198,8 @@ private:
>      }
>  
>      ClipRects(const ClipRects& other)
> -        : m_fixed(other.fixed())
> +        : RefCounted<ClipRects>()
> +        , m_fixed(other.fixed())
>          , m_overflowClipRect(other.overflowClipRect())
>          , m_fixedClipRect(other.fixedClipRect())
>          , m_posClipRect(other.posClipRect())

Fixed in r206704:  <https://trac.webkit.org/changeset/206704>