Fix double comparisons for Media Queries

third_party/WebKit/Source/core/css/MediaValues.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/css/MediaValues.h"
 
 #include "core/css/CSSHelper.h"
 #include "core/css/MediaValuesCached.h"
 #include "core/css/MediaValuesDynamic.h"
 #include "core/dom/Document.h"
@@ skipping 141 matching lines @@
     return frame->settings()->availableHoverTypes();
 }
 
 bool MediaValues::computeLengthImpl(double value, CSSPrimitiveValue::UnitType type, unsigned defaultFontSize, double viewportWidth, double viewportHeight, double& result)
 {
     // The logic in this function is duplicated from CSSToLengthConversionData::zoomedComputedPixels()
     // because MediaValues::computeLength() needs nearly identical logic, but we haven't found a way to make
     // CSSToLengthConversionData::zoomedComputedPixels() more generic (to solve both cases) without hurting performance.
 
     // FIXME - Unite the logic here with CSSToLengthConversionData in a performant way.
-    double factor = 0;
+    double nominator = 0;
+    double denominator = 1;
     switch (type) {
     case CSSPrimitiveValue::UnitType::Ems:
     case CSSPrimitiveValue::UnitType::Rems:
-        factor = defaultFontSize;
+        nominator = defaultFontSize;
         break;
     case CSSPrimitiveValue::UnitType::Pixels:
     case CSSPrimitiveValue::UnitType::UserUnits:
-        factor = 1;
+        nominator = 1;

[rune, 2016/04/20 07:04:46]

Wouldn't it be simpler to just set "value" for each case in this switch and just
drop nominator/denominator?

And even return true instead of break and drop the code after the switch?

[Yoav Weiss, 2016/04/20 07:26:57]

Yeah, changed it to suggested form

         break;
     case CSSPrimitiveValue::UnitType::Exs:
         // FIXME: We have a bug right now where the zoom will be applied twice to EX units.
         // FIXME: We don't seem to be able to cache fontMetrics related values.
         // Trying to access them is triggering some sort of microtask. Serving the spec's default instead.
-        factor = defaultFontSize / 2.0;
+        nominator = defaultFontSize;
+        denominator = 2.0;
         break;
     case CSSPrimitiveValue::UnitType::Chs:
         // FIXME: We don't seem to be able to cache fontMetrics related values.
         // Trying to access them is triggering some sort of microtask. Serving the (future) spec default instead.
-        factor = defaultFontSize / 2.0;
+        nominator = defaultFontSize;
+        denominator = 2.0;
         break;
     case CSSPrimitiveValue::UnitType::ViewportWidth:
-        factor = viewportWidth / 100.0;
+        nominator = viewportWidth;
+        denominator = 100.0;
         break;
     case CSSPrimitiveValue::UnitType::ViewportHeight:
-        factor = viewportHeight / 100.0;
+        nominator = viewportHeight;
+        denominator = 100.0;
         break;
     case CSSPrimitiveValue::UnitType::ViewportMin:
-        factor = std::min(viewportWidth, viewportHeight) / 100.0;
+        nominator = std::min(viewportWidth, viewportHeight);
+        denominator = 100.0;
         break;
     case CSSPrimitiveValue::UnitType::ViewportMax:
-        factor = std::max(viewportWidth, viewportHeight) / 100.0;
+        nominator = std::max(viewportWidth, viewportHeight);
+        denominator = 100.0;
         break;
     case CSSPrimitiveValue::UnitType::Centimeters:
-        factor = cssPixelsPerCentimeter;
+        nominator = cssPixelsPerCentimeter;
         break;
     case CSSPrimitiveValue::UnitType::Millimeters:
-        factor = cssPixelsPerMillimeter;
+        nominator = cssPixelsPerMillimeter;
         break;
     case CSSPrimitiveValue::UnitType::Inches:
-        factor = cssPixelsPerInch;
+        nominator = cssPixelsPerInch;
         break;
     case CSSPrimitiveValue::UnitType::Points:
-        factor = cssPixelsPerPoint;
+        nominator = cssPixelsPerPoint;
         break;
     case CSSPrimitiveValue::UnitType::Picas:
-        factor = cssPixelsPerPica;
+        nominator = cssPixelsPerPica;
         break;
     default:
         return false;
     }
 
-    ASSERT(factor >= 0);
-    result = value * factor;
+    ASSERT(nominator >= 0);
+    result = (value * nominator) / denominator;
     return true;
 }
 
 LocalFrame* MediaValues::frameFrom(Document& document)
 {
     Document* executingDocument = document.importsController() ? document.importsController()->master() : &document;
     ASSERT(executingDocument);
     return executingDocument->frame();
 }
 
 } // namespace blink