Add use-chromium-style-naming option to Blink GC plugin.

Add use-chromium-style-naming option to Blink GC plugin.

The rewrite_to_chrome_style tool contains a list of methods
ignored for renaming, but it missed several Oilpan methods.
This flag allows the plugin to use either Blink-style naming
or Chromium-style naming, so the build doesn't break after
the giant Blink rename.


BUG=584538
R=haraken@chromium.org, sigbjornf@opera.com

Committed: https://chromium.googlesource.com/chromium/src/+/2de974ba92d8f924d01a6f9c7bb7ae7636ac9b4c

[dcheng, 2016-12-16 01:39:04]

Description was changed from

==========
Add use-chromium-style-naming option to Blink GC plugin.

BUG=584538
==========

to

==========
Add use-chromium-style-naming option to Blink GC plugin.

This allows the plugin to use either Blink-style naming or
Chromium-style renaming, so the build doesn't break after the
giant Blink rename.

Previously, the tool blacklisted a subset of special Oilpan
methods, but it seems better to just teach the plugin the new
naming scheme.

BUG=584538
==========

[dcheng, 2016-12-16 01:39:04]

dcheng@chromium.org changed reviewers:
+ haraken@chromium.org, sigbjornf@opera.com

[dcheng, 2016-12-16 01:40:23]

I'm not quite sure what the best way to ensure test coverage of this is. I guess
we could duplicate all the test files and add use-chromium-style-naming in the
duplicated tests? WDYT?

https://codereview.chromium.org/2578103003/diff/1/tools/clang/blink_gc_plugin...
File tools/clang/blink_gc_plugin/Config.h (right):

https://codereview.chromium.org/2578103003/diff/1/tools/clang/blink_gc_plugin...
tools/clang/blink_gc_plugin/Config.h:21: extern const char* kCreateName;
This is horrible, but this is just temporary code that we'll remove once we
default this option to on.

[haraken, 2016-12-16 01:45:06]

LGTM

On 2016/12/16 01:40:23, dcheng wrote:
> I'm not quite sure what the best way to ensure test coverage of this is. I
guess
> we could duplicate all the test files and add use-chromium-style-naming in the
> duplicated tests? WDYT?

Sounds good to me.

[sof, 2016-12-16 07:47:51]

On 2016/12/16 01:40:23, dcheng wrote:
> I'm not quite sure what the best way to ensure test coverage of this is. I
guess
> we could duplicate all the test files and add use-chromium-style-naming in the
> duplicated tests? WDYT?
> 

lgtm; case-insensitive matching of expected & actual outputs + transition name
macros ("traceWillBeTrace", or something similar) could be an alternative?

[sof, 2016-12-16 07:55:56]

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

