From 5f359baacdf92fabcece83f0a2b30f74c7c02a3c Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Mon, 22 Feb 2016 10:57:32 +0100 Subject: Fix linking with libpthread WebKit use libpthread directly but is depending on other qt modules causing it to be linked against, which might break unless -lpthread is last. Instead just add it explicitly after the static libraries. Change-Id: I2b95cff2c96373f8dce6f95052c4fccbe1982b33 Reviewed-by: Simon Hausmann (cherry picked from commit 5dd4bb67cfce812fd7686e43616e2069f354a7df) Reviewed-by: Allan Sandfeld Jensen --- Tools/qmake/mkspecs/features/default_post.prf | 1 + 1 file changed, 1 insertion(+) diff --git a/Tools/qmake/mkspecs/features/default_post.prf b/Tools/qmake/mkspecs/features/default_post.prf index 67276b74b..39bb3f7ae 100644 --- a/Tools/qmake/mkspecs/features/default_post.prf +++ b/Tools/qmake/mkspecs/features/default_post.prf @@ -201,6 +201,7 @@ needToLink() { linkAgainstLibrary($$library, $$eval(WEBKIT.$${library_identifier}.root_source_dir)) LIBS += $$eval(WEBKIT.$${library_identifier}.dependent_libs) } + posix:!darwin: LIBS += -lpthread } creating_module { -- cgit v1.2.1 From 4aed18cdbc79105b704a298e88af563f30e2eeb0 Mon Sep 17 00:00:00 2001 From: Konstantin Tokarev Date: Fri, 26 Feb 2016 19:25:07 +0300 Subject: isLinkVisited: Don't build complete URL unless we use QWebHistoryInterface. visitedURL() is non-trivial function with memory allocations, so we should not call it for nothing. Change-Id: I72e14fac75640cb6b22dd1444b9a061e8432dd7e Reviewed-by: Allan Sandfeld Jensen --- Source/WebKit/qt/WebCoreSupport/PlatformStrategiesQt.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/WebKit/qt/WebCoreSupport/PlatformStrategiesQt.cpp b/Source/WebKit/qt/WebCoreSupport/PlatformStrategiesQt.cpp index a8487f2c4..7fcf3de45 100644 --- a/Source/WebKit/qt/WebCoreSupport/PlatformStrategiesQt.cpp +++ b/Source/WebKit/qt/WebCoreSupport/PlatformStrategiesQt.cpp @@ -200,14 +200,14 @@ bool PlatformStrategiesQt::isLinkVisited(Page* page, LinkHash hash, const KURL& { ASSERT(hash); - Vector url; - visitedURL(baseURL, attributeURL, url); - // If the Qt4.4 interface for the history is used, we will have to fallback // to the old global history. QWebHistoryInterface* iface = QWebHistoryInterface::defaultInterface(); - if (iface) + if (iface) { + Vector url; + visitedURL(baseURL, attributeURL, url); return iface->historyContains(QString(reinterpret_cast(url.data()), url.size())); + } return page->group().isLinkVisited(hash); } -- cgit v1.2.1 From 81f43efbb2112b693b21d8f95cd627e9fd1032b9 Mon Sep 17 00:00:00 2001 From: Konstantin Tokarev Date: Fri, 26 Feb 2016 18:59:30 +0300 Subject: Updated changelog for 5.6.0. Change-Id: I21cd29080be8f1aeedad6f19f1f11d3df2d9ae1d Reviewed-by: Allan Sandfeld Jensen --- dist/changes-5.6.0 | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/dist/changes-5.6.0 b/dist/changes-5.6.0 index 0964b19b2..b042f2925 100644 --- a/dist/changes-5.6.0 +++ b/dist/changes-5.6.0 @@ -19,12 +19,21 @@ information about a particular change. **************************************************************************** -* Library * +* Important Behavior Changes * +**************************************************************************** + + - The quality parameter in canvas.toDataURL only applies to JPEG images + now, in accordance with section 4.12.4.4 of the HTML5 spec. + +**************************************************************************** +* Library * **************************************************************************** QtWebkit -------- - [QTBUG-44563] Render anchors as clickable links in PDF documents + - % unit heights didn't work if parent block height was set in vh + - Fixed regression: some errors were not reported to ErrorPageExtension QtWebkitQML ----------- -- cgit v1.2.1 From 3ecebca8361030d0adeb65af21e77334ab91e481 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 3 Mar 2016 20:07:42 +0100 Subject: Make public headers compile with -Wzero-as-null-pointer-constant ... or equivalent. QtBase 5.6 headers already compile that way, so let the other modules follow suit. Change-Id: I9e521f91abe7972843eeeb6406f8605778dabbaf Task-number: QTBUG-45291 Reviewed-by: Lars Knoll Reviewed-by: Allan Sandfeld Jensen --- Source/WebKit/qt/Api/qwebhistoryinterface.h | 2 +- Source/WebKit/qt/Api/qwebpluginfactory.h | 4 ++-- Source/WebKit/qt/WidgetApi/qgraphicswebview.h | 6 +++--- Source/WebKit/qt/WidgetApi/qwebinspector.h | 2 +- Source/WebKit/qt/WidgetApi/qwebpage.h | 6 +++--- Source/WebKit/qt/WidgetApi/qwebview.h | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Source/WebKit/qt/Api/qwebhistoryinterface.h b/Source/WebKit/qt/Api/qwebhistoryinterface.h index a49c58684..81186aa38 100644 --- a/Source/WebKit/qt/Api/qwebhistoryinterface.h +++ b/Source/WebKit/qt/Api/qwebhistoryinterface.h @@ -29,7 +29,7 @@ class QWEBKIT_EXPORT QWebHistoryInterface : public QObject { Q_OBJECT public: - QWebHistoryInterface(QObject *parent = 0); + QWebHistoryInterface(QObject *parent = Q_NULLPTR); ~QWebHistoryInterface(); static void setDefaultInterface(QWebHistoryInterface *defaultInterface); diff --git a/Source/WebKit/qt/Api/qwebpluginfactory.h b/Source/WebKit/qt/Api/qwebpluginfactory.h index 24084fe18..26f855066 100644 --- a/Source/WebKit/qt/Api/qwebpluginfactory.h +++ b/Source/WebKit/qt/Api/qwebpluginfactory.h @@ -48,7 +48,7 @@ public: QList mimeTypes; }; - explicit QWebPluginFactory(QObject* parent = 0); + explicit QWebPluginFactory(QObject* parent = Q_NULLPTR); virtual ~QWebPluginFactory(); virtual QList plugins() const = 0; @@ -62,7 +62,7 @@ public: {}; class ExtensionReturn {}; - virtual bool extension(Extension extension, const ExtensionOption* option = 0, ExtensionReturn* output = 0); + virtual bool extension(Extension extension, const ExtensionOption* option = Q_NULLPTR, ExtensionReturn* output = Q_NULLPTR); virtual bool supportsExtension(Extension extension) const; private: diff --git a/Source/WebKit/qt/WidgetApi/qgraphicswebview.h b/Source/WebKit/qt/WidgetApi/qgraphicswebview.h index c9e61f75a..0d990086f 100644 --- a/Source/WebKit/qt/WidgetApi/qgraphicswebview.h +++ b/Source/WebKit/qt/WidgetApi/qgraphicswebview.h @@ -54,7 +54,7 @@ class QWEBKITWIDGETS_EXPORT QGraphicsWebView : public QGraphicsWidget { Q_FLAGS(QPainter::RenderHints) public: - explicit QGraphicsWebView(QGraphicsItem* parent = 0); + explicit QGraphicsWebView(QGraphicsItem* parent = Q_NULLPTR); ~QGraphicsWebView(); QWebPage* page() const; @@ -84,7 +84,7 @@ public: QAction* pageAction(QWebPage::WebAction action) const; void triggerPageAction(QWebPage::WebAction action, bool checked = false); - bool findText(const QString& subString, QWebPage::FindFlags options = 0); + bool findText(const QString& subString, QWebPage::FindFlags options = QWebPage::FindFlags()); bool resizesToContents() const; void setResizesToContents(bool enabled); @@ -94,7 +94,7 @@ public: virtual void setGeometry(const QRectF& rect); virtual void updateGeometry(); - virtual void paint(QPainter*, const QStyleOptionGraphicsItem* options, QWidget* widget = 0); + virtual void paint(QPainter*, const QStyleOptionGraphicsItem* options, QWidget* widget = Q_NULLPTR); virtual QVariant itemChange(GraphicsItemChange change, const QVariant& value); virtual bool event(QEvent*); diff --git a/Source/WebKit/qt/WidgetApi/qwebinspector.h b/Source/WebKit/qt/WidgetApi/qwebinspector.h index c333fa25a..3fcb853f7 100644 --- a/Source/WebKit/qt/WidgetApi/qwebinspector.h +++ b/Source/WebKit/qt/WidgetApi/qwebinspector.h @@ -30,7 +30,7 @@ class QWebInspectorPrivate; class QWEBKITWIDGETS_EXPORT QWebInspector : public QWidget { Q_OBJECT public: - QWebInspector(QWidget* parent = 0); + QWebInspector(QWidget* parent = Q_NULLPTR); ~QWebInspector(); void setPage(QWebPage* page); diff --git a/Source/WebKit/qt/WidgetApi/qwebpage.h b/Source/WebKit/qt/WidgetApi/qwebpage.h index 24fe1383e..a9c71fd42 100644 --- a/Source/WebKit/qt/WidgetApi/qwebpage.h +++ b/Source/WebKit/qt/WidgetApi/qwebpage.h @@ -265,7 +265,7 @@ public: }; - explicit QWebPage(QObject *parent = 0); + explicit QWebPage(QObject *parent = Q_NULLPTR); ~QWebPage(); QWebFrame *mainFrame() const; @@ -317,7 +317,7 @@ public: QVariant inputMethodQuery(Qt::InputMethodQuery property) const; - bool findText(const QString &subString, FindFlags options = 0); + bool findText(const QString &subString, FindFlags options = FindFlags()); void setForwardUnsupportedContent(bool forward); bool forwardUnsupportedContent() const; @@ -383,7 +383,7 @@ public: }; - virtual bool extension(Extension extension, const ExtensionOption *option = 0, ExtensionReturn *output = 0); + virtual bool extension(Extension extension, const ExtensionOption *option = Q_NULLPTR, ExtensionReturn *output = Q_NULLPTR); virtual bool supportsExtension(Extension extension) const; QWebPageAdapter* handle() const; diff --git a/Source/WebKit/qt/WidgetApi/qwebview.h b/Source/WebKit/qt/WidgetApi/qwebview.h index 43439dfa5..ac0946773 100644 --- a/Source/WebKit/qt/WidgetApi/qwebview.h +++ b/Source/WebKit/qt/WidgetApi/qwebview.h @@ -54,7 +54,7 @@ class QWEBKITWIDGETS_EXPORT QWebView : public QWidget { Q_PROPERTY(QPainter::RenderHints renderHints READ renderHints WRITE setRenderHints) Q_FLAGS(QPainter::RenderHints) public: - explicit QWebView(QWidget* parent = 0); + explicit QWebView(QWidget* parent = Q_NULLPTR); virtual ~QWebView(); QWebPage* page() const; @@ -104,7 +104,7 @@ public: void setRenderHints(QPainter::RenderHints hints); void setRenderHint(QPainter::RenderHint hint, bool enabled = true); - bool findText(const QString& subString, QWebPage::FindFlags options = 0); + bool findText(const QString& subString, QWebPage::FindFlags options = QWebPage::FindFlags()); virtual bool event(QEvent*); -- cgit v1.2.1 From d832cfa7c3ba44b56a566a998c69f7436dee2581 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 3 Mar 2016 20:08:26 +0100 Subject: Make more ctors explicit Added explicit where it was missing. This is not a source- incompatible change, because code that breaks by this is a bug. Let's not have this sitting around in an LTS. Change-Id: If2e979aa5772b8dc0b9a4a9360914740144d47a7 Reviewed-by: Lars Knoll Reviewed-by: Allan Sandfeld Jensen --- Source/WebKit/qt/Api/qwebhistoryinterface.h | 2 +- Source/WebKit/qt/WidgetApi/qwebinspector.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/WebKit/qt/Api/qwebhistoryinterface.h b/Source/WebKit/qt/Api/qwebhistoryinterface.h index 81186aa38..3038d22bd 100644 --- a/Source/WebKit/qt/Api/qwebhistoryinterface.h +++ b/Source/WebKit/qt/Api/qwebhistoryinterface.h @@ -29,7 +29,7 @@ class QWEBKIT_EXPORT QWebHistoryInterface : public QObject { Q_OBJECT public: - QWebHistoryInterface(QObject *parent = Q_NULLPTR); + explicit QWebHistoryInterface(QObject *parent = Q_NULLPTR); ~QWebHistoryInterface(); static void setDefaultInterface(QWebHistoryInterface *defaultInterface); diff --git a/Source/WebKit/qt/WidgetApi/qwebinspector.h b/Source/WebKit/qt/WidgetApi/qwebinspector.h index 3fcb853f7..d6a4aad3c 100644 --- a/Source/WebKit/qt/WidgetApi/qwebinspector.h +++ b/Source/WebKit/qt/WidgetApi/qwebinspector.h @@ -30,7 +30,7 @@ class QWebInspectorPrivate; class QWEBKITWIDGETS_EXPORT QWebInspector : public QWidget { Q_OBJECT public: - QWebInspector(QWidget* parent = Q_NULLPTR); + explicit QWebInspector(QWidget* parent = Q_NULLPTR); ~QWebInspector(); void setPage(QWebPage* page); -- cgit v1.2.1 From 49ad697403f7031f292924096851b498f788bbfd Mon Sep 17 00:00:00 2001 From: Javier Fernandez Date: Mon, 7 Mar 2016 15:05:43 +0100 Subject: Breaking several cyclic references between SVG animated properties. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Based on upstream fix by sabouhallawa@apple.com at http://trac.webkit.org/changeset/196268 The leak happens because of cyclic reference between SVGListPropertyTearOff and SVGAnimatedListPropertyTearOff which is derived from SVGAnimatedProperty. There is also cyclic reference between SVGAnimatedProperty and SVGElement and this causes the whole document to be leaked. So if the JS requests, for example, an instance of SVGPolylineElement.points, the whole document will be leaked. The fix depends on having the cyclic reference as is since the owning and the owned classes have to live together if any of them is referenced. But the owning class caches a raw 'ref-counted' pointer of the owned class. If it is requested for an instance of the owned class it returned a RefPtr<> of it. Once the owned class is not used, it can delete itself. The only thing needed here is to notify the owner class of the deletion so it cleans its caches and be able to create a new pointer if it is requested for an instance of the owned class later. Revert the change of r181345 in SVGAnimatedProperty::lookupOrCreateWrapper() to break the cyclic reference between SVGElement and SVGAnimatedProperty. Also apply the same approach in SVGAnimatedListPropertyTearOff::baseVal() and animVal() to break cyclic reference between SVGListPropertyTearOff and SVGAnimatedListPropertyTearOff. Change-Id: Ied6a077299e47855feb235a1c9310f1a58aad91b Reviewed-by: Konstantin Tokarev Reviewed-by: Michael Brüning --- Source/WebCore/bindings/scripts/CodeGeneratorJS.pm | 6 +- Source/WebCore/svg/SVGPathElement.cpp | 23 +++--- Source/WebCore/svg/SVGPathElement.h | 8 +- Source/WebCore/svg/SVGPathSegWithContext.h | 2 +- Source/WebCore/svg/SVGPolyElement.cpp | 12 +-- Source/WebCore/svg/SVGPolyElement.h | 4 +- Source/WebCore/svg/SVGViewSpec.cpp | 8 +- Source/WebCore/svg/SVGViewSpec.h | 2 +- .../properties/SVGAnimatedListPropertyTearOff.h | 86 ++++++++++++++-------- .../SVGAnimatedPathSegListPropertyTearOff.h | 30 ++++---- .../WebCore/svg/properties/SVGAnimatedProperty.h | 22 +++--- .../svg/properties/SVGAnimatedPropertyMacros.h | 4 +- .../SVGAnimatedTransformListPropertyTearOff.h | 22 ++++-- .../svg/properties/SVGListPropertyTearOff.h | 6 ++ .../properties/SVGPathSegListPropertyTearOff.cpp | 4 +- 15 files changed, 145 insertions(+), 94 deletions(-) diff --git a/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm b/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm index df4234e23..2cca58703 100644 --- a/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm +++ b/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm @@ -3624,7 +3624,11 @@ sub NativeToJSValue return "toJSNewlyCreated(exec, $globalObject, WTF::getPtr($value))"; } - if ($codeGenerator->IsSVGAnimatedType($interfaceName) or $interfaceName eq "SVGViewSpec") { + # $type has to be used here because SVGViewSpec.transform is of type SVGTransformList. + if ($codeGenerator->IsSVGTypeNeedingTearOff($type) and $type =~ /(? to real type, so the right toJS() method can be invoked. + $value = "static_cast<" . GetNativeType($type) . ">($value" . ".get())"; + } elsif ($codeGenerator->IsSVGAnimatedType($interfaceName) or $interfaceName eq "SVGViewSpec") { # Convert from abstract SVGProperty to real type, so the right toJS() method can be invoked. $value = "static_cast<" . GetNativeType($type) . ">($value)"; } elsif ($codeGenerator->IsSVGTypeNeedingTearOff($type) and not $interfaceName =~ /List$/) { diff --git a/Source/WebCore/svg/SVGPathElement.cpp b/Source/WebCore/svg/SVGPathElement.cpp index 3324d0b5d..b030c9a83 100644 --- a/Source/WebCore/svg/SVGPathElement.cpp +++ b/Source/WebCore/svg/SVGPathElement.cpp @@ -310,10 +310,15 @@ void SVGPathElement::removedFrom(ContainerNode* rootParent) SVGPathByteStream* SVGPathElement::pathByteStream() const { - SVGAnimatedProperty* property = SVGAnimatedProperty::lookupWrapper(this, dPropertyInfo()); + RefPtr property = SVGAnimatedProperty::lookupWrapper(this, dPropertyInfo()); if (!property || !property->isAnimating()) return m_pathByteStream.get(); - return static_cast(property)->animatedPathByteStream(); + + SVGPathByteStream* animatedPathByteStream = static_cast(property.get())->animatedPathByteStream(); + if (!animatedPathByteStream) + return m_pathByteStream.get(); + + return animatedPathByteStream; } PassRefPtr SVGPathElement::lookupOrCreateDWrapper(SVGElement* contextElement) @@ -321,7 +326,7 @@ PassRefPtr SVGPathElement::lookupOrCreateDWrapper(SVGElemen ASSERT(contextElement); SVGPathElement* ownerType = toSVGPathElement(contextElement); - if (SVGAnimatedProperty* property = SVGAnimatedProperty::lookupWrapper(ownerType, dPropertyInfo())) + if (RefPtr property = SVGAnimatedProperty::lookupWrapper(ownerType, dPropertyInfo())) return property; // Build initial SVGPathSegList. @@ -340,26 +345,26 @@ void SVGPathElement::synchronizeD(SVGElement* contextElement) ownerType->m_pathSegList.synchronize(ownerType, dPropertyInfo()->attributeName, ownerType->m_pathSegList.value.valueAsString()); } -SVGPathSegListPropertyTearOff* SVGPathElement::pathSegList() +RefPtr SVGPathElement::pathSegList() { m_pathSegList.shouldSynchronize = true; - return static_cast(static_pointer_cast(lookupOrCreateDWrapper(this))->baseVal()); + return static_cast(static_pointer_cast(lookupOrCreateDWrapper(this))->baseVal().get()); } -SVGPathSegListPropertyTearOff* SVGPathElement::normalizedPathSegList() +RefPtr SVGPathElement::normalizedPathSegList() { // FIXME: https://bugs.webkit.org/show_bug.cgi?id=15412 - Implement normalized path segment lists! return 0; } -SVGPathSegListPropertyTearOff* SVGPathElement::animatedPathSegList() +RefPtr SVGPathElement::animatedPathSegList() { m_pathSegList.shouldSynchronize = true; m_isAnimValObserved = true; - return static_cast(static_pointer_cast(lookupOrCreateDWrapper(this))->animVal()); + return static_cast(static_pointer_cast(lookupOrCreateDWrapper(this))->animVal().get()); } -SVGPathSegListPropertyTearOff* SVGPathElement::animatedNormalizedPathSegList() +RefPtr SVGPathElement::animatedNormalizedPathSegList() { // FIXME: https://bugs.webkit.org/show_bug.cgi?id=15412 - Implement normalized path segment lists! return 0; diff --git a/Source/WebCore/svg/SVGPathElement.h b/Source/WebCore/svg/SVGPathElement.h index c81ead198..0ca66ac11 100644 --- a/Source/WebCore/svg/SVGPathElement.h +++ b/Source/WebCore/svg/SVGPathElement.h @@ -83,10 +83,10 @@ public: PassRefPtr createSVGPathSegCurvetoQuadraticSmoothRel(float x, float y, SVGPathSegRole role = PathSegUndefinedRole); // Used in the bindings only. - SVGPathSegListPropertyTearOff* pathSegList(); - SVGPathSegListPropertyTearOff* animatedPathSegList(); - SVGPathSegListPropertyTearOff* normalizedPathSegList(); - SVGPathSegListPropertyTearOff* animatedNormalizedPathSegList(); + RefPtr pathSegList(); + RefPtr animatedPathSegList(); + RefPtr normalizedPathSegList(); + RefPtr animatedNormalizedPathSegList(); SVGPathByteStream* pathByteStream() const; diff --git a/Source/WebCore/svg/SVGPathSegWithContext.h b/Source/WebCore/svg/SVGPathSegWithContext.h index 701067227..4ba1447bf 100644 --- a/Source/WebCore/svg/SVGPathSegWithContext.h +++ b/Source/WebCore/svg/SVGPathSegWithContext.h @@ -33,7 +33,7 @@ public: { } - SVGAnimatedProperty* animatedProperty() const + RefPtr animatedProperty() const { switch (m_role) { case PathSegUndefinedRole: diff --git a/Source/WebCore/svg/SVGPolyElement.cpp b/Source/WebCore/svg/SVGPolyElement.cpp index fd5c652f1..977234d5e 100644 --- a/Source/WebCore/svg/SVGPolyElement.cpp +++ b/Source/WebCore/svg/SVGPolyElement.cpp @@ -88,8 +88,8 @@ void SVGPolyElement::parseAttribute(const QualifiedName& name, const AtomicStrin if (!pointsListFromSVGData(newList, value)) document()->accessSVGExtensions()->reportError("Problem parsing points=\"" + value + "\""); - if (SVGAnimatedProperty* wrapper = SVGAnimatedProperty::lookupWrapper(this, pointsPropertyInfo())) - static_cast(wrapper)->detachListWrappers(newList.size()); + if (RefPtr wrapper = SVGAnimatedProperty::lookupWrapper(this, pointsPropertyInfo())) + static_pointer_cast(wrapper)->detachListWrappers(newList.size()); m_points.value = newList; return; @@ -147,16 +147,16 @@ PassRefPtr SVGPolyElement::lookupOrCreatePointsWrapper(SVGE (ownerType, pointsPropertyInfo(), ownerType->m_points.value); } -SVGListPropertyTearOff* SVGPolyElement::points() +RefPtr > SVGPolyElement::points() { m_points.shouldSynchronize = true; - return static_cast*>(static_pointer_cast(lookupOrCreatePointsWrapper(this))->baseVal()); + return static_cast*>(static_pointer_cast(lookupOrCreatePointsWrapper(this))->baseVal().get()); } -SVGListPropertyTearOff* SVGPolyElement::animatedPoints() +RefPtr > SVGPolyElement::animatedPoints() { m_points.shouldSynchronize = true; - return static_cast*>(static_pointer_cast(lookupOrCreatePointsWrapper(this))->animVal()); + return static_cast*>(static_pointer_cast(lookupOrCreatePointsWrapper(this))->animVal().get()); } } diff --git a/Source/WebCore/svg/SVGPolyElement.h b/Source/WebCore/svg/SVGPolyElement.h index bd0dd49aa..f399a427e 100644 --- a/Source/WebCore/svg/SVGPolyElement.h +++ b/Source/WebCore/svg/SVGPolyElement.h @@ -33,8 +33,8 @@ namespace WebCore { class SVGPolyElement : public SVGGraphicsElement , public SVGExternalResourcesRequired { public: - SVGListPropertyTearOff* points(); - SVGListPropertyTearOff* animatedPoints(); + RefPtr > points(); + RefPtr > animatedPoints(); SVGPointList& pointList() const { return m_points.value; } diff --git a/Source/WebCore/svg/SVGViewSpec.cpp b/Source/WebCore/svg/SVGViewSpec.cpp index 253c082fb..5ac02dbdd 100644 --- a/Source/WebCore/svg/SVGViewSpec.cpp +++ b/Source/WebCore/svg/SVGViewSpec.cpp @@ -117,8 +117,8 @@ void SVGViewSpec::setTransformString(const String& transform) SVGTransformList newList; newList.parse(transform); - if (SVGAnimatedProperty* wrapper = SVGAnimatedProperty::lookupWrapper(m_contextElement, transformPropertyInfo())) - static_cast(wrapper)->detachListWrappers(newList.size()); + if (RefPtr wrapper = SVGAnimatedProperty::lookupWrapper(m_contextElement, transformPropertyInfo())) + static_pointer_cast(wrapper)->detachListWrappers(newList.size()); m_transform = newList; } @@ -148,12 +148,12 @@ SVGElement* SVGViewSpec::viewTarget() const return toSVGElement(element); } -SVGTransformListPropertyTearOff* SVGViewSpec::transform() +RefPtr SVGViewSpec::transform() { if (!m_contextElement) return 0; // Return the animVal here, as its readonly by default - which is exactly what we want here. - return static_cast(static_pointer_cast(lookupOrCreateTransformWrapper(this))->animVal()); + return static_cast(static_pointer_cast(lookupOrCreateTransformWrapper(this))->animVal().get()); } PassRefPtr SVGViewSpec::viewBoxAnimated() diff --git a/Source/WebCore/svg/SVGViewSpec.h b/Source/WebCore/svg/SVGViewSpec.h index f8634afd6..3887db7f5 100644 --- a/Source/WebCore/svg/SVGViewSpec.h +++ b/Source/WebCore/svg/SVGViewSpec.h @@ -69,7 +69,7 @@ public: void resetContextElement() { m_contextElement = 0; } // Custom non-animated 'transform' property. - SVGTransformListPropertyTearOff* transform(); + RefPtr transform(); SVGTransformList transformBaseValue() const { return m_transform; } // Custom animated 'viewBox' property. diff --git a/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h index 65ee9e58d..136ba8253 100644 --- a/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h +++ b/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h @@ -40,35 +40,49 @@ public: typedef SVGListPropertyTearOff ListPropertyTearOff; typedef PropertyType ContentType; - virtual ListProperty* baseVal() + virtual PassRefPtr baseVal() { - if (!m_baseVal) - m_baseVal = ListPropertyTearOff::create(this, BaseValRole, m_values, m_wrappers); - return static_cast(m_baseVal.get()); + if (m_baseVal) + return m_baseVal; + + RefPtr property = ListPropertyTearOff::create(this, BaseValRole, m_values, m_wrappers).get(); + m_baseVal = property.get(); + return property.release(); + } + + virtual PassRefPtr animVal() + { + if (m_animVal) + return m_animVal; + + RefPtr property = ListPropertyTearOff::create(this, AnimValRole, m_values, m_wrappers).get(); + m_animVal = property.get(); + return property.release(); } - virtual ListProperty* animVal() + void propertyWillBeDeleted(const ListProperty& property) { - if (!m_animVal) - m_animVal = ListPropertyTearOff::create(this, AnimValRole, m_values, m_wrappers); - return static_cast(m_animVal.get()); + if (&property == m_baseVal) + m_baseVal = 0; + else if (&property == m_animVal) + m_animVal = 0; } virtual bool isAnimatedListTearOff() const { return true; } - int findItem(SVGProperty* property) const + int findItem(SVGProperty* property) { // This should ever be called for our baseVal, as animVal can't modify the list. // It's safe to cast to ListPropertyTearOff here as all classes inheriting from us supply their own removeItemFromList() method. typedef SVGPropertyTearOff::ListItemType> ListItemTearOff; - return static_cast(m_baseVal.get())->findItem(static_cast(property)); + return static_pointer_cast(baseVal())->findItem(static_cast(property)); } void removeItemFromList(size_t itemIndex, bool shouldSynchronizeWrappers) { // This should ever be called for our baseVal, as animVal can't modify the list. // It's safe to cast to ListPropertyTearOff here as all classes inheriting from us supply their own removeItemFromList() method. - static_cast(m_baseVal.get())->removeItemFromList(itemIndex, shouldSynchronizeWrappers); + static_pointer_cast(baseVal())->removeItemFromList(itemIndex, shouldSynchronizeWrappers); } void detachListWrappers(unsigned newListSize) @@ -79,8 +93,8 @@ public: PropertyType& currentAnimatedValue() { ASSERT(m_isAnimating); - ASSERT(m_animVal); - return static_cast(m_animVal.get())->values(); + ASSERT(m_animatingAnimVal); + return static_pointer_cast(m_animatingAnimVal)->values(); } const PropertyType& currentBaseValue() const @@ -91,6 +105,7 @@ public: void animationStarted(PropertyType* newAnimVal, bool shouldOwnValues = false) { ASSERT(!m_isAnimating); + ASSERT(!m_animatingAnimVal); ASSERT(newAnimVal); ASSERT(m_values.size() == m_wrappers.size()); ASSERT(m_animatedWrappers.isEmpty()); @@ -99,49 +114,49 @@ public: if (!newAnimVal->isEmpty()) m_animatedWrappers.fill(0, newAnimVal->size()); - ListProperty* animVal = static_cast(this->animVal()); - animVal->setValuesAndWrappers(newAnimVal, &m_animatedWrappers, shouldOwnValues); - ASSERT(animVal->values().size() == animVal->wrappers().size()); - ASSERT(animVal->wrappers().size() == m_animatedWrappers.size()); + m_animatingAnimVal = animVal(); + m_animatingAnimVal->setValuesAndWrappers(newAnimVal, &m_animatedWrappers, shouldOwnValues); + ASSERT(m_animatingAnimVal->values().size() == m_animatingAnimVal->wrappers().size()); + ASSERT(m_animatingAnimVal->wrappers().size() == m_animatedWrappers.size()); m_isAnimating = true; } void animationEnded() { ASSERT(m_isAnimating); - ASSERT(m_animVal); + ASSERT(m_animatingAnimVal); ASSERT(m_values.size() == m_wrappers.size()); - ListProperty* animVal = static_cast(m_animVal.get()); - ASSERT(animVal->values().size() == animVal->wrappers().size()); - ASSERT(animVal->wrappers().size() == m_animatedWrappers.size()); + ASSERT(m_animatingAnimVal->values().size() == m_animatingAnimVal->wrappers().size()); + ASSERT(m_animatingAnimVal->wrappers().size() == m_animatedWrappers.size()); - animVal->setValuesAndWrappers(&m_values, &m_wrappers, false); - ASSERT(animVal->values().size() == animVal->wrappers().size()); - ASSERT(animVal->wrappers().size() == m_wrappers.size()); + m_animatingAnimVal->setValuesAndWrappers(&m_values, &m_wrappers, false); + ASSERT(m_animatingAnimVal->values().size() == m_animatingAnimVal->wrappers().size()); + ASSERT(m_animatingAnimVal->wrappers().size() == m_wrappers.size()); m_animatedWrappers.clear(); + m_animatingAnimVal = 0; m_isAnimating = false; } void synchronizeWrappersIfNeeded() { + ASSERT(m_isAnimating); + ASSERT(m_animatingAnimVal); + // Eventually the wrapper list needs synchronization because any SVGAnimateLengthList::calculateAnimatedValue() call may // mutate the length of our values() list, and thus the wrapper() cache needs synchronization, to have the same size. // Also existing wrappers which point directly at elements in the existing SVGLengthList have to be detached (so a copy // of them is created, so existing animVal variables in JS are kept-alive). If we'd detach them later the underlying // SVGLengthList was already mutated, and our list item wrapper tear offs would point nowhere. Assertions would fire. - ListProperty* animVal = static_cast(m_animVal.get()); - animVal->detachListWrappers(animVal->values().size()); + m_animatingAnimVal->detachListWrappers(m_animatingAnimVal->values().size()); - ASSERT(animVal->values().size() == animVal->wrappers().size()); - ASSERT(animVal->wrappers().size() == m_animatedWrappers.size()); + ASSERT(m_animatingAnimVal->values().size() == m_animatingAnimVal->wrappers().size()); + ASSERT(m_animatingAnimVal->wrappers().size() == m_animatedWrappers.size()); } void animValWillChange() { - ASSERT(m_isAnimating); - ASSERT(m_animVal); ASSERT(m_values.size() == m_wrappers.size()); synchronizeWrappersIfNeeded(); } @@ -164,6 +179,8 @@ protected: SVGAnimatedListPropertyTearOff(SVGElement* contextElement, const QualifiedName& attributeName, AnimatedPropertyType animatedPropertyType, PropertyType& values) : SVGAnimatedProperty(contextElement, attributeName, animatedPropertyType) , m_values(values) + , m_baseVal(0) + , m_animVal(0) { if (!values.isEmpty()) m_wrappers.fill(0, values.size()); @@ -174,8 +191,13 @@ protected: ListWrapperCache m_wrappers; ListWrapperCache m_animatedWrappers; - RefPtr m_baseVal; - RefPtr m_animVal; + // Cache the raw pointer but return a RefPtr<>. This will break the cyclic reference + // between SVGListPropertyTearOff and SVGAnimatedListPropertyTearOff once the property + // pointer is not needed. + ListProperty* m_baseVal; + ListProperty* m_animVal; + + RefPtr m_animatingAnimVal; }; } diff --git a/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h index eb6dd19e3..1cfe0230e 100644 --- a/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h +++ b/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h @@ -32,32 +32,36 @@ namespace WebCore { class SVGAnimatedPathSegListPropertyTearOff : public SVGAnimatedListPropertyTearOff { public: - virtual SVGListProperty* baseVal() + virtual PassRefPtr baseVal() OVERRIDE { - if (!m_baseVal) - m_baseVal = SVGPathSegListPropertyTearOff::create(this, BaseValRole, PathSegUnalteredRole, m_values, m_wrappers); - return static_cast*>(m_baseVal.get()); + if (m_baseVal) + return m_baseVal; + + RefPtr property = SVGPathSegListPropertyTearOff::create(this, BaseValRole, PathSegUnalteredRole, m_values, m_wrappers); + m_baseVal = property.get(); + return property.release(); } - virtual SVGListProperty* animVal() + virtual PassRefPtr animVal() OVERRIDE { - if (!m_animVal) - m_animVal = SVGPathSegListPropertyTearOff::create(this, AnimValRole, PathSegUnalteredRole, m_values, m_wrappers); - return static_cast*>(m_animVal.get()); + if (m_animVal) + return m_animVal; + + RefPtr property = SVGPathSegListPropertyTearOff::create(this, AnimValRole, PathSegUnalteredRole, m_values, m_wrappers); + m_animVal = property.get(); + return property.release(); } - int findItem(const RefPtr& segment) const + int findItem(const RefPtr& segment) { // This should ever be called for our baseVal, as animVal can't modify the list. - ASSERT(m_baseVal); - return static_cast(m_baseVal.get())->findItem(segment); + return static_cast(baseVal().get())->findItem(segment); } void removeItemFromList(size_t itemIndex, bool shouldSynchronizeWrappers) { // This should ever be called for our baseVal, as animVal can't modify the list. - ASSERT(m_baseVal); - static_cast(m_baseVal.get())->removeItemFromList(itemIndex, shouldSynchronizeWrappers); + static_cast(baseVal().get())->removeItemFromList(itemIndex, shouldSynchronizeWrappers); } static PassRefPtr create(SVGElement* contextElement, const QualifiedName& attributeName, AnimatedPropertyType animatedPropertyType, SVGPathSegList& values) diff --git a/Source/WebCore/svg/properties/SVGAnimatedProperty.h b/Source/WebCore/svg/properties/SVGAnimatedProperty.h index 93f076b2b..d40a50434 100644 --- a/Source/WebCore/svg/properties/SVGAnimatedProperty.h +++ b/Source/WebCore/svg/properties/SVGAnimatedProperty.h @@ -53,18 +53,22 @@ public: { ASSERT(info); SVGAnimatedPropertyDescription key(element, info->propertyIdentifier); - RefPtr wrapper = animatedPropertyCache()->get(key); - if (!wrapper) { - wrapper = TearOffType::create(element, info->attributeName, info->animatedPropertyType, property); - if (info->animatedPropertyState == PropertyIsReadOnly) - wrapper->setIsReadOnly(); - animatedPropertyCache()->set(key, wrapper.get()); - } + Cache::AddResult result = animatedPropertyCache()->add(key, 0); + if (!result.isNewEntry) + return static_cast(result.iterator->value); + + RefPtr wrapper = TearOffType::create(element, info->attributeName, info->animatedPropertyType, property); + if (info->animatedPropertyState == PropertyIsReadOnly) + wrapper->setIsReadOnly(); + + // Cache the raw pointer but return a RefPtr<>. This will break the cyclic reference + // between SVGAnimatedProperty and SVGElement once the property pointer is not needed. + result.iterator->value = wrapper.get(); return static_pointer_cast(wrapper); } template - static TearOffType* lookupWrapper(OwnerType* element, const SVGPropertyInfo* info) + static PassRefPtr lookupWrapper(OwnerType* element, const SVGPropertyInfo* info) { ASSERT(info); SVGAnimatedPropertyDescription key(element, info->propertyIdentifier); @@ -72,7 +76,7 @@ public: } template - static TearOffType* lookupWrapper(const OwnerType* element, const SVGPropertyInfo* info) + static PassRefPtr lookupWrapper(const OwnerType* element, const SVGPropertyInfo* info) { return lookupWrapper(const_cast(element), info); } diff --git a/Source/WebCore/svg/properties/SVGAnimatedPropertyMacros.h b/Source/WebCore/svg/properties/SVGAnimatedPropertyMacros.h index 1ddbc5c6c..e31015e52 100644 --- a/Source/WebCore/svg/properties/SVGAnimatedPropertyMacros.h +++ b/Source/WebCore/svg/properties/SVGAnimatedPropertyMacros.h @@ -118,7 +118,7 @@ public: \ static const SVGPropertyInfo* LowerProperty##PropertyInfo(); \ PropertyType& LowerProperty() const \ { \ - if (TearOffType* wrapper = SVGAnimatedProperty::lookupWrapper(this, LowerProperty##PropertyInfo())) { \ + if (RefPtr wrapper = SVGAnimatedProperty::lookupWrapper(this, LowerProperty##PropertyInfo())) { \ if (wrapper->isAnimating()) \ return wrapper->currentAnimatedValue(); \ } \ @@ -179,7 +179,7 @@ private: \ DECLARE_ANIMATED_PROPERTY(TearOffType, PropertyType, UpperProperty, LowerProperty) \ void detachAnimated##UpperProperty##ListWrappers(unsigned newListSize) \ { \ - if (TearOffType* wrapper = SVGAnimatedProperty::lookupWrapper(this, LowerProperty##PropertyInfo())) \ + if (RefPtr wrapper = SVGAnimatedProperty::lookupWrapper(this, LowerProperty##PropertyInfo())) \ wrapper->detachListWrappers(newListSize); \ } diff --git a/Source/WebCore/svg/properties/SVGAnimatedTransformListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGAnimatedTransformListPropertyTearOff.h index 6f643e87c..2953f2ff6 100644 --- a/Source/WebCore/svg/properties/SVGAnimatedTransformListPropertyTearOff.h +++ b/Source/WebCore/svg/properties/SVGAnimatedTransformListPropertyTearOff.h @@ -29,18 +29,24 @@ namespace WebCore { class SVGAnimatedTransformListPropertyTearOff : public SVGAnimatedListPropertyTearOff { public: - virtual SVGListPropertyTearOff* baseVal() + virtual PassRefPtr baseVal() OVERRIDE { - if (!m_baseVal) - m_baseVal = SVGTransformListPropertyTearOff::create(this, BaseValRole, m_values, m_wrappers); - return static_cast*>(m_baseVal.get()); + if (m_baseVal) + return m_baseVal; + + RefPtr property = SVGTransformListPropertyTearOff::create(this, BaseValRole, m_values, m_wrappers); + m_baseVal = property.get(); + return property.release(); } - virtual SVGListPropertyTearOff* animVal() + virtual PassRefPtr animVal() OVERRIDE { - if (!m_animVal) - m_animVal = SVGTransformListPropertyTearOff::create(this, AnimValRole, m_values, m_wrappers); - return static_cast*>(m_animVal.get()); + if (m_animVal) + return m_animVal; + + RefPtr property = SVGTransformListPropertyTearOff::create(this, AnimValRole, m_values, m_wrappers); + m_animVal = property.get(); + return property.release(); } static PassRefPtr create(SVGElement* contextElement, const QualifiedName& attributeName, AnimatedPropertyType animatedPropertyType, SVGTransformList& values) diff --git a/Source/WebCore/svg/properties/SVGListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGListPropertyTearOff.h index 3c6552467..326a89a74 100644 --- a/Source/WebCore/svg/properties/SVGListPropertyTearOff.h +++ b/Source/WebCore/svg/properties/SVGListPropertyTearOff.h @@ -121,6 +121,12 @@ protected: { } + virtual ~SVGListPropertyTearOff() + { + if (m_animatedProperty) + m_animatedProperty->propertyWillBeDeleted(*this); + } + virtual bool isReadOnly() const { if (m_role == AnimValRole) diff --git a/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.cpp b/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.cpp index 573d453a4..51c48b4c3 100644 --- a/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.cpp +++ b/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.cpp @@ -95,7 +95,7 @@ SVGPathElement* SVGPathSegListPropertyTearOff::contextElement() const bool SVGPathSegListPropertyTearOff::processIncomingListItemValue(const ListItemType& newItem, unsigned* indexToModify) { SVGPathSegWithContext* newItemWithContext = static_cast(newItem.get()); - SVGAnimatedProperty* animatedPropertyOfItem = newItemWithContext->animatedProperty(); + RefPtr animatedPropertyOfItem = newItemWithContext->animatedProperty(); // Alter the role, after calling animatedProperty(), as that may influence the returned animated property. newItemWithContext->setContextAndRole(contextElement(), m_pathSegRole); @@ -111,7 +111,7 @@ bool SVGPathSegListPropertyTearOff::processIncomingListItemValue(const ListItemT // Spec: If newItem is already in a list, it is removed from its previous list before it is inserted into this list. // 'newItem' is already living in another list. If it's not our list, synchronize the other lists wrappers after the removal. bool livesInOtherList = animatedPropertyOfItem != m_animatedProperty; - SVGAnimatedPathSegListPropertyTearOff* propertyTearOff = static_cast(animatedPropertyOfItem); + RefPtr propertyTearOff = static_pointer_cast(animatedPropertyOfItem); int indexToRemove = propertyTearOff->findItem(newItem.get()); ASSERT(indexToRemove != -1); -- cgit v1.2.1 From 21a11f7197da091eae93e987e73fbf5537dfaf3a Mon Sep 17 00:00:00 2001 From: Said Abou-Hallawa Date: Tue, 8 Mar 2016 16:13:36 +0100 Subject: Fixed crashes on SVG path animation use cases. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Based on upstream fix by said@apple.com at http://trac.webkit.org/changeset/196670 A destructor was added to SVGListPropertyTearOff that notifies its wrapper (the SVGAnimatedListPropertyTearoff) about its deletion. This allows the wrapper to nullify any references to the wrapped content. We needed to do the same thing for SVGPathSegListPropertyTearOff. Both SVGPathSegListPropertyTearOff and SVGListPropertyTearOff inherit from SVGListProperty and both hold pointers to SVGAnimatedListPropertyTearOff which needs to be notified. Change-Id: I1873825c7bdc07bf06cd5c300156ebe084f2607e Reviewed-by: Konstantin Tokarev Reviewed-by: Michael Brüning --- Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.h index ee704d681..4c1a30c4b 100644 --- a/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.h +++ b/Source/WebCore/svg/properties/SVGPathSegListPropertyTearOff.h @@ -111,6 +111,12 @@ public: return Base::appendItemValues(newItem, ec); } + virtual ~SVGPathSegListPropertyTearOff() + { + if (m_animatedProperty) + m_animatedProperty->propertyWillBeDeleted(*this); + } + private: SVGPathSegListPropertyTearOff(AnimatedListPropertyTearOff* animatedProperty, SVGPropertyRole role, SVGPathSegRole pathSegRole, SVGPathSegList& values, ListWrapperCache& wrappers) : SVGListProperty(role, values, &wrappers) -- cgit v1.2.1 From 7205faf1a546a690f68176989100109e9a3335b7 Mon Sep 17 00:00:00 2001 From: Javier Fernandez Date: Mon, 7 Mar 2016 23:56:13 +0100 Subject: Many assertion failures and crashes on SVG path animation cases when JS garbage collection happens quickly. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Based on upstream fix by said@apple.com at http://trac.webkit.org/changeset/197125 Since the whole document was leaking once an SVGAnimatedProperty was created so there was no way to produce this bug. After fixing the leak, one crash and one assert got uncovered. Both of them happen because of the fact: "if an SVGAnimatedProperty is not referenced it will be deleted." * svg/SVGPathElement.cpp: (WebCore::SVGPathElement::lookupOrCreateDWrapper): The code in this function was assuming that the wrapper will be created only once which happens when SVGAnimatedProperty::lookupOrCreateWrapper() is called. Before making this single call, lookupOrCreateDWrapper() was building an initial SVGPathSegList from byte stream. But now SVGAnimatedProperty::lookupWrapper() can return false even after creating the SVGAnimatedProperty because it was deleted later. Calling buildSVGPathSegListFromByteStream() more than once was causing SVGAnimatedListPropertyTearOff::animationStarted() to fire the assertion ASSERT(m_values.size() == m_wrappers.size()) because the path segments were appended twice to m_values which is in fact SVGPathElement::m_pathSegList.value. The fix is to build the initial SVGPathSegList only once which should happen when m_pathSegList.value.isEmpty(). (WebCore::SVGPathElement::animatedPropertyWillBeDeleted): * svg/SVGPathElement.h: * svg/properties/SVGAnimatedPathSegListPropertyTearOff.h: (WebCore::SVGAnimatedPathSegListPropertyTearOff::~SVGAnimatedPathSegListPropertyTearOff): SVGPathElement is assuming the following equivalence relation: m_pathSegList.shouldSynchronize ~ SVGAnimatedProperty_is_created_and_not_null. SVGPathElement::animatedPathSegList() and animatedNormalizedPathSegList() set m_pathSegList.shouldSynchronize to true when SVGAnimatedProperty is created but nothing sets m_pathSegList.shouldSynchronize back to false. This was not a problem when the SVGAnimatedProperty was leaking but after ensuring it is deleted when it is not referenced this equivalence relation becomes untrue sometimes. This caused SVGPathElement::svgAttributeChanged() to crash when we check m_pathSegList.shouldSynchronize and if it is true we assume that SVGAnimatedProperty::lookupWrapper() will return a non-null pointer and therefore we deference this pointer and call SVGAnimatedProperty::isAnimating(). To fix this crash we need to set m_pathSegList.shouldSynchronize back to false when the associated SVGAnimatedProperty is deleted. Change-Id: I05be755635b02d0d76105fc2eb21c2f013298c4e Reviewed-by: Michael Brüning --- Source/WebCore/svg/SVGPathElement.cpp | 13 +++++++++++-- Source/WebCore/svg/SVGPathElement.h | 4 +++- .../svg/properties/SVGAnimatedPathSegListPropertyTearOff.h | 7 +++++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/Source/WebCore/svg/SVGPathElement.cpp b/Source/WebCore/svg/SVGPathElement.cpp index b030c9a83..c34c476f4 100644 --- a/Source/WebCore/svg/SVGPathElement.cpp +++ b/Source/WebCore/svg/SVGPathElement.cpp @@ -329,13 +329,22 @@ PassRefPtr SVGPathElement::lookupOrCreateDWrapper(SVGElemen if (RefPtr property = SVGAnimatedProperty::lookupWrapper(ownerType, dPropertyInfo())) return property; - // Build initial SVGPathSegList. - buildSVGPathSegListFromByteStream(ownerType->m_pathByteStream.get(), ownerType, ownerType->m_pathSegList.value, UnalteredParsing); + if (ownerType->m_pathSegList.value.isEmpty()) + buildSVGPathSegListFromByteStream(ownerType->m_pathByteStream.get(), ownerType, ownerType->m_pathSegList.value, UnalteredParsing); return SVGAnimatedProperty::lookupOrCreateWrapper (ownerType, dPropertyInfo(), ownerType->m_pathSegList.value); } +void SVGPathElement::animatedPropertyWillBeDeleted() +{ + // m_pathSegList.shouldSynchronize is set to true when the 'd' wrapper for m_pathSegList + // is created and cached. We need to reset it back to false when this wrapper is deleted + // so we can be sure if shouldSynchronize is true, SVGAnimatedProperty::lookupWrapper() + // will return a valid cached 'd' wrapper for the m_pathSegList. + m_pathSegList.shouldSynchronize = false; +} + void SVGPathElement::synchronizeD(SVGElement* contextElement) { ASSERT(contextElement); diff --git a/Source/WebCore/svg/SVGPathElement.h b/Source/WebCore/svg/SVGPathElement.h index 0ca66ac11..e9db8a56e 100644 --- a/Source/WebCore/svg/SVGPathElement.h +++ b/Source/WebCore/svg/SVGPathElement.h @@ -98,7 +98,9 @@ public: bool isAnimValObserved() const { return m_isAnimValObserved; } -private: + void animatedPropertyWillBeDeleted(); + + private: SVGPathElement(const QualifiedName&, Document*); virtual bool isValid() const { return SVGTests::isValid(); } diff --git a/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h b/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h index 1cfe0230e..dd5396aff 100644 --- a/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h +++ b/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h @@ -113,6 +113,13 @@ private: : SVGAnimatedListPropertyTearOff(contextElement, attributeName, animatedPropertyType, values) , m_animatedPathByteStream(0) { + ASSERT(contextElement); + ASSERT(toSVGPathElement(contextElement)); + } + + virtual ~SVGAnimatedPathSegListPropertyTearOff() + { + static_cast(contextElement())->animatedPropertyWillBeDeleted(); } SVGPathByteStream* m_animatedPathByteStream; -- cgit v1.2.1 From 3b4c850361abbc2aae556dbf99c8e4c8086ae569 Mon Sep 17 00:00:00 2001 From: Konstantin Tokarev Date: Tue, 22 Mar 2016 15:58:19 +0300 Subject: Allow using system SQLite without pkg-config Change-Id: Ifff0f8877a2d2d77a04468c205c3353f043e7738 Reviewed-by: Allan Sandfeld Jensen --- Source/WebCore/WebCore.pri | 2 +- Tools/qmake/config.tests/libsqlite3/libsqlite3.cpp | 34 ++++++++++++++++++++++ Tools/qmake/config.tests/libsqlite3/libsqlite3.pro | 5 ++++ Tools/qmake/mkspecs/features/features.prf | 2 +- 4 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 Tools/qmake/config.tests/libsqlite3/libsqlite3.cpp create mode 100644 Tools/qmake/config.tests/libsqlite3/libsqlite3.pro diff --git a/Source/WebCore/WebCore.pri b/Source/WebCore/WebCore.pri index ffe389d46..01893948e 100644 --- a/Source/WebCore/WebCore.pri +++ b/Source/WebCore/WebCore.pri @@ -233,7 +233,7 @@ use?(GRAPHICS_SURFACE) { } have?(sqlite3) { - mac { + osx|contains(QT_CONFIG, no-pkg-config) { LIBS += -lsqlite3 } else { PKGCONFIG += sqlite3 diff --git a/Tools/qmake/config.tests/libsqlite3/libsqlite3.cpp b/Tools/qmake/config.tests/libsqlite3/libsqlite3.cpp new file mode 100644 index 000000000..34434d195 --- /dev/null +++ b/Tools/qmake/config.tests/libsqlite3/libsqlite3.cpp @@ -0,0 +1,34 @@ +/* + * Copyright (C) 2016 Konstantin Tokavev + * + * 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 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. + */ + +#include + +int main(int, char**) +{ + sqlite3 *db; + sqlite3_open("data.db", &db); + sqlite3_close(db); + return 0; +} diff --git a/Tools/qmake/config.tests/libsqlite3/libsqlite3.pro b/Tools/qmake/config.tests/libsqlite3/libsqlite3.pro new file mode 100644 index 000000000..2c39c5266 --- /dev/null +++ b/Tools/qmake/config.tests/libsqlite3/libsqlite3.pro @@ -0,0 +1,5 @@ +SOURCES = libsqlite3.cpp +OBJECTS_DIR = obj +LIBS += -lsqlite3 + +load(qt_build_config) diff --git a/Tools/qmake/mkspecs/features/features.prf b/Tools/qmake/mkspecs/features/features.prf index 7b0b49d90..6a9e9a219 100644 --- a/Tools/qmake/mkspecs/features/features.prf +++ b/Tools/qmake/mkspecs/features/features.prf @@ -117,7 +117,7 @@ defineTest(detectFeatures) { } # Try to use an system wide SQlite installation - if(!contains(QT_CONFIG, no-pkg-config):packagesExist("sqlite3"))|mac { + if(!contains(QT_CONFIG, no-pkg-config):packagesExist("sqlite3"))|config_libsqlite3 { WEBKIT_CONFIG += have_sqlite3 } else { SQLITE3SRCDIR = $$(SQLITE3SRCDIR) -- cgit v1.2.1 From b555ab7169f23d47fe980aa0c76c0eb9d18d6ab1 Mon Sep 17 00:00:00 2001 From: Konstantin Tokarev Date: Fri, 18 Mar 2016 21:57:07 +0300 Subject: Allow building QtWebKit on Windows with non-ICU Qt build. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also, from now use_wchar_unicode is never silently enabled when ICU config test fails, and requires qmake argument WEBKIT_CONFIG+=use_wchar_unicode. Change-Id: I434f5245c796b723a3bb116f62f8d53d05c3b4f7 Reviewed-by: Michael Brüning --- Source/WebCore/Target.pri | 6 +++--- Tools/qmake/mkspecs/features/configure.prf | 2 +- Tools/qmake/mkspecs/features/features.prf | 2 -- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Source/WebCore/Target.pri b/Source/WebCore/Target.pri index d1aad9f9b..e525aa1fc 100644 --- a/Source/WebCore/Target.pri +++ b/Source/WebCore/Target.pri @@ -1122,7 +1122,6 @@ SOURCES += \ platform/text/TextCodecUTF8.cpp \ platform/text/TextCodecICU.cpp \ platform/text/TextEncoding.cpp \ - platform/text/TextEncodingDetectorICU.cpp \ platform/text/TextEncodingRegistry.cpp \ platform/text/TextStream.cpp \ platform/ThreadGlobalData.cpp \ @@ -2978,11 +2977,12 @@ mac { platform/text/cf/StringImplCF.cpp } -contains(QT_CONFIG,icu)|mac: SOURCES += platform/text/TextBreakIteratorICU.cpp use?(wchar_unicode): { SOURCES += platform/text/wchar/TextBreakIteratorWchar.cpp \ platform/text/TextEncodingDetectorNone.cpp - SOURCES -= platform/text/TextEncodingDetectorICU.cpp +} else { + SOURCES += platform/text/TextBreakIteratorICU.cpp \ + platform/text/TextEncodingDetectorICU.cpp } mac { diff --git a/Tools/qmake/mkspecs/features/configure.prf b/Tools/qmake/mkspecs/features/configure.prf index 757ddfe4e..49f849576 100644 --- a/Tools/qmake/mkspecs/features/configure.prf +++ b/Tools/qmake/mkspecs/features/configure.prf @@ -120,7 +120,7 @@ defineTest(finalizeConfigure) { } # Sanity checks that would prevent us from building the whole project altogether. - !android:!mac:!config_icu { + !config_icu:!osx:!use?(wchar_unicode) { addReasonForSkippingBuild("ICU is required.") } production_build:blackberry { diff --git a/Tools/qmake/mkspecs/features/features.prf b/Tools/qmake/mkspecs/features/features.prf index 6a9e9a219..bb3432bd2 100644 --- a/Tools/qmake/mkspecs/features/features.prf +++ b/Tools/qmake/mkspecs/features/features.prf @@ -142,8 +142,6 @@ defineTest(detectFeatures) { # IndexedDB requires leveldb enable?(indexed_database): WEBKIT_CONFIG += use_leveldb - !config_icu:!mac: WEBKIT_CONFIG += use_wchar_unicode - export(WEBKIT_CONFIG) export(CONFIGURE_WARNINGS) } -- cgit v1.2.1 From 4d435a7c294703e7a4a48cc962bc4f2a8db3f9c0 Mon Sep 17 00:00:00 2001 From: Konstantin Tokarev Date: Sun, 10 Apr 2016 01:46:27 +0000 Subject: Fixed compilation of JPEGImageDecoder with libjpeg v9. https://bugs.webkit.org/show_bug.cgi?id=156445 Patch by Konstantin Tokarev on 2016-04-09 Reviewed by Michael Catanzaro. ICU defines TRUE and FALSE macros, breaking libjpeg v9 headers. No new tests needed. * platform/image-decoders/jpeg/JPEGImageDecoder.h: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@199278 268f45cc-cd09-0410-ab3c-d52691b4dbfc Change-Id: I82db8bae210f8b03bd472a82925bd308fa01b6ca Reviewed-by: Allan Sandfeld Jensen --- Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h b/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h index b1f61a14d..3231eedba 100644 --- a/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h +++ b/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h @@ -40,6 +40,9 @@ #define XMD_H #endif +// ICU defines TRUE and FALSE macros, breaking libjpeg v9 headers +#undef TRUE +#undef FALSE extern "C" { #include "jpeglib.h" } -- cgit v1.2.1 From 71136c9621103522e85654c8e144d5f1c961de1c Mon Sep 17 00:00:00 2001 From: Maurice van der Pot Date: Mon, 31 Mar 2014 10:21:27 +0000 Subject: Fix mixed use of booleans in JPEGImageDecoder.cpp https://bugs.webkit.org/show_bug.cgi?id=122412 Patch by Maurice van der Pot on 2014-03-31 Reviewed by Darin Adler. Trivial fix for compilation error; no new tests. * platform/image-decoders/jpeg/JPEGImageDecoder.cpp: (WebCore::JPEGImageReader::decode): (WebCore::fill_input_buffer): Use TRUE/FALSE defined by libjpeg for libjpeg booleans git-svn-id: http://svn.webkit.org/repository/webkit/trunk@166490 268f45cc-cd09-0410-ab3c-d52691b4dbfc Change-Id: I6c669c951fa4bc87862b261ad1a9dd05016086e3 Reviewed-by: Allan Sandfeld Jensen --- .../WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp b/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp index 7da75c874..38aa71950 100644 --- a/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp +++ b/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp @@ -334,7 +334,7 @@ public: switch (m_state) { case JPEG_HEADER: // Read file parameters with jpeg_read_header(). - if (jpeg_read_header(&m_info, true) == JPEG_SUSPENDED) + if (jpeg_read_header(&m_info, TRUE) == JPEG_SUSPENDED) return false; // I/O suspension. switch (m_info.jpeg_color_space) { @@ -420,9 +420,9 @@ public: // of progressive JPEG. m_info.dct_method = dctMethod(); m_info.dither_mode = ditherMode(); - m_info.do_fancy_upsampling = doFancyUpsampling(); - m_info.enable_2pass_quant = false; - m_info.do_block_smoothing = true; + m_info.do_fancy_upsampling = doFancyUpsampling() ? TRUE : FALSE; + m_info.enable_2pass_quant = FALSE; + m_info.do_block_smoothing = TRUE; // Start decompressor. if (!jpeg_start_decompress(&m_info)) @@ -573,7 +573,7 @@ boolean fill_input_buffer(j_decompress_ptr) // Our decode step always sets things up properly, so if this method is ever // called, then we have hit the end of the buffer. A return value of false // indicates that we have no data to supply yet. - return false; + return FALSE; } void term_source(j_decompress_ptr jd) -- cgit v1.2.1 From c1b8d4bf2a36cd59e31758a9e6af872c17c4cfb8 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Tue, 3 May 2016 13:34:22 +0200 Subject: Only load QImageIO plugins from white-listed formats Not all QImage plugins are safe to load from the internet. We should only load formats that are well-used on the internet and we can be reasonably sure are safe. [ChangeLog][WebKit][Behavior Change] QtWebkit will no longer support any QImage plugin with the Size option, but instead only decode formats that have been whitelisted. If you are using QtWebKit for controlled content and wish to override the white-listed it can now be done with the environment variable QTWEBKIT_IMAGEFORMAT_WHITELIST which takes a comma-separated list of QImageIO formats. Change-Id: Ifc4f1a3addfa4ec117697a12000db3c265422314 Reviewed-by: Richard J. Moore --- .../platform/graphics/qt/ImageDecoderQt.cpp | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/Source/WebCore/platform/graphics/qt/ImageDecoderQt.cpp b/Source/WebCore/platform/graphics/qt/ImageDecoderQt.cpp index 2917815bd..74696c23d 100644 --- a/Source/WebCore/platform/graphics/qt/ImageDecoderQt.cpp +++ b/Source/WebCore/platform/graphics/qt/ImageDecoderQt.cpp @@ -31,6 +31,7 @@ #include #include +#include #include namespace WebCore { @@ -45,6 +46,25 @@ ImageDecoderQt::~ImageDecoderQt() { } +static const char* s_formatWhiteList[] = {"png", "jpeg", "gif", "webp", "bmp", "svg", "ico", 0}; + +static bool isFormatWhiteListed(const QByteArray &format) +{ + static QSet whiteListSet; + if (whiteListSet.isEmpty()) { + QByteArray whiteListEnv = qgetenv("QTWEBKIT_IMAGEFORMAT_WHITELIST"); + if (!whiteListEnv.isEmpty()) + whiteListSet = QSet::fromList(whiteListEnv.split(',')); + + const char **formatIt = s_formatWhiteList; + while (*formatIt) { + whiteListSet.insert(QByteArray(*formatIt)); + ++formatIt; + } + } + return whiteListSet.contains(format); +} + void ImageDecoderQt::setData(SharedBuffer* data, bool allDataReceived) { if (failed()) @@ -73,6 +93,11 @@ void ImageDecoderQt::setData(SharedBuffer* data, bool allDataReceived) // QImageReader only allows retrieving the format before reading the image m_format = m_reader->format(); + if (!isFormatWhiteListed(m_format)) { + qWarning("Image of format '%s' blocked because it is not considered safe. If you are sure it is safe to do so, you can white-list the format by setting the environment variable QTWEBKIT_IMAGEFORMAT_WHITELIST=%s", m_format.constData(), m_format.constData()); + setFailed(); + m_reader.clear(); + } } bool ImageDecoderQt::isSizeAvailable() -- cgit v1.2.1 From b6198bf56c2aa3f91a7773d21375abcc4bcb69fc Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Fri, 22 Apr 2016 21:05:49 +0200 Subject: make use of COPIES Change-Id: I050003e46c437a3fadce11d417861f40d60f6c20 Reviewed-by: Joerg Bornemann --- Source/WebKit/qt/declarative/experimental/experimental.pri | 11 +++-------- Source/WebKit/qt/declarative/public.pri | 11 +++-------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/Source/WebKit/qt/declarative/experimental/experimental.pri b/Source/WebKit/qt/declarative/experimental/experimental.pri index 547e66bcc..c59f4569e 100644 --- a/Source/WebKit/qt/declarative/experimental/experimental.pri +++ b/Source/WebKit/qt/declarative/experimental/experimental.pri @@ -11,14 +11,9 @@ TARGET.module_name = QtWebKit/experimental CONFIG += plugin -QMLDIRFILE = $${_PRO_FILE_PWD_}/qmldir -copy2build.input = QMLDIRFILE -copy2build.output = $${ROOT_BUILD_DIR}/imports/$${TARGET.module_name}/qmldir -!contains(TEMPLATE_PREFIX, vc):copy2build.variable_out = PRE_TARGETDEPS -copy2build.commands = $$QMAKE_COPY ${QMAKE_FILE_IN} ${QMAKE_FILE_OUT} -copy2build.name = COPY ${QMAKE_FILE_IN} -copy2build.CONFIG += no_link -QMAKE_EXTRA_COMPILERS += copy2build +cpqmldir.files = $${_PRO_FILE_PWD_}/qmldir +cpqmldir.path = $${ROOT_BUILD_DIR}/imports/$${TARGET.module_name} +COPIES += cpqmldir contains(QT_CONFIG, reduce_exports):CONFIG += hide_symbols diff --git a/Source/WebKit/qt/declarative/public.pri b/Source/WebKit/qt/declarative/public.pri index d669162e4..bdef38045 100644 --- a/Source/WebKit/qt/declarative/public.pri +++ b/Source/WebKit/qt/declarative/public.pri @@ -11,14 +11,9 @@ TARGET.module_name = QtWebKit CONFIG += plugin -QMLDIRFILE = $${_PRO_FILE_PWD_}/qmldir -copy2build.input = QMLDIRFILE -copy2build.output = $${ROOT_BUILD_DIR}/imports/$${TARGET.module_name}/qmldir -!contains(TEMPLATE_PREFIX, vc):copy2build.variable_out = PRE_TARGETDEPS -copy2build.commands = $$QMAKE_COPY ${QMAKE_FILE_IN} ${QMAKE_FILE_OUT} -copy2build.name = COPY ${QMAKE_FILE_IN} -copy2build.CONFIG += no_link -QMAKE_EXTRA_COMPILERS += copy2build +cpqmldir.files = $${_PRO_FILE_PWD_}/qmldir +cpqmldir.path = $${ROOT_BUILD_DIR}/imports/$${TARGET.module_name} +COPIES += cpqmldir contains(QT_CONFIG, reduce_exports):CONFIG += hide_symbols -- cgit v1.2.1 From a750b262b856178eee47d546944bd095662dccf7 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Tue, 3 May 2016 19:46:54 +0200 Subject: decruft project file the "created by qt creator" header is not supposed to be checked in. Change-Id: I78294e52858387946b2bafd258f7be6b0b5d87d6 Reviewed-by: Joerg Bornemann --- Source/WebKit/qt/tests/hybridPixmap/hybridPixmap.pro | 3 --- 1 file changed, 3 deletions(-) diff --git a/Source/WebKit/qt/tests/hybridPixmap/hybridPixmap.pro b/Source/WebKit/qt/tests/hybridPixmap/hybridPixmap.pro index 0e49a7056..99197e1bb 100644 --- a/Source/WebKit/qt/tests/hybridPixmap/hybridPixmap.pro +++ b/Source/WebKit/qt/tests/hybridPixmap/hybridPixmap.pro @@ -1,6 +1,3 @@ -# ------------------------------------------------- -# Project created by QtCreator 2009-12-10T11:25:02 -# ------------------------------------------------- include(../tests.pri) TARGET = hybridPixmap SOURCES += widget.cpp -- cgit v1.2.1