Using Enum in place of bool for better code readability

Source/core/css/FontSize.cpp

Index: Source/core/css/FontSize.cpp
diff --git a/Source/core/css/FontSize.cpp b/Source/core/css/FontSize.cpp
index 6c1b53c528205b776fb8ad2348efd357c0cf083e..d56dcf03c5a16d2d80cbdd5aa5d4134359295876 100644
--- a/Source/core/css/FontSize.cpp
+++ b/Source/core/css/FontSize.cpp
@@ -120,17 +120,25 @@ static const int strictFontSizeTable[fontSizeTableMax - fontSizeTableMin + 1][to
 // factors for each keyword value.
 static const float fontSizeFactors[totalKeywords] = { 0.60f, 0.75f, 0.89f, 1.0f, 1.2f, 1.5f, 2.0f, 3.0f };
 
-float FontSize::fontSizeForKeyword(const Document* document, int keyword, bool shouldUseFixedDefaultSize)
+static int inline getRowFromMediumFontSizeInRange(const Settings* settings, bool quirksMode, FixedPitchFontType fixedPitchFontType, int& mediumSize)

[Inactive, 2014/06/26 19:43:11]

We don't usually use "get" prefix.

[h.joshi, 2014/06/27 06:55:27]

Done, removed "get" from method name.

 {
+    mediumSize = fixedPitchFontType == FixedPitchFont ? settings->defaultFixedFontSize() : settings->defaultFontSize();
+    if (mediumSize >= fontSizeTableMin && mediumSize <= fontSizeTableMax)
+        return mediumSize - fontSizeTableMin;
+    return -1;
+}
+
+float FontSize::fontSizeForKeyword(const Document* document, CSSValueID keyword, FixedPitchFontType fixedPitchFontType)
+{
+    ASSERT(keyword >= CSSValueXxSmall && keyword <= CSSValueWebkitXxxLarge);
     const Settings* settings = document->settings();
     if (!settings)
         return 1.0f;
 
     bool quirksMode = document->inQuirksMode();
-    int mediumSize = shouldUseFixedDefaultSize ? settings->defaultFixedFontSize() : settings->defaultFontSize();
-    if (mediumSize >= fontSizeTableMin && mediumSize <= fontSizeTableMax) {
-        // Look up the entry in the table.
-        int row = mediumSize - fontSizeTableMin;
+    int mediumSize = 0;
+    int row = getRowFromMediumFontSizeInRange(settings, quirksMode, fixedPitchFontType, mediumSize);
+    if (row >= 0) {
         int col = (keyword - CSSValueXxSmall);
         return quirksMode ? quirksFontSizeTable[row][col] : strictFontSizeTable[row][col];
     }
@@ -146,25 +154,26 @@ template<typename T>
 static int findNearestLegacyFontSize(int pixelFontSize, const T* table, int multiplier)
 {
     // Ignore table[0] because xx-small does not correspond to any legacy font size.
-    for (int i = 1; i < totalKeywords - 1; i++) {
-        if (pixelFontSize * 2 < (table[i] + table[i + 1]) * multiplier)
+    pixelFontSize = pixelFontSize << 1;

[Inactive, 2014/06/26 19:43:11]

Less readable IMHO, I would keep it as it was.

[h.joshi, 2014/06/27 06:55:27]

Done, deleted my changes from this method.
We were doing "pixelFontSize * 2" inside loop which is not good that was the
reason made changes, reverted my changes now.

+    int numTotalKeywords = totalKeywords - 1;

[Inactive, 2014/06/26 19:43:11]

I think this is less readable. Especially since the variable is badly named.
This is not the total number of keywords, but this index for the last keyword.
"totalKeywords" *is* the total number of keywords.

[h.joshi, 2014/06/27 06:55:27]

Reverted my changes for this method.

+    for (int i = 1; i < numTotalKeywords; i++) {
+        if (pixelFontSize < (table[i] + table[i + 1]) * multiplier)
             return i;
     }
-    return totalKeywords - 1;
+    return numTotalKeywords;
 }
 
-int FontSize::legacyFontSize(const Document* document, int pixelFontSize, bool shouldUseFixedDefaultSize)
+int FontSize::legacyFontSize(const Document* document, int pixelFontSize, FixedPitchFontType fixedPitchFontType)
 {
     const Settings* settings = document->settings();
     if (!settings)
         return 1;
 
     bool quirksMode = document->inQuirksMode();
-    int mediumSize = shouldUseFixedDefaultSize ? settings->defaultFixedFontSize() : settings->defaultFontSize();
-    if (mediumSize >= fontSizeTableMin && mediumSize <= fontSizeTableMax) {
-        int row = mediumSize - fontSizeTableMin;
+    int mediumSize = 0;
+    int row = getRowFromMediumFontSizeInRange(settings, quirksMode, fixedPitchFontType, mediumSize);
+    if (row >= 0)
         return findNearestLegacyFontSize<int>(pixelFontSize, quirksMode ? quirksFontSizeTable[row] : strictFontSizeTable[row], 1);
-    }
 
     return findNearestLegacyFontSize<float>(pixelFontSize, fontSizeFactors, mediumSize);
 }