https://codereview.chromium.org/2578103003/diff/1/tools/clang/blink_gc_plugin...
tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp:33: const
std::vector<std::string>& args) override {
Quirky formatting.

[dcheng, 2016-12-20 08:48:24]

Patchset #2 (id:20001) has been deleted

[dcheng, 2016-12-20 08:52:18]

PTAL. I choose to just create a copy of the entire tests directory, since it
will be easier to clean up post-rename.

https://codereview.chromium.org/2578103003/diff/40001/tools/clang/blink_gc_pl...
File
tools/clang/blink_gc_plugin/tests/legacy_naming/base_class_must_define_virtual_trace.txt
(right):

https://codereview.chromium.org/2578103003/diff/40001/tools/clang/blink_gc_pl...
tools/clang/blink_gc_plugin/tests/legacy_naming/base_class_must_define_virtual_trace.txt:8:
2 warnings generated.
Rietveld picked up most of these as moves, but some show as copies.

This directory is basically a straight copy of the old tests directory, with the
exception of minor relative path fixes in test.py.

[sof, 2016-12-20 13:51:54]

https://codereview.chromium.org/2578103003/diff/40001/tools/clang/blink_gc_pl...
File tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp (right):

https://codereview.chromium.org/2578103003/diff/40001/tools/clang/blink_gc_pl...
tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp:33: const
std::vector<std::string>& args) override {
misindented.

https://codereview.chromium.org/2578103003/diff/40001/tools/clang/blink_gc_pl...
File tools/clang/blink_gc_plugin/CMakeLists.txt (right):

https://codereview.chromium.org/2578103003/diff/40001/tools/clang/blink_gc_pl...
tools/clang/blink_gc_plugin/CMakeLists.txt:35: python
legacy_naming/tests/test.py
tests/legacy_naming/ ?

[dcheng, 2016-12-20 17:31:18]

https://codereview.chromium.org/2578103003/diff/40001/tools/clang/blink_gc_pl...
File tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp (right):

https://codereview.chromium.org/2578103003/diff/40001/tools/clang/blink_gc_pl...
tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp:33: const
std::vector<std::string>& args) override {
On 2016/12/20 13:51:54, sof wrote:
> misindented.

Done.

https://codereview.chromium.org/2578103003/diff/40001/tools/clang/blink_gc_pl...
File tools/clang/blink_gc_plugin/CMakeLists.txt (right):

https://codereview.chromium.org/2578103003/diff/40001/tools/clang/blink_gc_pl...
tools/clang/blink_gc_plugin/CMakeLists.txt:35: python
legacy_naming/tests/test.py
On 2016/12/20 13:51:54, sof wrote:
> tests/legacy_naming/ ?

Done.

[sof, 2016-12-20 18:52:45]

lgtm

I don't understand the last para in the description; what "tool" is it referring
to?

[dcheng, 2016-12-20 19:59:21]

Description was changed from

==========
Add use-chromium-style-naming option to Blink GC plugin.

This allows the plugin to use either Blink-style naming or
Chromium-style renaming, so the build doesn't break after the
giant Blink rename.

Previously, the tool blacklisted a subset of special Oilpan
methods, but it seems better to just teach the plugin the new
naming scheme.

BUG=584538
==========

to

==========
Add use-chromium-style-naming option to Blink GC plugin.

The rewrite_to_chrome_style tool contains a list of methods
ignored for renaming, but it missed several Oilpan methods.
This flag allows the plugin to use either Blink-style naming
or Chromium-style naming, so the build doesn't break after
the giant Blink rename.


BUG=584538
==========

[dcheng, 2016-12-20 19:59:21]

dcheng@chromium.org changed reviewers:
+ thakis@chromium.org

[dcheng, 2016-12-20 19:59:55]

updated the CL description to specifically reference the rewrite_to_chrome_style
tool

+thakis as FYI, since this is going to require a clang roll

[Nico, 2016-12-20 20:23:40]

On 2016/12/20 19:59:55, dcheng wrote:
> updated the CL description to specifically reference the
rewrite_to_chrome_style
> tool
> 
> +thakis as FYI, since this is going to require a clang roll

After this lands, sync your local checkout past the point where it landed. Then
run `tools/clang/scripts/upload_revision.py 289944 2` while on your master
branch. Let me know once the jobs started by this script have finished (`git cl
web`, should be done after 90 min or so). I'll then push the new binaries to
goma and will let you know once they're everywhere (will also take 90 min or
so). Then you can land the roll. (see also docs/updating_clang.md which is
roughly up to date, except that i think `--clang_revision` is false)

[commit-bot: I haz the power, 2016-12-20 22:20:13]

Description was changed from

==========
Add use-chromium-style-naming option to Blink GC plugin.

The rewrite_to_chrome_style tool contains a list of methods
ignored for renaming, but it missed several Oilpan methods.
This flag allows the plugin to use either Blink-style naming
or Chromium-style naming, so the build doesn't break after
the giant Blink rename.


BUG=584538
==========

to

==========
Add use-chromium-style-naming option to Blink GC plugin.

The rewrite_to_chrome_style tool contains a list of methods
ignored for renaming, but it missed several Oilpan methods.
This flag allows the plugin to use either Blink-style naming
or Chromium-style naming, so the build doesn't break after
the giant Blink rename.

BUG=584538
R=haraken@chromium.org, sigbjornf@opera.com

Committed: https://crrev.com/2de974ba92d8f924d01a6f9c7bb7ae7636ac9b4c
Cr-Commit-Position: refs/heads/master@{#439892}
==========

[commit-bot: I haz the power, 2016-12-20 22:20:15]

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

[dcheng, 2016-12-20 22:22:19]

Description was changed from

==========
Add use-chromium-style-naming option to Blink GC plugin.

The rewrite_to_chrome_style tool contains a list of methods
ignored for renaming, but it missed several Oilpan methods.
This flag allows the plugin to use either Blink-style naming
or Chromium-style naming, so the build doesn't break after
the giant Blink rename.

BUG=584538
R=haraken@chromium.org, sigbjornf@opera.com

Committed: https://crrev.com/2de974ba92d8f924d01a6f9c7bb7ae7636ac9b4c
Cr-Commit-Position: refs/heads/master@{#439892}
==========

to

==========
Add use-chromium-style-naming option to Blink GC plugin.

The rewrite_to_chrome_style tool contains a list of methods
ignored for renaming, but it missed several Oilpan methods.
This flag allows the plugin to use either Blink-style naming
or Chromium-style naming, so the build doesn't break after
the giant Blink rename.


BUG=584538
R=haraken@chromium.org, sigbjornf@opera.com

Committed:
https://chromium.googlesource.com/chromium/src/+/2de974ba92d8f924d01a6f9c7bb7...
==========

[dcheng, 2016-12-20 22:22:21]

Committed patchset #4 (id:80001) manually as
2de974ba92d8f924d01a6f9c7bb7ae7636ac9b4c (presubmit successful).