Changeset 211238 in webkit


Ignore:
Timestamp:
Jan 26, 2017 4:11:54 PM (7 years ago)
Author:
fpizlo@apple.com
Message:

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:
Location:
trunk/Source/WebCore
Files:
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r211236 r211238  
     12017-01-26  Filip Pizlo  <fpizlo@apple.com>
     2
     3        EventTarget should visit the JSEventListeners using visitAdditionalChildren
     4        https://bugs.webkit.org/show_bug.cgi?id=167462
     5
     6        Reviewed by Michael Saboff.
     7
     8        No new tests because this is already caught by existing testing. This would show up as ASSERTs
     9        in debug, and we suspect it might be at fault for null deref crashes.
     10       
     11        Previously, EventTarget would have its event listeners visited by its subclasses' visitChildren
     12        methods. Every subclass of EventTarget would call EventTarget's visitJSEventListeners. For
     13        example, this means that if JSFoo has seven classes between it and JSEventTarget in the JSCell
     14        class hierarchy, then JSFoo::visitChildren would end up calling visitJSEventListeners seven extra
     15        times.
     16       
     17        Also, the weird way that visitJSEventListeners was called meant that it was not part of the GC's
     18        output constraint processing. This meant that it would not be called when the GC tried to
     19        terminate. So, if something about the event listener changes during a GC cycle, the GC would
     20        potentially fail to mark one of the references.
     21       
     22        Both problems can be solved by simply moving the call to visitJSEventListeners into
     23        visitAdditionalChildren.
     24
     25        * bindings/js/JSDOMWindowCustom.cpp:
     26        (WebCore::JSDOMWindow::visitAdditionalChildren):
     27        * bindings/js/JSEventTargetCustom.cpp:
     28        (WebCore::JSEventTarget::visitAdditionalChildren):
     29        * bindings/scripts/CodeGeneratorJS.pm:
     30        (GenerateImplementation):
     31        * dom/EventTarget.idl:
     32
    1332017-01-26  Andy Estes  <aestes@apple.com>
    234
  • trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp

    r211033 r211238  
    11/*
    2  * Copyright (C) 2007-2010, 2016 Apple Inc. All rights reserved.
     2 * Copyright (C) 2007-2017 Apple Inc. All rights reserved.
    33 * Copyright (C) 2011 Google Inc. All rights reserved.
    44 *
     
    5353    if (Frame* frame = wrapped().frame())
    5454        visitor.addOpaqueRoot(frame);
     55   
     56    // Normally JSEventTargetCustom.cpp's JSEventTarget::visitAdditionalChildren() would call this. But
     57    // even though DOMWindow is an EventTarget, JSDOMWindow does not subclass JSEventTarget, so we need
     58    // to do this here.
     59    wrapped().visitJSEventListeners(visitor);
    5560}
    5661
  • trunk/Source/WebCore/bindings/js/JSEventTargetCustom.cpp

    r208124 r211238  
    11/*
    2  * Copyright (C) 2008, 2016 Apple Inc. All Rights Reserved.
     2 * Copyright (C) 2008-2017 Apple Inc. All Rights Reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    8383}
    8484
     85void JSEventTarget::visitAdditionalChildren(SlotVisitor& visitor)
     86{
     87    wrapped().visitJSEventListeners(visitor);
     88}
     89
    8590} // namespace WebCore
  • trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp

    r210037 r211238  
    4343    ScriptExecutionContext& context = wrapped();
    4444    visitor.addOpaqueRoot(&context);
     45   
     46    // Normally JSEventTargetCustom.cpp's JSEventTarget::visitAdditionalChildren() would call this. But
     47    // even though WorkerGlobalScope is an EventTarget, JSWorkerGlobalScope does not subclass
     48    // JSEventTarget, so we need to do this here.
     49    wrapped().visitJSEventListeners(visitor);
    4550}
    4651
  • trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

    r211024 r211238  
    41594159        push(@implContent, "    ASSERT_GC_OBJECT_INHERITS(thisObject, info());\n");
    41604160        push(@implContent, "    Base::visitChildren(thisObject, visitor);\n");
    4161         if ($codeGenerator->InheritsInterface($interface, "EventTarget")) {
    4162             push(@implContent, "    thisObject->wrapped().visitJSEventListeners(visitor);\n");
    4163         }
    41644161        push(@implContent, "    thisObject->visitAdditionalChildren(visitor);\n") if $interface->extendedAttributes->{JSCustomMarkFunction};
    41654162        if ($interface->extendedAttributes->{ReportExtraMemoryCost}) {
  • trunk/Source/WebCore/dom/EventTarget.idl

    r209424 r211238  
    11/*
    2  * Copyright (C) 2006, 2007 Apple Inc. All rights reserved.
     2 * Copyright (C) 2006-2017 Apple Inc. All rights reserved.
    33 * Copyright (C) 2006 Samuel Weinig <sam.weinig@gmail.com>
    44 *
     
    2424    IsImmutablePrototypeExoticObjectOnPrototype,
    2525    JSCustomHeader,
     26    JSCustomMarkFunction,
    2627    JSCustomToNativeObject,
    2728] interface EventTarget {
Note: See TracChangeset for help on using the changeset viewer.