[CSSRegions]Use RefPtr's instead of weak references on DOMNamedFlowCollection
https://bugs.webkit.org/show_bug.cgi?id=95311
Patch by Andrei Onea <onea@adobe.com> on 2012-09-12
Reviewed by Andreas Kling.
Source/WebCore:
Currently, DOMNamedFlowCollection holds a collection of raw pointers to WebKitNamedFlow.
This causes a crash when there is a JS instance of a NamedFlowCollection snapshot taken
before the NamedFlow is deleted, since the memory previously occupied by the NamedFlow
can be accessed. Because of this, we need to use RefPtr's for the snapshot, so that such
dangling references extend the lifetime of the NamedFlow objects.
Test: fast/regions/webkit-named-flow-collection-crash.html
* dom/DOMNamedFlowCollection.cpp:
(WebCore::DOMNamedFlowCollection::DOMNamedFlowCollection):
(WebCore::DOMNamedFlowCollection::item):
(WebCore::DOMNamedFlowCollection::namedItem):
(WebCore):
(WebCore::DOMNamedFlowCollection::DOMNamedFlowHashFunctions::hash):
(WebCore::DOMNamedFlowCollection::DOMNamedFlowHashFunctions::equal):
(DOMNamedFlowCollection::DOMNamedFlowHashFunctions):
(WebCore::DOMNamedFlowCollection::DOMNamedFlowHashTranslator::hash):
(WebCore::DOMNamedFlowCollection::DOMNamedFlowHashTranslator::equal):
Create new internal ListHashSet for RefPtr<WebKitNamedFlow>.
* dom/DOMNamedFlowCollection.h:
(WebCore::DOMNamedFlowCollection::create):
(DOMNamedFlowCollection):
* dom/NamedFlowCollection.cpp:
(WebCore::NamedFlowCollection::createCSSOMSnapshot):
(WebCore):
(WebCore::NamedFlowCollection::NamedFlowHashFunctions::hash):
(WebCore::NamedFlowCollection::NamedFlowHashFunctions::equal):
(NamedFlowCollection::NamedFlowHashFunctions):
(WebCore::NamedFlowCollection::NamedFlowHashTranslator::hash):
(WebCore::NamedFlowCollection::NamedFlowHashTranslator::equal):
Move back the definitions for NamedFlowHashFunctions and NamedFlowHashTranslator
to the .cpp file.
* dom/NamedFlowCollection.h:
(NamedFlowCollection):
LayoutTests:
Added test for crash which occurs when there is a raw pointer to a NamedFlow
(in a NamedFlowCollection) which has been freed.
* fast/regions/webkit-named-flow-collection-crash-expected.txt: Added.
* fast/regions/webkit-named-flow-collection-crash.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@128325 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index d42756b..b3430b1 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2012-09-12 Andrei Onea <onea@adobe.com>
+
+ [CSSRegions]Use RefPtr's instead of weak references on DOMNamedFlowCollection
+ https://bugs.webkit.org/show_bug.cgi?id=95311
+
+ Reviewed by Andreas Kling.
+
+ Added test for crash which occurs when there is a raw pointer to a NamedFlow
+ (in a NamedFlowCollection) which has been freed.
+
+ * fast/regions/webkit-named-flow-collection-crash-expected.txt: Added.
+ * fast/regions/webkit-named-flow-collection-crash.html: Added.
+
2012-09-12 MORITA Hajime <morrita@google.com>
[Shadow DOM] Unpolished elements should reject author shadows
diff --git a/LayoutTests/fast/regions/webkit-named-flow-collection-crash-expected.txt b/LayoutTests/fast/regions/webkit-named-flow-collection-crash-expected.txt
new file mode 100644
index 0000000..b3f1a63
--- /dev/null
+++ b/LayoutTests/fast/regions/webkit-named-flow-collection-crash-expected.txt
@@ -0,0 +1,4 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS
diff --git a/LayoutTests/fast/regions/webkit-named-flow-collection-crash.html b/LayoutTests/fast/regions/webkit-named-flow-collection-crash.html
new file mode 100644
index 0000000..22bec47
--- /dev/null
+++ b/LayoutTests/fast/regions/webkit-named-flow-collection-crash.html
@@ -0,0 +1,31 @@
+<!doctype html>
+<html>
+ <head>
+ <style>
+ #content { -webkit-flow-into: flow; }
+ #container { -webkit-flow-from: flow; width: 100px; height: 100px;}
+ </style>
+ </head>
+ <script src="../../fast/js/resources/js-test-pre.js"></script>
+ <body>
+ <div id="content"></div>
+ <div id="container"></div>
+ <script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+ var namedFlowCollection = document.webkitGetNamedFlows();
+ document.body.removeChild(document.getElementById("content"));
+ document.body.removeChild(document.getElementById("container"));
+
+ // Force a re-layout event, which forces the deletion of the underlying NamedFlow.
+ document.body.offsetTop;
+
+ var namedFlow = namedFlowCollection.flow;
+ if (namedFlow.name == "flow")
+ document.body.innerText = "PASS";
+ else
+ document.body.innerText = "FAIL";
+ </script>
+ <script src="../../fast/js/resources/js-test-post.js"></script>
+ </body>
+</html>
\ No newline at end of file
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index b217ab1..0d8e855 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,45 @@
+2012-09-12 Andrei Onea <onea@adobe.com>
+
+ [CSSRegions]Use RefPtr's instead of weak references on DOMNamedFlowCollection
+ https://bugs.webkit.org/show_bug.cgi?id=95311
+
+ Reviewed by Andreas Kling.
+
+ Currently, DOMNamedFlowCollection holds a collection of raw pointers to WebKitNamedFlow.
+ This causes a crash when there is a JS instance of a NamedFlowCollection snapshot taken
+ before the NamedFlow is deleted, since the memory previously occupied by the NamedFlow
+ can be accessed. Because of this, we need to use RefPtr's for the snapshot, so that such
+ dangling references extend the lifetime of the NamedFlow objects.
+
+ Test: fast/regions/webkit-named-flow-collection-crash.html
+
+ * dom/DOMNamedFlowCollection.cpp:
+ (WebCore::DOMNamedFlowCollection::DOMNamedFlowCollection):
+ (WebCore::DOMNamedFlowCollection::item):
+ (WebCore::DOMNamedFlowCollection::namedItem):
+ (WebCore):
+ (WebCore::DOMNamedFlowCollection::DOMNamedFlowHashFunctions::hash):
+ (WebCore::DOMNamedFlowCollection::DOMNamedFlowHashFunctions::equal):
+ (DOMNamedFlowCollection::DOMNamedFlowHashFunctions):
+ (WebCore::DOMNamedFlowCollection::DOMNamedFlowHashTranslator::hash):
+ (WebCore::DOMNamedFlowCollection::DOMNamedFlowHashTranslator::equal):
+ Create new internal ListHashSet for RefPtr<WebKitNamedFlow>.
+ * dom/DOMNamedFlowCollection.h:
+ (WebCore::DOMNamedFlowCollection::create):
+ (DOMNamedFlowCollection):
+ * dom/NamedFlowCollection.cpp:
+ (WebCore::NamedFlowCollection::createCSSOMSnapshot):
+ (WebCore):
+ (WebCore::NamedFlowCollection::NamedFlowHashFunctions::hash):
+ (WebCore::NamedFlowCollection::NamedFlowHashFunctions::equal):
+ (NamedFlowCollection::NamedFlowHashFunctions):
+ (WebCore::NamedFlowCollection::NamedFlowHashTranslator::hash):
+ (WebCore::NamedFlowCollection::NamedFlowHashTranslator::equal):
+ Move back the definitions for NamedFlowHashFunctions and NamedFlowHashTranslator
+ to the .cpp file.
+ * dom/NamedFlowCollection.h:
+ (NamedFlowCollection):
+
2012-09-12 MORITA Hajime <morrita@google.com>
[Shadow DOM] Unpolished elements should reject author shadows
diff --git a/Source/WebCore/dom/DOMNamedFlowCollection.cpp b/Source/WebCore/dom/DOMNamedFlowCollection.cpp
index 30fbab3..aedd9e9 100644
--- a/Source/WebCore/dom/DOMNamedFlowCollection.cpp
+++ b/Source/WebCore/dom/DOMNamedFlowCollection.cpp
@@ -34,9 +34,10 @@
namespace WebCore {
-DOMNamedFlowCollection::DOMNamedFlowCollection(NamedFlowCollection::NamedFlowSet& set)
+DOMNamedFlowCollection::DOMNamedFlowCollection(const Vector<WebKitNamedFlow*>& namedFlows)
{
- m_namedFlows.swap(set);
+ for (Vector<WebKitNamedFlow*>::const_iterator it = namedFlows.begin(); it != namedFlows.end(); ++it)
+ m_namedFlows.add(*it);
}
unsigned long DOMNamedFlowCollection::length() const
@@ -48,7 +49,7 @@
{
if (index >= static_cast<unsigned long>(m_namedFlows.size()))
return 0;
- NamedFlowCollection::NamedFlowSet::const_iterator it = m_namedFlows.begin();
+ DOMNamedFlowSet::const_iterator it = m_namedFlows.begin();
for (unsigned long i = 0; i < index; ++i)
++it;
return *it;
@@ -56,7 +57,7 @@
PassRefPtr<WebKitNamedFlow> DOMNamedFlowCollection::namedItem(const AtomicString& name) const
{
- NamedFlowCollection::NamedFlowSet::const_iterator it = m_namedFlows.find<String, NamedFlowCollection::NamedFlowHashTranslator>(name);
+ DOMNamedFlowSet::const_iterator it = m_namedFlows.find<String, DOMNamedFlowHashTranslator>(name);
if (it != m_namedFlows.end())
return *it;
return 0;
@@ -66,6 +67,20 @@
{
return namedItem(name);
}
+
+// The HashFunctions object used by the HashSet to compare between RefPtr<NamedFlows>.
+// It is safe to set safeToCompareToEmptyOrDeleted because the HashSet will never contain null pointers or deleted values.
+struct DOMNamedFlowCollection::DOMNamedFlowHashFunctions {
+ static unsigned hash(PassRefPtr<WebKitNamedFlow> key) { return DefaultHash<String>::Hash::hash(key->name()); }
+ static bool equal(PassRefPtr<WebKitNamedFlow> a, PassRefPtr<WebKitNamedFlow> b) { return a->name() == b->name(); }
+ static const bool safeToCompareToEmptyOrDeleted = true;
+};
+
+// The HashTranslator is used to lookup a RefPtr<NamedFlow> in the set using a name.
+struct DOMNamedFlowCollection::DOMNamedFlowHashTranslator {
+ static unsigned hash(const String& key) { return DefaultHash<String>::Hash::hash(key); }
+ static bool equal(PassRefPtr<WebKitNamedFlow> a, const String& b) { return a->name() == b; }
+};
} // namespace WebCore
diff --git a/Source/WebCore/dom/DOMNamedFlowCollection.h b/Source/WebCore/dom/DOMNamedFlowCollection.h
index 02fb58c..62c8ca0 100644
--- a/Source/WebCore/dom/DOMNamedFlowCollection.h
+++ b/Source/WebCore/dom/DOMNamedFlowCollection.h
@@ -30,8 +30,10 @@
#define DOMNamedFlowCollection_h
#include "NamedFlowCollection.h"
+#include <wtf/ListHashSet.h>
#include <wtf/PassRefPtr.h>
#include <wtf/RefCounted.h>
+#include <wtf/Vector.h>
namespace WebCore {
@@ -40,9 +42,9 @@
class DOMNamedFlowCollection : public RefCounted<DOMNamedFlowCollection> {
public:
- static PassRefPtr<DOMNamedFlowCollection> create(NamedFlowCollection::NamedFlowSet& set)
+ static PassRefPtr<DOMNamedFlowCollection> create(const Vector<WebKitNamedFlow*>& namedFlows)
{
- return adoptRef(new DOMNamedFlowCollection(set));
+ return adoptRef(new DOMNamedFlowCollection(namedFlows));
}
unsigned long length() const;
@@ -52,8 +54,12 @@
bool hasNamedItem(const AtomicString& name) const;
private:
- explicit DOMNamedFlowCollection(NamedFlowCollection::NamedFlowSet&);
- NamedFlowCollection::NamedFlowSet m_namedFlows;
+ struct DOMNamedFlowHashFunctions;
+ struct DOMNamedFlowHashTranslator;
+
+ typedef ListHashSet<RefPtr<WebKitNamedFlow>, 1, DOMNamedFlowHashFunctions> DOMNamedFlowSet;
+ explicit DOMNamedFlowCollection(const Vector<WebKitNamedFlow*>&);
+ DOMNamedFlowSet m_namedFlows;
};
} // namespace WebCore
diff --git a/Source/WebCore/dom/NamedFlowCollection.cpp b/Source/WebCore/dom/NamedFlowCollection.cpp
index 5547506..4cbefe5 100644
--- a/Source/WebCore/dom/NamedFlowCollection.cpp
+++ b/Source/WebCore/dom/NamedFlowCollection.cpp
@@ -106,11 +106,25 @@
}
PassRefPtr<DOMNamedFlowCollection> NamedFlowCollection::createCSSOMSnapshot()
{
- NamedFlowSet createdFlows;
+ Vector<WebKitNamedFlow*> createdFlows;
for (NamedFlowSet::iterator it = m_namedFlows.begin(); it != m_namedFlows.end(); ++it)
if ((*it)->flowState() == WebKitNamedFlow::FlowStateCreated)
- createdFlows.add(*it);
+ createdFlows.append(*it);
return DOMNamedFlowCollection::create(createdFlows);
}
+// The HashFunctions object used by the HashSet to compare between NamedFlows.
+// It is safe to set safeToCompareToEmptyOrDeleted because the HashSet will never contain null pointers or deleted values.
+struct NamedFlowCollection::NamedFlowHashFunctions {
+ static unsigned hash(WebKitNamedFlow* key) { return DefaultHash<String>::Hash::hash(key->name()); }
+ static bool equal(WebKitNamedFlow* a, WebKitNamedFlow* b) { return a->name() == b->name(); }
+ static const bool safeToCompareToEmptyOrDeleted = true;
+};
+
+// The HashTranslator is used to lookup a NamedFlow in the set using a name.
+struct NamedFlowCollection::NamedFlowHashTranslator {
+ static unsigned hash(const String& key) { return DefaultHash<String>::Hash::hash(key); }
+ static bool equal(WebKitNamedFlow* a, const String& b) { return a->name() == b; }
+};
+
} // namespace WebCore
diff --git a/Source/WebCore/dom/NamedFlowCollection.h b/Source/WebCore/dom/NamedFlowCollection.h
index 175b5e7..f214d0b 100644
--- a/Source/WebCore/dom/NamedFlowCollection.h
+++ b/Source/WebCore/dom/NamedFlowCollection.h
@@ -60,32 +60,18 @@
PassRefPtr<DOMNamedFlowCollection> createCSSOMSnapshot();
+private:
struct NamedFlowHashFunctions;
struct NamedFlowHashTranslator;
typedef ListHashSet<WebKitNamedFlow*, 1, NamedFlowHashFunctions> NamedFlowSet;
-private:
-
explicit NamedFlowCollection(Document*);
Document* m_document;
NamedFlowSet m_namedFlows;
};
-// The HashFunctions object used by the HashSet to compare between NamedFlows.
-// It is safe to set safeToCompareToEmptyOrDeleted because the HashSet will never contain null pointers or deleted values.
-struct NamedFlowCollection::NamedFlowHashFunctions {
- static unsigned hash(WebKitNamedFlow* key) { return DefaultHash<String>::Hash::hash(key->name()); }
- static bool equal(WebKitNamedFlow* a, WebKitNamedFlow* b) { return a->name() == b->name(); }
- static const bool safeToCompareToEmptyOrDeleted = true;
-};
-
-// The HashTranslator is used to lookup a NamedFlow in the set using a name.
-struct NamedFlowCollection::NamedFlowHashTranslator {
- static unsigned hash(const String& key) { return DefaultHash<String>::Hash::hash(key); }
- static bool equal(WebKitNamedFlow* a, const String& b) { return a->name() == b; }
-};
} // namespace WebCore
#endif // NamedFlowCollection_h
