- Revision
- 128325
- Author
- commit-qu...@webkit.org
- Date
- 2012-09-12 09:30:04 -0700 (Wed, 12 Sep 2012)
Log Message
[CSSRegions]Use RefPtr's instead of weak references on DOMNamedFlowCollection
https://bugs.webkit.org/show_bug.cgi?id=95311
Patch by Andrei Onea <o...@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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (128324 => 128325)
--- trunk/LayoutTests/ChangeLog 2012-09-12 16:25:34 UTC (rev 128324)
+++ trunk/LayoutTests/ChangeLog 2012-09-12 16:30:04 UTC (rev 128325)
@@ -1,3 +1,16 @@
+2012-09-12 Andrei Onea <o...@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 <morr...@google.com>
[Shadow DOM] Unpolished elements should reject author shadows
Added: trunk/LayoutTests/fast/regions/webkit-named-flow-collection-crash-expected.txt (0 => 128325)
--- trunk/LayoutTests/fast/regions/webkit-named-flow-collection-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/regions/webkit-named-flow-collection-crash-expected.txt 2012-09-12 16:30:04 UTC (rev 128325)
@@ -0,0 +1,4 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS
Added: trunk/LayoutTests/fast/regions/webkit-named-flow-collection-crash.html (0 => 128325)
--- trunk/LayoutTests/fast/regions/webkit-named-flow-collection-crash.html (rev 0)
+++ trunk/LayoutTests/fast/regions/webkit-named-flow-collection-crash.html 2012-09-12 16:30:04 UTC (rev 128325)
@@ -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=""
+ <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=""
+ </body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (128324 => 128325)
--- trunk/Source/WebCore/ChangeLog 2012-09-12 16:25:34 UTC (rev 128324)
+++ trunk/Source/WebCore/ChangeLog 2012-09-12 16:30:04 UTC (rev 128325)
@@ -1,3 +1,45 @@
+2012-09-12 Andrei Onea <o...@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 <morr...@google.com>
[Shadow DOM] Unpolished elements should reject author shadows
Modified: trunk/Source/WebCore/dom/DOMNamedFlowCollection.cpp (128324 => 128325)
--- trunk/Source/WebCore/dom/DOMNamedFlowCollection.cpp 2012-09-12 16:25:34 UTC (rev 128324)
+++ trunk/Source/WebCore/dom/DOMNamedFlowCollection.cpp 2012-09-12 16:30:04 UTC (rev 128325)
@@ -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
Modified: trunk/Source/WebCore/dom/DOMNamedFlowCollection.h (128324 => 128325)
--- trunk/Source/WebCore/dom/DOMNamedFlowCollection.h 2012-09-12 16:25:34 UTC (rev 128324)
+++ trunk/Source/WebCore/dom/DOMNamedFlowCollection.h 2012-09-12 16:30:04 UTC (rev 128325)
@@ -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
Modified: trunk/Source/WebCore/dom/NamedFlowCollection.cpp (128324 => 128325)
--- trunk/Source/WebCore/dom/NamedFlowCollection.cpp 2012-09-12 16:25:34 UTC (rev 128324)
+++ trunk/Source/WebCore/dom/NamedFlowCollection.cpp 2012-09-12 16:30:04 UTC (rev 128325)
@@ -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
Modified: trunk/Source/WebCore/dom/NamedFlowCollection.h (128324 => 128325)
--- trunk/Source/WebCore/dom/NamedFlowCollection.h 2012-09-12 16:25:34 UTC (rev 128324)
+++ trunk/Source/WebCore/dom/NamedFlowCollection.h 2012-09-12 16:30:04 UTC (rev 128325)
@@ -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
