Title: [144381] trunk/Source/WebCore
- Revision
- 144381
- Author
- [email protected]
- Date
- 2013-02-28 15:25:38 -0800 (Thu, 28 Feb 2013)
Log Message
[V8] Remove the world->isMainWorld() check from minorGCPrologue()
https://bugs.webkit.org/show_bug.cgi?id=111114
Reviewed by Adam Barth.
A couple of weeks ago, I introduced the following check to minorGCPrologue() in r142419.
void minorGCPrologue() {
// A minor GC can handle the main world only.
DOMWrapperWorld* world = worldForEnteredContextWithoutContextCheck();
if (world && world->isMainWorld()) {
MinorGCWrapperVisitor visitor(isolate);
v8::V8::VisitHandlesForPartialDependence(isolate, &visitor);
visitor.notifyFinished();
}
}
- The check makes no sense. A GC should not care about what world we are in.
There is no concept of worlds in GC.
- worldForEnteredContextWithoutContextCheck() returns 0 for the main world.
So if a GC runs in the main world, the minor DOM GC is skipped.
- worldForEnteredContextWithoutContextCheck() caused a Chromium crash
(https://code.google.com/p/chromium/issues/detail?id=177587)
We should remove the check.
No tests. No change in behavior.
* bindings/v8/DOMWrapperWorld.h:
(WebCore::DOMWrapperWorld::getWorld):
* bindings/v8/V8Binding.h:
* bindings/v8/V8GCController.cpp:
(WebCore::V8GCController::minorGCPrologue):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (144380 => 144381)
--- trunk/Source/WebCore/ChangeLog 2013-02-28 23:24:59 UTC (rev 144380)
+++ trunk/Source/WebCore/ChangeLog 2013-02-28 23:25:38 UTC (rev 144381)
@@ -1,3 +1,41 @@
+2013-02-28 Kentaro Hara <[email protected]>
+
+ [V8] Remove the world->isMainWorld() check from minorGCPrologue()
+ https://bugs.webkit.org/show_bug.cgi?id=111114
+
+ Reviewed by Adam Barth.
+
+ A couple of weeks ago, I introduced the following check to minorGCPrologue() in r142419.
+
+ void minorGCPrologue() {
+ // A minor GC can handle the main world only.
+ DOMWrapperWorld* world = worldForEnteredContextWithoutContextCheck();
+ if (world && world->isMainWorld()) {
+ MinorGCWrapperVisitor visitor(isolate);
+ v8::V8::VisitHandlesForPartialDependence(isolate, &visitor);
+ visitor.notifyFinished();
+ }
+ }
+
+ - The check makes no sense. A GC should not care about what world we are in.
+ There is no concept of worlds in GC.
+
+ - worldForEnteredContextWithoutContextCheck() returns 0 for the main world.
+ So if a GC runs in the main world, the minor DOM GC is skipped.
+
+ - worldForEnteredContextWithoutContextCheck() caused a Chromium crash
+ (https://code.google.com/p/chromium/issues/detail?id=177587)
+
+ We should remove the check.
+
+ No tests. No change in behavior.
+
+ * bindings/v8/DOMWrapperWorld.h:
+ (WebCore::DOMWrapperWorld::getWorld):
+ * bindings/v8/V8Binding.h:
+ * bindings/v8/V8GCController.cpp:
+ (WebCore::V8GCController::minorGCPrologue):
+
2013-02-28 Bruno de Oliveira Abinader <[email protected]>
Create GraphicsContext3DState to aggregate state objects
Modified: trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.h (144380 => 144381)
--- trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.h 2013-02-28 23:24:59 UTC (rev 144380)
+++ trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.h 2013-02-28 23:25:38 UTC (rev 144381)
@@ -67,10 +67,6 @@
assertContextHasCorrectPrototype(context);
return static_cast<DOMWrapperWorld*>(context->GetAlignedPointerFromEmbedderData(v8ContextIsolatedWorld));
}
- static DOMWrapperWorld* getWorldWithoutContextCheck(v8::Handle<v8::Context> context)
- {
- return static_cast<DOMWrapperWorld*>(context->GetAlignedPointerFromEmbedderData(v8ContextIsolatedWorld));
- }
// Associates an isolated world (see above for description) with a security
// origin. XMLHttpRequest instances used in that world will be considered
Modified: trunk/Source/WebCore/bindings/v8/V8Binding.h (144380 => 144381)
--- trunk/Source/WebCore/bindings/v8/V8Binding.h 2013-02-28 23:24:59 UTC (rev 144380)
+++ trunk/Source/WebCore/bindings/v8/V8Binding.h 2013-02-28 23:25:38 UTC (rev 144381)
@@ -458,22 +458,6 @@
return DOMWrapperWorld::getWorld(context);
}
- // This is a slightly different version of worldForEnteredContext().
- // The difference is just that worldForEnteredContextWithoutContextCheck()
- // does not call assertContextHasCorrectPrototype() (which is enabled on
- // Debug builds only). Because assertContextHasCorrectPrototype() crashes
- // if it is called when a current context is not completely initialized,
- // you have to use worldForEnteredContextWithoutContextCheck() if you need
- // to get a DOMWrapperWorld while a current context is being initialized.
- // See https://bugs.webkit.org/show_bug.cgi?id=108579#c15 for more details.
- inline DOMWrapperWorld* worldForEnteredContextWithoutContextCheck()
- {
- v8::Handle<v8::Context> context = v8::Context::GetEntered();
- if (context.IsEmpty())
- return 0;
- return DOMWrapperWorld::getWorldWithoutContextCheck(context);
- }
-
// If the current context causes out of memory, _javascript_ setting
// is disabled and it returns true.
bool handleOutOfMemory();
Modified: trunk/Source/WebCore/bindings/v8/V8GCController.cpp (144380 => 144381)
--- trunk/Source/WebCore/bindings/v8/V8GCController.cpp 2013-02-28 23:24:59 UTC (rev 144380)
+++ trunk/Source/WebCore/bindings/v8/V8GCController.cpp 2013-02-28 23:25:38 UTC (rev 144381)
@@ -258,8 +258,13 @@
ASSERT(V8DOMWrapper::maybeDOMWrapper(value));
ASSERT(V8Node::HasInstance(wrapper, m_isolate));
Node* node = V8Node::toNative(wrapper);
- m_nodesInNewSpace.append(node);
- node->setV8CollectableDuringMinorGC(true);
+ // A minor DOM GC can handle only node wrappers in the main world.
+ // Note that node->wrapper().IsEmpty() returns true for nodes that
+ // do not have wrappers in the main world.
+ if (!node->wrapper().IsEmpty()) {
+ m_nodesInNewSpace.append(node);
+ node->setV8CollectableDuringMinorGC(true);
+ }
}
void notifyFinished()
@@ -364,13 +369,9 @@
v8::Isolate* isolate = v8::Isolate::GetCurrent();
v8::HandleScope scope;
- // A minor GC can handle the main world only.
- DOMWrapperWorld* world = worldForEnteredContextWithoutContextCheck();
- if (world && world->isMainWorld()) {
- MinorGCWrapperVisitor visitor(isolate);
- v8::V8::VisitHandlesForPartialDependence(isolate, &visitor);
- visitor.notifyFinished();
- }
+ MinorGCWrapperVisitor visitor(isolate);
+ v8::V8::VisitHandlesForPartialDependence(isolate, &visitor);
+ visitor.notifyFinished();
}
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
