Bug 131552 – Wrap CSS length conversion arguments in an object

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Bem Jones-Bey]

This is a port of a patch from Blink by [e-mail]

[Bem Jones-Bey]

Created attachment 229146 [details]
Patch

[WebKit Commit Bot]

Attachment 229146 [details] did not pass style-queue:


ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:217:  color_indexMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:260:  aspect_ratioMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:274:  device_aspect_ratioMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:318:  device_pixel_ratioMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:365:  device_heightMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:378:  device_widthMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:427:  min_colorMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:432:  max_colorMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:437:  min_color_indexMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:442:  max_color_indexMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:447:  min_monochromeMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:452:  max_monochromeMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:457:  min_aspect_ratioMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:462:  max_aspect_ratioMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:467:  min_device_aspect_ratioMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:472:  max_device_aspect_ratioMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:477:  min_device_pixel_ratioMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:482:  max_device_pixel_ratioMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:487:  min_heightMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:492:  max_heightMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:497:  min_widthMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:502:  max_widthMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:507:  min_device_heightMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:512:  max_device_heightMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:517:  min_device_widthMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:522:  max_device_widthMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:527:  min_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:532:  max_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:555:  transform_2dMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:564:  transform_3dMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:590:  view_modeMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:635:  video_playable_inlineMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 32 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andreas Kling]

Comment on attachment 229146 [details]
Patch

This look reasonable.

[WebKit Commit Bot]

Comment on attachment 229146 [details]
Patch

Clearing flags on attachment: 229146

Committed r167937: <http://trac.webkit.org/changeset/167937>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[zalan]

>particular adding a RenderView for resolving viewport units at style
My only concern was whether doing any unnecessary renderView->viewportSize() calls in order to eliminate the RenderView* parameter had any performance impact, but apparently this patch doesnt do that change at all (so not sure whether the changelog entry reflects what this patch does)

[Bem Jones-Bey]

(In reply to comment #6)
> >particular adding a RenderView for resolving viewport units at style
> My only concern was whether doing any unnecessary renderView->viewportSize() calls in order to eliminate the RenderView* parameter had any performance impact, but apparently this patch doesnt do that change at all (so not sure whether the changelog entry reflects what this patch does)

The RenderView change will happen in a follow up patch, the changelog here just mentions it as rationale for why we'd want to have a class for these arguments.

I will be posting the followup change to Bug 87846. While the current patch up there passes around IntSize, given the performance concerns that you voiced, I will be changing that to be a RenderView and to only make the viewportSize() call when resolving the actual viewport units.

[zalan]

(In reply to comment #7)
> (In reply to comment #6)
> > >particular adding a RenderView for resolving viewport units at style
> > My only concern was whether doing any unnecessary renderView->viewportSize() calls in order to eliminate the RenderView* parameter had any performance impact, but apparently this patch doesnt do that change at all (so not sure whether the changelog entry reflects what this patch does)
> 
> The RenderView change will happen in a follow up patch, the changelog here just mentions it as rationale for why we'd want to have a class for these arguments.
> 
> I will be posting the followup change to Bug 87846. While the current patch up there passes around IntSize, given the performance concerns that you voiced, I will be changing that to be a RenderView and to only make the viewportSize() call when resolving the actual viewport units.

ok, cool. Thanks!