New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 673645 link

Starred by 1 user
Status: Fixed
Owner:
█ sigbjo...@opera.com
Email to this user bounced
Closed: Dec 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Make prefinalizer registration automatic

Reported by sigbjo...@opera.com, Dec 13 2016 
To deal with GCed objects that need to run finalization actions as part of being destructed, "prefinalizers" are used.

Class declaration for these will contain USING_PRE_FINALIZER(..) and the constructors are required to explicitly register the object's prefinalizer with its owning thread.

That last bit is manual and easy to forget; we should simplify the prefinalizer model and not require it, but make the registration happen implicitly by declaring USING_PRE_FINALIZER().
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 13 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4a50af8bd437d768140c896e644d4a3918c7f2d0

commit 4a50af8bd437d768140c896e644d4a3918c7f2d0
Author: sigbjornf <sigbjornf@opera.com>
Date: Tue Dec 13 07:08:12 2016

Avoid conditional Animation prefinalizers.

Recast the conditionally-eager finalization of Animation objects - only
needed if the Animation object has a CompositorAnimationPlayer attached -
wrapping instead the player object inside an eagerly-finalized object.

By doing so, we remove the need to support explicit prefinalizer
registration.

R=haraken
BUG= 673645 

Review-Url: https://codereview.chromium.org/2570503002
Cr-Commit-Position: refs/heads/master@{#438089}

[modify] https://crrev.com/4a50af8bd437d768140c896e644d4a3918c7f2d0/third_party/WebKit/Source/core/animation/Animation.cpp
[modify] https://crrev.com/4a50af8bd437d768140c896e644d4a3918c7f2d0/third_party/WebKit/Source/core/animation/Animation.h
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 13 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bc86f5fb6fa58808c4d7937b6279e89d977b6b69

commit bc86f5fb6fa58808c4d7937b6279e89d977b6b69
Author: sigbjornf <sigbjornf@opera.com>
Date: Tue Dec 13 09:28:54 2016

Implicit prefinalizer registration.

Switch to implicit registration of prefinalizers along with removing
the ability to dynamically unregister a prefinalizer; the latter
being an unused feature.

The requirement to manually register a prefinalizer has proven to be
a chore and a source of bugs. Case in point: HTMLCanvasElement
currently declares a prefinalizer, but doesn't register it. Simplify
the programming model by automatically registering prefinalizers.

R=haraken
BUG= 673645 

Review-Url: https://codereview.chromium.org/2565983002
Cr-Commit-Position: refs/heads/master@{#438110}

[modify] https://crrev.com/bc86f5fb6fa58808c4d7937b6279e89d977b6b69/third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp
[modify] https://crrev.com/bc86f5fb6fa58808c4d7937b6279e89d977b6b69/third_party/WebKit/Source/platform/heap/BlinkGCAPIReference.md
[modify] https://crrev.com/bc86f5fb6fa58808c4d7937b6279e89d977b6b69/third_party/WebKit/Source/platform/heap/HeapTest.cpp
[modify] https://crrev.com/bc86f5fb6fa58808c4d7937b6279e89d977b6b69/third_party/WebKit/Source/platform/heap/ThreadState.h
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 13 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9a49b977d6609b49e78e50f4b1efce71739c80ff

commit 9a49b977d6609b49e78e50f4b1efce71739c80ff
Author: sigbjornf <sigbjornf@opera.com>
Date: Tue Dec 13 12:07:14 2016

Retire ThreadState::registerPreFinalizer<T>()

The registration of the finalization callback now happens under-the-hood
and automatically.

R=haraken
BUG= 673645 

Review-Url: https://codereview.chromium.org/2570463005
Cr-Commit-Position: refs/heads/master@{#438136}

[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/animation/Animation.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/css/CSSCrossfadeValue.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/css/StyleRuleImport.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/dom/PendingScript.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/events/PromiseRejectionEvent.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/fetch/MockResourceClient.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/fetch/ResourceOwner.h
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/html/forms/ChooserOnlyTemporalInputTypeView.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/html/parser/HTMLParserScriptRunner.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/loader/ImageLoader.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/loader/resource/MockFontResourceClient.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/style/StyleFetchedImage.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/style/StyleFetchedImageSet.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/core/svg/SVGUseElement.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/broadcastchannel/BroadcastChannel.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/fetch/BlobBytesConsumer.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/fetch/FetchManager.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/mediastream/MediaDevices.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/nfc/NFC.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/notifications/NotificationResourcesLoader.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/peerconnection/RTCDTMFSender.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/peerconnection/RTCDataChannel.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerRegistration.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/vr/VRController.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/webaudio/AudioNode.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/webmidi/MIDIAccess.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/modules/webusb/USB.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/platform/heap/ThreadState.h
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/platform/mac/ScrollAnimatorMac.mm
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/platform/mediastream/MediaStreamComponent.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/platform/scroll/ScrollAnimatorCompositorCoordinator.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/web/ColorChooserPopupUIController.cpp
[modify] https://crrev.com/9a49b977d6609b49e78e50f4b1efce71739c80ff/third_party/WebKit/Source/web/WebPluginContainerImpl.cpp
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 13 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/375dffda9aeb0240d96aa44e25c49f2cedc5f22a

commit 375dffda9aeb0240d96aa44e25c49f2cedc5f22a
Author: sigbjornf <sigbjornf@opera.com>
Date: Tue Dec 13 15:14:27 2016

Remove PreFinalizer{Callback} type aliases from view.

Internal types, no good reason to expose these to the outside.

R=
BUG= 673645 

Review-Url: https://codereview.chromium.org/2573783002
Cr-Commit-Position: refs/heads/master@{#438170}

[modify] https://crrev.com/375dffda9aeb0240d96aa44e25c49f2cedc5f22a/third_party/WebKit/Source/platform/heap/BlinkGC.h
[modify] https://crrev.com/375dffda9aeb0240d96aa44e25c49f2cedc5f22a/third_party/WebKit/Source/platform/heap/ThreadState.h

Comment 5 by sigbjo...@opera.com, Dec 13 2016

Status: Fixed (was: Started)
►

Sign in to add a comment