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

Issue 2536403002: blink_gc_plugin should require tracing for classes that have a trace method

Created:
4 years ago by keishi
Modified:
4 years ago
Reviewers:
haraken, oilpan-reviews, sof
CC:
chromium-reviews, blink-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

blink_gc_plugin should require tracing for classes that have a trace method blink_gc_plugin should require tracing for classes that have a trace method even if it doesn't have any fields that require tracing. EventTargetWithInlineData has GC_PLUGIN_IGNORE so it doesn't have fields that require tracing but it still needs to be traced. BUG=None

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -3 lines) Patch
M third_party/WebKit/Source/core/events/EventTarget.h View 2 chunks +2 lines, -2 lines 0 comments Download
M tools/clang/blink_gc_plugin/RecordInfo.cpp View 1 chunk +1 line, -1 line 0 comments Download
A tools/clang/blink_gc_plugin/tests/base_with_trace_requires_tracing.h View 1 chunk +35 lines, -0 lines 0 comments Download
A tools/clang/blink_gc_plugin/tests/base_with_trace_requires_tracing.cpp View 1 chunk +21 lines, -0 lines 0 comments Download
A tools/clang/blink_gc_plugin/tests/base_with_trace_requires_tracing.txt View 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (2 generated)
keishi
PTAL
4 years ago (2016-11-30 04:47:33 UTC) #2
haraken
LGTM
4 years ago (2016-11-30 04:48:46 UTC) #3
sof
4 years ago (2016-11-30 08:04:00 UTC) #5
lgtm, good thing to do now (as we're way past supporting classes which could be
compiled with & without Oilpan.)

Doesn't depend on separately rolling the GC plugin first, so fine to keep the
two changes together.

Powered by Google App Engine
This is Rietveld 408576698