BlinkGCPlugin: Add support for traceAfterDispatchImpl().

BlinkGCPlugin: Add support for traceAfterDispatchImpl().

This patch adds support for trace call validation in traceAfterDispatchImpl()
which can be called from traceAfterDispatch().

The changes in this patch should be mostly straightforward except for the
introduction of Config::GetTraceMethodType(). The motivation to this change is
based on the fact that we now have four different types of trace functions.
The function is extended from the former Config::IsTraceMethod() so it can
return the type of the trace method as defined in TraceMethodType. The new
Config::IsTraceMethod() is implemented in terms of it.

This patch comes with tests that check traceAfterDispatchImpl() use cases (both
valid and invalid), and some obvious refactorings to follow Chromium coding
style.

BUG=none
R=kouhei@chromium.org
CC=zerny@chromium.org, oilpan-reviews@chromium.org

Committed: https://crrev.com/1d8ad73cca9d20a5e45821edd30c22bfa105840e
Cr-Commit-Position: refs/heads/master@{#317541}

[kouhei (in TOK), 2015-02-20 08:07:21]

Reviewing from 新幹線

https://codereview.chromium.org/940263004/diff/1/tools/clang/blink_gc_plugin/...
File tools/clang/blink_gc_plugin/RecordInfo.cpp (right):

https://codereview.chromium.org/940263004/diff/1/tools/clang/blink_gc_plugin/...
tools/clang/blink_gc_plugin/RecordInfo.cpp:438: if (it->getNameAsString() ==
kFinalizeName) {
Should we also identify those in Config::GetTraceMethodType?
It seems a bit odd to have the comparisons here and there.

[Yuta Kitamura, 2015-02-20 09:40:27]

https://codereview.chromium.org/940263004/diff/1/tools/clang/blink_gc_plugin/...
File tools/clang/blink_gc_plugin/RecordInfo.cpp (right):

https://codereview.chromium.org/940263004/diff/1/tools/clang/blink_gc_plugin/...
tools/clang/blink_gc_plugin/RecordInfo.cpp:438: if (it->getNameAsString() ==
kFinalizeName) {
On 2015/02/20 08:07:21, kouhei wrote:
> Should we also identify those in Config::GetTraceMethodType?
> It seems a bit odd to have the comparisons here and there.

I somewhat agree that it's silly to sprinkle direct name comparisons
over the code base, but it's less attracting for me to do abstraction
over kFinalizeName and others, because:

* They are just one-time constants used only here (unlike trace
  family); and
* There's one difference between trace method family and these:
  Config::GetTraceMethodType() (former IsTraceMethod()) checks the
  type of the arguments i.e. Visitor* or VisitorDispatcher, while
  the argument types of finalizeGarbageCollectedObject
  (kFinalizeName) and others are unchecked.

Maybe I should put this case section in the end of this switch block?
That might be more readable.

[kouhei (in TOK), 2015-02-20 10:28:43]

On 2015/02/20 09:40:27, Yuta Kitamura wrote:
>
https://codereview.chromium.org/940263004/diff/1/tools/clang/blink_gc_plugin/...
> File tools/clang/blink_gc_plugin/RecordInfo.cpp (right):
> 
>
https://codereview.chromium.org/940263004/diff/1/tools/clang/blink_gc_plugin/...
> tools/clang/blink_gc_plugin/RecordInfo.cpp:438: if (it->getNameAsString() ==
> kFinalizeName) {
> On 2015/02/20 08:07:21, kouhei wrote:
> > Should we also identify those in Config::GetTraceMethodType?
> > It seems a bit odd to have the comparisons here and there.
> 
> I somewhat agree that it's silly to sprinkle direct name comparisons
> over the code base, but it's less attracting for me to do abstraction
> over kFinalizeName and others, because:
> 
> * They are just one-time constants used only here (unlike trace
>   family); and
> * There's one difference between trace method family and these:
>   Config::GetTraceMethodType() (former IsTraceMethod()) checks the
>   type of the arguments i.e. Visitor* or VisitorDispatcher, while
>   the argument types of finalizeGarbageCollectedObject
>   (kFinalizeName) and others are unchecked.
> 
> Maybe I should put this case section in the end of this switch block?
> That might be more readable.

Thanks for the explanation. No strong opinion here. ps1 lgtm

[Yuta Kitamura, 2015-02-23 03:35:15]

New patchsets have been uploaded after l-g-t-m from kouhei@chromium.org

[Yuta Kitamura, 2015-02-23 03:37:48]

Thanks, I have applied the adjustment of "case" tags order
I mentioned in my comment, and will cq it.

[Yuta Kitamura, 2015-02-23 03:37:53]

The CQ bit was checked by yutak@chromium.org

[commit-bot: I haz the power, 2015-02-23 03:38:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/940263004/20001

[commit-bot: I haz the power, 2015-02-23 03:41:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-23 03:41:06]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[Yuta Kitamura, 2015-02-23 04:06:19]

The CQ bit was checked by yutak@chromium.org

[commit-bot: I haz the power, 2015-02-23 04:07:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/940263004/20001

[commit-bot: I haz the power, 2015-02-23 04:17:34]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-23 04:18:24]

Patchset 2 (id:??) landed as
https://crrev.com/1d8ad73cca9d20a5e45821edd30c22bfa105840e
Cr-Commit-Position: refs/heads/master@{#317541}