Diff
Modified: branches/safari-603-branch/Source/WebCore/ChangeLog (211258 => 211259)
--- branches/safari-603-branch/Source/WebCore/ChangeLog 2017-01-27 08:01:45 UTC (rev 211258)
+++ branches/safari-603-branch/Source/WebCore/ChangeLog 2017-01-27 08:01:48 UTC (rev 211259)
@@ -1,3 +1,39 @@
+2017-01-26 Matthew Hanson <[email protected]>
+
+ Merge r211238. rdar://problem/30216742
+
+ 2017-01-26 Filip Pizlo <[email protected]>
+
+ EventTarget should visit the JSEventListeners using visitAdditionalChildren
+ https://bugs.webkit.org/show_bug.cgi?id=167462
+
+ Reviewed by Michael Saboff.
+
+ No new tests because this is already caught by existing testing. This would show up as ASSERTs
+ in debug, and we suspect it might be at fault for null deref crashes.
+
+ Previously, EventTarget would have its event listeners visited by its subclasses' visitChildren
+ methods. Every subclass of EventTarget would call EventTarget's visitJSEventListeners. For
+ example, this means that if JSFoo has seven classes between it and JSEventTarget in the JSCell
+ class hierarchy, then JSFoo::visitChildren would end up calling visitJSEventListeners seven extra
+ times.
+
+ Also, the weird way that visitJSEventListeners was called meant that it was not part of the GC's
+ output constraint processing. This meant that it would not be called when the GC tried to
+ terminate. So, if something about the event listener changes during a GC cycle, the GC would
+ potentially fail to mark one of the references.
+
+ Both problems can be solved by simply moving the call to visitJSEventListeners into
+ visitAdditionalChildren.
+
+ * bindings/js/JSDOMWindowCustom.cpp:
+ (WebCore::JSDOMWindow::visitAdditionalChildren):
+ * bindings/js/JSEventTargetCustom.cpp:
+ (WebCore::JSEventTarget::visitAdditionalChildren):
+ * bindings/scripts/CodeGeneratorJS.pm:
+ (GenerateImplementation):
+ * dom/EventTarget.idl:
+
2017-01-26 Andreas Kling <[email protected]>
Branch-specific fix for a crash seen after merging r201777.
Modified: branches/safari-603-branch/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp (211258 => 211259)
--- branches/safari-603-branch/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2017-01-27 08:01:45 UTC (rev 211258)
+++ branches/safari-603-branch/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2017-01-27 08:01:48 UTC (rev 211259)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007-2010, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2007-2017 Apple Inc. All rights reserved.
* Copyright (C) 2011 Google Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
@@ -53,6 +53,11 @@
{
if (Frame* frame = wrapped().frame())
visitor.addOpaqueRoot(frame);
+
+ // Normally JSEventTargetCustom.cpp's JSEventTarget::visitAdditionalChildren() would call this. But
+ // even though DOMWindow is an EventTarget, JSDOMWindow does not subclass JSEventTarget, so we need
+ // to do this here.
+ wrapped().visitJSEventListeners(visitor);
}
#if ENABLE(USER_MESSAGE_HANDLERS)
Modified: branches/safari-603-branch/Source/WebCore/bindings/js/JSEventTargetCustom.cpp (211258 => 211259)
--- branches/safari-603-branch/Source/WebCore/bindings/js/JSEventTargetCustom.cpp 2017-01-27 08:01:45 UTC (rev 211258)
+++ branches/safari-603-branch/Source/WebCore/bindings/js/JSEventTargetCustom.cpp 2017-01-27 08:01:48 UTC (rev 211259)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2008, 2016 Apple Inc. All Rights Reserved.
+ * Copyright (C) 2008-2017 Apple Inc. All Rights Reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -82,4 +82,9 @@
return nullptr;
}
+void JSEventTarget::visitAdditionalChildren(SlotVisitor& visitor)
+{
+ wrapped().visitJSEventListeners(visitor);
+}
+
} // namespace WebCore
Modified: branches/safari-603-branch/Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp (211258 => 211259)
--- branches/safari-603-branch/Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp 2017-01-27 08:01:45 UTC (rev 211258)
+++ branches/safari-603-branch/Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp 2017-01-27 08:01:48 UTC (rev 211259)
@@ -42,6 +42,11 @@
visitor.addOpaqueRoot(navigator);
ScriptExecutionContext& context = wrapped();
visitor.addOpaqueRoot(&context);
+
+ // Normally JSEventTargetCustom.cpp's JSEventTarget::visitAdditionalChildren() would call this. But
+ // even though WorkerGlobalScope is an EventTarget, JSWorkerGlobalScope does not subclass
+ // JSEventTarget, so we need to do this here.
+ wrapped().visitJSEventListeners(visitor);
}
JSValue JSWorkerGlobalScope::importScripts(ExecState& state)
Modified: branches/safari-603-branch/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (211258 => 211259)
--- branches/safari-603-branch/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2017-01-27 08:01:45 UTC (rev 211258)
+++ branches/safari-603-branch/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2017-01-27 08:01:48 UTC (rev 211259)
@@ -3982,9 +3982,6 @@
push(@implContent, " auto* thisObject = jsCast<${className}*>(cell);\n");
push(@implContent, " ASSERT_GC_OBJECT_INHERITS(thisObject, info());\n");
push(@implContent, " Base::visitChildren(thisObject, visitor);\n");
- if ($codeGenerator->InheritsInterface($interface, "EventTarget")) {
- push(@implContent, " thisObject->wrapped().visitJSEventListeners(visitor);\n");
- }
push(@implContent, " thisObject->visitAdditionalChildren(visitor);\n") if $interface->extendedAttributes->{JSCustomMarkFunction};
if ($interface->extendedAttributes->{ReportExtraMemoryCost}) {
push(@implContent, " visitor.reportExtraMemoryVisited(thisObject->wrapped().memoryCost());\n");
Modified: branches/safari-603-branch/Source/WebCore/dom/EventTarget.idl (211258 => 211259)
--- branches/safari-603-branch/Source/WebCore/dom/EventTarget.idl 2017-01-27 08:01:45 UTC (rev 211258)
+++ branches/safari-603-branch/Source/WebCore/dom/EventTarget.idl 2017-01-27 08:01:48 UTC (rev 211259)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2006, 2007 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2017 Apple Inc. All rights reserved.
* Copyright (C) 2006 Samuel Weinig <[email protected]>
*
* This library is free software; you can redistribute it and/or
@@ -23,6 +23,7 @@
Exposed=(Window,Worker),
IsImmutablePrototypeExoticObjectOnPrototype,
JSCustomHeader,
+ JSCustomMarkFunction,
JSCustomToNativeObject,
] interface EventTarget {
[ImplementedAs=addEventListenerForBindings] void addEventListener([AtomicString] DOMString type, EventListener? callback, optional (AddEventListenerOptions or boolean) options = false);
