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

Issue 290333008: Oilpan: add [WillBeGarbageCollected] for Element. (Closed)

Created:
6 years, 7 months ago by sof
Modified:
6 years, 7 months ago
CC:
blink-reviews, blink-reviews-html_chromium.org, arv+blink, eae+blinkwatch, abarth-chromium, blink-reviews-dom_chromium.org, dglazkov+blink, dominicc+watchlist_chromium.org, blink-reviews-bindings_chromium.org, Inactive, watchdog-blink-watchlist_google.com, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Oilpan: add [WillBeGarbageCollected] for Element. Annotate Element as being a heap allocated object with Oilpan enabled, and generate binding layer code which assumes so for Element and its derived interface objects. To accommodate this, bindings code generated for a named constructor need to consult the "gc_type" of its interface, and no longer not assume RefCounted. Along similar lines, custom element processing can no longer assume that the elements will be RefCounted-based either. R=haraken@chromium.org,tkent@chromium.org BUG=357163 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=174537

Patch Set 1 #

Total comments: 2

Patch Set 2 : Rebased + remove WebNode FIXME #

Unified diffs Side-by-side diffs Delta from patch set Stats (+89 lines, -38 lines) Patch
M Source/bindings/templates/interface.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/tests/idls/TestInterfaceWillBeGarbageCollected.idl View 1 chunk +1 line, -0 lines 0 comments Download
M Source/bindings/tests/results/V8TestInterfaceWillBeGarbageCollected.h View 1 chunk +6 lines, -0 lines 0 comments Download
M Source/bindings/tests/results/V8TestInterfaceWillBeGarbageCollected.cpp View 1 chunk +52 lines, -0 lines 0 comments Download
M Source/bindings/v8/CustomElementWrapper.h View 2 chunks +2 lines, -1 line 0 comments Download
M Source/bindings/v8/CustomElementWrapper.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/v8/custom/V8HTMLAllCollectionCustom.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/dom/ContainerNode.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/dom/ContainerNode.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/dom/Element.cpp View 1 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/dom/Element.idl View 1 1 chunk +3 lines, -1 line 0 comments Download
M Source/core/dom/ParentNode.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/dom/SelectorQuery.h View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/dom/SelectorQuery.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/html/HTMLAllCollection.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/HTMLAllCollection.cpp View 2 chunks +2 lines, -3 lines 0 comments Download
M Source/core/html/HTMLFormControlsCollection.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/HTMLFormControlsCollection.cpp View 2 chunks +2 lines, -3 lines 0 comments Download
M Source/core/html/HTMLFormElement.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/HTMLFormElement.cpp View 2 chunks +2 lines, -3 lines 0 comments Download
M Source/core/html/HTMLOptionsCollection.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/HTMLOptionsCollection.cpp View 2 chunks +2 lines, -3 lines 0 comments Download
M Source/web/WebNode.cpp View 1 1 chunk +1 line, -8 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
sof
Please take a look.
6 years, 7 months ago (2014-05-21 16:19:20 UTC) #1
haraken
LGTM https://codereview.chromium.org/290333008/diff/1/Source/core/dom/Element.idl File Source/core/dom/Element.idl (right): https://codereview.chromium.org/290333008/diff/1/Source/core/dom/Element.idl#newcode23 Source/core/dom/Element.idl:23: WillBeGarbageCollected, Add a FIXME to remove this once ...
6 years, 7 months ago (2014-05-21 17:56:28 UTC) #2
tkent
lgtm. We can remove a FIXME in WebNode::querySelector.
6 years, 7 months ago (2014-05-21 17:59:33 UTC) #3
sof
On 2014/05/21 17:59:33, tkent wrote: > lgtm. > We can remove a FIXME in WebNode::querySelector. ...
6 years, 7 months ago (2014-05-21 21:45:04 UTC) #4
sof
https://codereview.chromium.org/290333008/diff/1/Source/core/dom/Element.idl File Source/core/dom/Element.idl (right): https://codereview.chromium.org/290333008/diff/1/Source/core/dom/Element.idl#newcode23 Source/core/dom/Element.idl:23: WillBeGarbageCollected, On 2014/05/21 17:56:28, haraken wrote: > > Add ...
6 years, 7 months ago (2014-05-21 21:45:14 UTC) #5
tkent
lgtm
6 years, 7 months ago (2014-05-21 23:43:15 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sigbjornf@opera.com/290333008/20001
6 years, 7 months ago (2014-05-21 23:43:58 UTC) #7
Mads Ager (chromium)
LGTM! Thanks for doing these cleanups so quickly!
6 years, 7 months ago (2014-05-22 06:45:14 UTC) #8
commit-bot: I haz the power
6 years, 7 months ago (2014-05-22 09:07:53 UTC) #9
Message was sent while issue was closed.
Change committed as 174537

Powered by Google App Engine
This is Rietveld 408576698