Bug 131552 - Wrap CSS length conversion arguments in an object
Summary: Wrap CSS length conversion arguments in an object
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Bem Jones-Bey
URL: https://codereview.chromium.org/64293...
Keywords:
Depends on:
Blocks: 87846
  Show dependency treegraph
 
Reported: 2014-04-11 11:42 PDT by Bem Jones-Bey
Modified: 2014-04-29 09:52 PDT (History)
16 users (show)

See Also:


Attachments
Patch (131.45 KB, patch)
2014-04-11 11:53 PDT, Bem Jones-Bey
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bem Jones-Bey 2014-04-11 11:42:16 PDT
This is a port of a patch from Blink by timloh@chromium.org
Comment 1 Bem Jones-Bey 2014-04-11 11:53:15 PDT
Created attachment 229146 [details]
Patch
Comment 2 WebKit Commit Bot 2014-04-11 11:55:46 PDT
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.
Comment 3 Andreas Kling 2014-04-28 19:45:04 PDT
Comment on attachment 229146 [details]
Patch

This look reasonable.
Comment 4 WebKit Commit Bot 2014-04-29 09:37:12 PDT
Comment on attachment 229146 [details]
Patch

Clearing flags on attachment: 229146

Committed r167937: <http://trac.webkit.org/changeset/167937>
Comment 5 WebKit Commit Bot 2014-04-29 09:37:19 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 zalan 2014-04-29 09:38:00 PDT
>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)
Comment 7 Bem Jones-Bey 2014-04-29 09:51:19 PDT
(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.
Comment 8 zalan 2014-04-29 09:52:35 PDT
(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!