New issue
Advanced search Search tips

Issue 641004 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 578344



Sign in to add a comment

rewrite_to_chrome_style: Both |static int s_field| and |int m_field| are rewritten to |field_|

Project Member Reported by lukasza@chromium.org, Aug 25 2016

Issue description

Example:

third_party/WebKit/Source/platform/fonts/FontCache.h:

 72: class PLATFORM_EXPORT FontCache {
...
191:    RefPtr<SkFontMgr> m_fontManager;
192:
193:    static SkFontMgr* s_fontManager;
 

Comment 1 by danakj@chromium.org, Aug 25 2016

lol, nice. we should just name one of these different probably. are there any other cases of this?
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/78115b26dfafd0fca676ac33caabd3aa8b43ecf4

commit 78115b26dfafd0fca676ac33caabd3aa8b43ecf4
Author: lukasza <lukasza@chromium.org>
Date: Tue Aug 30 03:48:48 2016

s/s_fontManager/s_staticFontManager/ to help rewrite_to_chrome_style tool.

This CL helps rewrite_to_chrome_style tool, by preventing the tool for
suggesting the following conflicting renames:
- |s_field| -> |field_|
- |m_field| -> |field_|

BUG= 641004 

Review-Url: https://codereview.chromium.org/2294433002
Cr-Commit-Position: refs/heads/master@{#415058}

[modify] https://crrev.com/78115b26dfafd0fca676ac33caabd3aa8b43ecf4/third_party/WebKit/Source/platform/fonts/FontCache.cpp
[modify] https://crrev.com/78115b26dfafd0fca676ac33caabd3aa8b43ecf4/third_party/WebKit/Source/platform/fonts/FontCache.h
[modify] https://crrev.com/78115b26dfafd0fca676ac33caabd3aa8b43ecf4/third_party/WebKit/Source/platform/fonts/linux/FontCacheLinux.cpp
[modify] https://crrev.com/78115b26dfafd0fca676ac33caabd3aa8b43ecf4/third_party/WebKit/Source/platform/fonts/win/FontCacheSkiaWin.cpp

Status: Fixed (was: Untriaged)
Status: Started (was: Fixed)
Also happens in third_party/WebKit/Source/core/animation/AnimationClock.h where both |m_currentTask| and |s_currentTask| are getting renamed into |current_task_|.
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1f0dfac7f60c84a476ce546a5ecc1687d4e7b658

commit 1f0dfac7f60c84a476ce546a5ecc1687d4e7b658
Author: lukasza <lukasza@chromium.org>
Date: Tue Dec 13 04:40:55 2016

Rename m_currentTask and s_currentTask/ to help rewrite_to_chrome_style tool.

This CL helps rewrite_to_chrome_style tool, by preventing the tool from
suggesting the following conflicting renames:
- |s_field| -> |field_|
- |m_field| -> |field_|

BUG= 641004 

Review-Url: https://codereview.chromium.org/2569433002
Cr-Commit-Position: refs/heads/master@{#438058}

[modify] https://crrev.com/1f0dfac7f60c84a476ce546a5ecc1687d4e7b658/third_party/WebKit/Source/core/animation/AnimationClock.cpp
[modify] https://crrev.com/1f0dfac7f60c84a476ce546a5ecc1687d4e7b658/third_party/WebKit/Source/core/animation/AnimationClock.h

Status: Fixed (was: Started)
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0fa434091907b99a0fa6fb96b5b85ca1a2519d25

commit 0fa434091907b99a0fa6fb96b5b85ca1a2519d25
Author: lukasza <lukasza@chromium.org>
Date: Wed Jan 04 23:56:27 2017

Rename m_fontSize *variable* to currentFontSize.

This is needed to avoid a conflict after automatic renaming done by
rewrite_to_chrome_style clang tool that would rename 2 local variables
to the same new name.  Specifically, the tool would:
- Rename |fontSize| variable on line 65 to |font_size|
- Rename |m_fontSize| variable on line 72 to |font_size|

This CL preemptively avoids the problem described above, by manually
renaming |m_fontSize| variable to |currentFontSize|.

BUG= 641004 

Review-Url: https://codereview.chromium.org/2612093003
Cr-Commit-Position: refs/heads/master@{#441520}

[modify] https://crrev.com/0fa434091907b99a0fa6fb96b5b85ca1a2519d25/third_party/WebKit/Source/core/layout/LayoutTextTrackContainer.cpp

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/149963d671825cc359fa59c489880036cd114f8e

commit 149963d671825cc359fa59c489880036cd114f8e
Author: lukasza <lukasza@chromium.org>
Date: Fri Jan 06 23:13:31 2017

Add 'DirectionString' suffix to |rtl|, |ltr| and |inherit| constant identifiers.

After automatic rename by rewrite_to_chrome_style clang tool, the |rtl|,
|ltr| and |inherit| constant identifiers in CanvasRenderingContext2D.cpp
would become |kRtl|, |kLtr|, |kInherit| and would unfortunately conflict
with identifiers defined elsewhere (e.g. with |kRtl| and |kLtr| enum
values from TextDirection enum defined in TextDirection.h).

This CL preemptively prevents the problem above, by adding a
'DirectionString' suffix to the 3 affected identifiers in
CanvasRenderingContext2D.cpp

BUG= 641004 

Review-Url: https://codereview.chromium.org/2611723004
Cr-Commit-Position: refs/heads/master@{#442088}

[modify] https://crrev.com/149963d671825cc359fa59c489880036cd114f8e/third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.cpp

Sign in to add a comment