Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(93)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 329853004: Remove RefCountedGarbageCollected from EventTarget objects (Closed)

Created:
6 years, 6 months ago by haraken
Modified:
6 years, 6 months ago
CC:
blink-reviews, jsbell+idb_chromium.org, ericu+idb_chromium.org, tzik, blink-reviews-css, philipj_slow, alecflett, ed+blinkwatch_opera.com, eric.carlson_apple.com, feature-media-reviews_chromium.org, dglazkov+blink, apavlov+blink_chromium.org, nhiroki, cmumford, darktears, dgrogan, rune+blink, kinuko+fileapi, rwlbuis
Visibility:
Public.

Description

Remove RefCountedGarbageCollected from EventTarget objects Now that EventTarget is on-heap, we don't need to use RefCountedGarbageCollected for classes that derive EventTargets. Another reason why these classes have to be RefCountedGarbageCollected is that they derive ActiveDOMObject, and ActiveDOMObject::setPendingActivity calls ref(). However, they don't need to be RefCountedGarbageCollected unless they actually use setPendingActivity(), so this CL just removes RefCountedGarbageCollected from classes that derive ActiveDOMObject but don't use setPendingActivity(). BUG=340522 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176011

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Total comments: 2

Patch Set 3 : #

Patch Set 4 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -29 lines) Patch
M Source/core/css/FontFaceSet.h View 2 3 2 chunks +2 lines, -3 lines 0 comments Download
M Source/modules/encryptedmedia/MediaKeySession.h View 1 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/encryptedmedia/MediaKeySession.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/filesystem/FileWriterBase.h View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/modules/indexeddb/IDBDatabase.h View 1 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/indexeddb/IDBDatabase.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/indexeddb/IDBOpenDBRequest.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/indexeddb/IDBRequest.h View 1 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/indexeddb/IDBRequest.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/indexeddb/IDBTransaction.h View 1 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/indexeddb/IDBTransaction.cpp View 1 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/notifications/Notification.h View 1 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/notifications/Notification.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/speech/SpeechRecognition.h View 1 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/speech/SpeechRecognition.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/speech/SpeechSynthesis.h View 1 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/speech/SpeechSynthesis.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download
M Source/modules/speech/SpeechSynthesisUtterance.h View 1 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/speech/SpeechSynthesisUtterance.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/platform/heap/Handle.h View 1 4 chunks +19 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
haraken
PTAL
6 years, 6 months ago (2014-06-12 02:10:08 UTC) #1
tkent
https://codereview.chromium.org/329853004/diff/1/Source/modules/encryptedmedia/MediaKeySession.h File Source/modules/encryptedmedia/MediaKeySession.h (right): https://codereview.chromium.org/329853004/diff/1/Source/modules/encryptedmedia/MediaKeySession.h#newcode67 Source/modules/encryptedmedia/MediaKeySession.h:67: : public GarbageCollectedFinalized<MediaKeySession>, public ActiveDOMObject, public ScriptWrappable, public EventTargetWithInlineData ...
6 years, 6 months ago (2014-06-12 02:21:57 UTC) #2
haraken
https://codereview.chromium.org/329853004/diff/1/Source/modules/encryptedmedia/MediaKeySession.h File Source/modules/encryptedmedia/MediaKeySession.h (right): https://codereview.chromium.org/329853004/diff/1/Source/modules/encryptedmedia/MediaKeySession.h#newcode67 Source/modules/encryptedmedia/MediaKeySession.h:67: : public GarbageCollectedFinalized<MediaKeySession>, public ActiveDOMObject, public ScriptWrappable, public EventTargetWithInlineData ...
6 years, 6 months ago (2014-06-12 02:26:27 UTC) #3
haraken
Updated the patch set. I confirmed that it builds with and without oilpan. PTAL.
6 years, 6 months ago (2014-06-12 04:49:42 UTC) #4
tkent
lgtm https://codereview.chromium.org/329853004/diff/20001/Source/core/css/FontFaceSet.h File Source/core/css/FontFaceSet.h (right): https://codereview.chromium.org/329853004/diff/20001/Source/core/css/FontFaceSet.h#newcode118 Source/core/css/FontFaceSet.h:118: return new FontFaceSet(document); Use adoptRefWillBeNoop
6 years, 6 months ago (2014-06-12 05:04:55 UTC) #5
haraken
Thanks for review! https://codereview.chromium.org/329853004/diff/20001/Source/core/css/FontFaceSet.h File Source/core/css/FontFaceSet.h (right): https://codereview.chromium.org/329853004/diff/20001/Source/core/css/FontFaceSet.h#newcode118 Source/core/css/FontFaceSet.h:118: return new FontFaceSet(document); On 2014/06/12 05:04:55, ...
6 years, 6 months ago (2014-06-12 05:59:16 UTC) #6
haraken
The CQ bit was checked by haraken@chromium.org
6 years, 6 months ago (2014-06-12 05:59:21 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/haraken@chromium.org/329853004/60001
6 years, 6 months ago (2014-06-12 06:00:32 UTC) #8
Mads Ager (chromium)
Cool! LGTM
6 years, 6 months ago (2014-06-12 06:52:13 UTC) #9
zerny-chromium
Great observation! LGTM2
6 years, 6 months ago (2014-06-12 07:21:05 UTC) #10
wibling-chromium
lgtm
6 years, 6 months ago (2014-06-12 07:33:56 UTC) #11
commit-bot: I haz the power
6 years, 6 months ago (2014-06-12 08:46:58 UTC) #12
Message was sent while issue was closed. 
Change committed as 176011

Powered by Google App Engine
This is Rietveld 408576698