1 files changed, 138 insertions, 152 deletions

diff --git a/Source/WebCore/page/WindowFeatures.cpp b/Source/WebCore/page/WindowFeatures.cpp
index d93420161..b8adcb449 100644
--- a/Source/WebCore/page/WindowFeatures.cpp
+++ b/Source/WebCore/page/WindowFeatures.cpp
@@ -25,154 +25,123 @@
 
 #include "FloatRect.h"
 #include <wtf/Assertions.h>
+#include <wtf/HashMap.h>
 #include <wtf/MathExtras.h>
 #include <wtf/text/StringHash.h>
 
 namespace WebCore {
 
-// Though isspace() considers \t and \v to be whitespace, Win IE doesn't when parsing window features.
-static bool isWindowFeaturesSeparator(UChar c)
+typedef HashMap<String, String, ASCIICaseInsensitiveHash> DialogFeaturesMap;
+
+static void setWindowFeature(WindowFeatures&, StringView key, StringView value);
+
+static DialogFeaturesMap parseDialogFeaturesMap(const String&);
+static std::optional<bool> boolFeature(const DialogFeaturesMap&, const char* key);
+static std::optional<float> floatFeature(const DialogFeaturesMap&, const char* key, float min, float max);
+
+static bool isSeparator(UChar character)
 {
-    return c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '=' || c == ',' || c == '\0';
+    return character == ' ' || character == '\t' || character == '\n' || character == '\r' || character == '=' || character == ',';
 }
 
-WindowFeatures::WindowFeatures(const String& features)
-    : xSet(false)
-    , ySet(false)
-    , widthSet(false)
-    , heightSet(false)
-    , fullscreen(false)
-    , dialog(false)
+WindowFeatures parseWindowFeatures(StringView featuresString)
 {
-    /*
-     The IE rule is: all features except for channelmode and fullscreen default to YES, but
-     if the user specifies a feature string, all features default to NO. (There is no public
-     standard that applies to this method.)
-
-     <http://msdn.microsoft.com/workshop/author/dhtml/reference/methods/open_0.asp>
-     We always allow a window to be resized, which is consistent with Firefox.
-     */
-
-    if (features.length() == 0) {
-        menuBarVisible = true;
-        statusBarVisible = true;
-        toolBarVisible = true;
-        locationBarVisible = true;
-        scrollbarsVisible = true;
-        resizable = true;
-        return;
-    }
+    // The IE rule is: all features except for channelmode and fullscreen default to YES, but
+    // if the user specifies a feature string, all features default to NO. (There is no public
+    // standard that applies to this method.)
+    //
+    // <http://msdn.microsoft.com/workshop/author/dhtml/reference/methods/open_0.asp>
+    // We always allow a window to be resized, which is consistent with Firefox.
 
-    menuBarVisible = false;
-    statusBarVisible = false;
-    toolBarVisible = false;
-    locationBarVisible = false;
-    scrollbarsVisible = false;
-    resizable = true;
-
-    // Tread lightly in this code -- it was specifically designed to mimic Win IE's parsing behavior.
-    int keyBegin, keyEnd;
-    int valueBegin, valueEnd;
-
-    int i = 0;
-    int length = features.length();
-    String buffer = features.lower();
-    while (i < length) {
-        // skip to first non-separator, but don't skip past the end of the string
-        while (isWindowFeaturesSeparator(buffer[i])) {
-            if (i >= length)
-                break;
-            i++;
-        }
-        keyBegin = i;
+    WindowFeatures features;
 
-        // skip to first separator
-        while (!isWindowFeaturesSeparator(buffer[i]))
-            i++;
-        keyEnd = i;
+    if (featuresString.isEmpty())
+        return features;
 
-        // skip to first '=', but don't skip past a ',' or the end of the string
-        while (buffer[i] != '=') {
-            if (buffer[i] == ',' || i >= length)
-                break;
-            i++;
-        }
+    features.menuBarVisible = false;
+    features.statusBarVisible = false;
+    features.toolBarVisible = false;
+    features.locationBarVisible = false;
+    features.scrollbarsVisible = false;
 
-        // skip to first non-separator, but don't skip past a ',' or the end of the string
-        while (isWindowFeaturesSeparator(buffer[i])) {
-            if (buffer[i] == ',' || i >= length)
-                break;
-            i++;
-        }
-        valueBegin = i;
+    processFeaturesString(featuresString, [&features](StringView key, StringView value) {
+        setWindowFeature(features, key, value);
+    });
+
+    return features;
+}
+
+void processFeaturesString(StringView features, std::function<void(StringView type, StringView value)> callback)
+{
+    unsigned length = features.length();
+    for (unsigned i = 0; i < length; ) {
+        // skip to first non-separator
+        while (i < length && isSeparator(features[i]))
+            ++i;
+        unsigned keyBegin = i;
 
         // skip to first separator
-        while (!isWindowFeaturesSeparator(buffer[i]))
+        while (i < length && !isSeparator(features[i]))
             i++;
-        valueEnd = i;
+        unsigned keyEnd = i;
 
-        ASSERT_WITH_SECURITY_IMPLICATION(i <= length);
+        // skip to first '=', but don't skip past a ','
+        while (i < length && features[i] != '=' && features[i] != ',')
+            ++i;
 
-        String keyString(buffer.substring(keyBegin, keyEnd - keyBegin));
-        String valueString(buffer.substring(valueBegin, valueEnd - valueBegin));
-        setWindowFeature(keyString, valueString);
+        // skip to first non-separator, but don't skip past a ','
+        while (i < length && isSeparator(features[i]) && features[i] != ',')
+            ++i;
+        unsigned valueBegin = i;
+
+        // skip to first separator
+        while (i < length && !isSeparator(features[i]))
+            ++i;
+        unsigned valueEnd = i;
+
+        callback(features.substring(keyBegin, keyEnd - keyBegin), features.substring(valueBegin, valueEnd - valueBegin));
     }
 }
 
-void WindowFeatures::setWindowFeature(const String& keyString, const String& valueString)
+static void setWindowFeature(WindowFeatures& features, StringView key, StringView value)
 {
-    int value;
-
     // Listing a key with no value is shorthand for key=yes
-    if (valueString.isEmpty() || valueString == "yes")
-        value = 1;
+    int numericValue;
+    if (value.isEmpty() || equalLettersIgnoringASCIICase(value, "yes"))
+        numericValue = 1;
     else
-        value = valueString.toInt();
+        numericValue = value.toInt();
 
-    // We treat keyString of "resizable" here as an additional feature rather than setting resizeable to true.
+    // We treat key of "resizable" here as an additional feature rather than setting resizeable to true.
     // This is consistent with Firefox, but could also be handled at another level.
 
-    if (keyString == "left" || keyString == "screenx") {
-        xSet = true;
-        x = value;
-    } else if (keyString == "top" || keyString == "screeny") {
-        ySet = true;
-        y = value;
-    } else if (keyString == "width" || keyString == "innerwidth") {
-        widthSet = true;
-        width = value;
-    } else if (keyString == "height" || keyString == "innerheight") {
-        heightSet = true;
-        height = value;
-    } else if (keyString == "menubar")
-        menuBarVisible = value;
-    else if (keyString == "toolbar")
-        toolBarVisible = value;
-    else if (keyString == "location")
-        locationBarVisible = value;
-    else if (keyString == "status")
-        statusBarVisible = value;
-    else if (keyString == "fullscreen")
-        fullscreen = value;
-    else if (keyString == "scrollbars")
-        scrollbarsVisible = value;
-    else if (value == 1)
-        additionalFeatures.append(keyString);
+    if (equalLettersIgnoringASCIICase(key, "left") || equalLettersIgnoringASCIICase(key, "screenx"))
+        features.x = numericValue;
+    else if (equalLettersIgnoringASCIICase(key, "top") || equalLettersIgnoringASCIICase(key, "screeny"))
+        features.y = numericValue;
+    else if (equalLettersIgnoringASCIICase(key, "width") || equalLettersIgnoringASCIICase(key, "innerwidth"))
+        features.width = numericValue;
+    else if (equalLettersIgnoringASCIICase(key, "height") || equalLettersIgnoringASCIICase(key, "innerheight"))
+        features.height = numericValue;
+    else if (equalLettersIgnoringASCIICase(key, "menubar"))
+        features.menuBarVisible = numericValue;
+    else if (equalLettersIgnoringASCIICase(key, "toolbar"))
+        features.toolBarVisible = numericValue;
+    else if (equalLettersIgnoringASCIICase(key, "location"))
+        features.locationBarVisible = numericValue;
+    else if (equalLettersIgnoringASCIICase(key, "status"))
+        features.statusBarVisible = numericValue;
+    else if (equalLettersIgnoringASCIICase(key, "fullscreen"))
+        features.fullscreen = numericValue;
+    else if (equalLettersIgnoringASCIICase(key, "scrollbars"))
+        features.scrollbarsVisible = numericValue;
+    else if (numericValue == 1)
+        features.additionalFeatures.append(key.toString());
 }
 
-WindowFeatures::WindowFeatures(const String& dialogFeaturesString, const FloatRect& screenAvailableRect)
-    : widthSet(true)
-    , heightSet(true)
-    , menuBarVisible(false)
-    , toolBarVisible(false)
-    , locationBarVisible(false)
-    , fullscreen(false)
-    , dialog(true)
+WindowFeatures parseDialogFeatures(const String& dialogFeaturesString, const FloatRect& screenAvailableRect)
 {
-    DialogFeaturesMap features;
-    parseDialogFeatures(dialogFeaturesString, features);
-
-    const bool trusted = false;
+    auto featuresMap = parseDialogFeaturesMap(dialogFeaturesString);
 
     // The following features from Microsoft's documentation are not implemented:
     // - default font settings
@@ -182,66 +151,81 @@ WindowFeatures::WindowFeatures(const String& dialogFeaturesString, const FloatRe
     // - help: boolFeature(features, "help", true), makes help icon appear in dialog (what does it do on Windows?)
     // - unadorned: trusted && boolFeature(features, "unadorned");
 
-    width = floatFeature(features, "dialogwidth", 100, screenAvailableRect.width(), 620); // default here came from frame size of dialog in MacIE
-    height = floatFeature(features, "dialogheight", 100, screenAvailableRect.height(), 450); // default here came from frame size of dialog in MacIE
+    WindowFeatures features;
 
-    x = floatFeature(features, "dialogleft", screenAvailableRect.x(), screenAvailableRect.maxX() - width, -1);
-    xSet = x > 0;
-    y = floatFeature(features, "dialogtop", screenAvailableRect.y(), screenAvailableRect.maxY() - height, -1);
-    ySet = y > 0;
+    features.menuBarVisible = false;
+    features.toolBarVisible = false;
+    features.locationBarVisible = false;
+    features.dialog = true;
 
-    if (boolFeature(features, "center", true)) {
-        if (!xSet) {
-            x = screenAvailableRect.x() + (screenAvailableRect.width() - width) / 2;
-            xSet = true;
-        }
-        if (!ySet) {
-            y = screenAvailableRect.y() + (screenAvailableRect.height() - height) / 2;
-            ySet = true;
-        }
+    float width = floatFeature(featuresMap, "dialogwidth", 100, screenAvailableRect.width()).value_or(620); // default here came from frame size of dialog in MacIE
+    float height = floatFeature(featuresMap, "dialogheight", 100, screenAvailableRect.height()).value_or(450); // default here came from frame size of dialog in MacIE
+
+    features.width = width;
+    features.height = height;
+
+    features.x = floatFeature(featuresMap, "dialogleft", screenAvailableRect.x(), screenAvailableRect.maxX() - width);
+    features.y = floatFeature(featuresMap, "dialogtop", screenAvailableRect.y(), screenAvailableRect.maxY() - height);
+
+    if (boolFeature(featuresMap, "center").value_or(true)) {
+        if (!features.x)
+            features.x = screenAvailableRect.x() + (screenAvailableRect.width() - width) / 2;
+        if (!features.y)
+            features.y = screenAvailableRect.y() + (screenAvailableRect.height() - height) / 2;
     }
 
-    resizable = boolFeature(features, "resizable");
-    scrollbarsVisible = boolFeature(features, "scroll", true);
-    statusBarVisible = boolFeature(features, "status", !trusted);
+    features.resizable = boolFeature(featuresMap, "resizable").value_or(false);
+    features.scrollbarsVisible = boolFeature(featuresMap, "scroll").value_or(true);
+    features.statusBarVisible = boolFeature(featuresMap, "status").value_or(false);
+
+    return features;
 }
 
-bool WindowFeatures::boolFeature(const DialogFeaturesMap& features, const char* key, bool defaultValue)
+static std::optional<bool> boolFeature(const DialogFeaturesMap& features, const char* key)
 {
-    DialogFeaturesMap::const_iterator it = features.find(key);
+    auto it = features.find(key);
     if (it == features.end())
-        return defaultValue;
-    const String& value = it->value;
-    return value.isNull() || value == "1" || value == "yes" || value == "on";
+        return std::nullopt;
+
+    auto& value = it->value;
+    return value.isNull()
+        || value == "1"
+        || equalLettersIgnoringASCIICase(value, "yes")
+        || equalLettersIgnoringASCIICase(value, "on");
 }
 
-float WindowFeatures::floatFeature(const DialogFeaturesMap& features, const char* key, float min, float max, float defaultValue)
+static std::optional<float> floatFeature(const DialogFeaturesMap& features, const char* key, float min, float max)
 {
-    DialogFeaturesMap::const_iterator it = features.find(key);
+    auto it = features.find(key);
     if (it == features.end())
-        return defaultValue;
+        return std::nullopt;
+
     // FIXME: The toDouble function does not offer a way to tell "0q" from string with no digits in it: Both
     // return the number 0 and false for ok. But "0q" should yield the minimum rather than the default.
     bool ok;
     double parsedNumber = it->value.toDouble(&ok);
     if ((!parsedNumber && !ok) || std::isnan(parsedNumber))
-        return defaultValue;
+        return std::nullopt;
     if (parsedNumber < min || max <= min)
         return min;
     if (parsedNumber > max)
         return max;
+
     // FIXME: Seems strange to cast a double to int and then convert back to a float. Why is this a good idea?
     return static_cast<int>(parsedNumber);
 }
 
-void WindowFeatures::parseDialogFeatures(const String& string, DialogFeaturesMap& map)
+static DialogFeaturesMap parseDialogFeaturesMap(const String& string)
 {
+    // FIXME: Not clear why we take such a different approach to parsing dialog features
+    // as opposed to window features (using a map, different parsing quirks).
+
+    DialogFeaturesMap features;
+
     Vector<String> vector;
     string.split(';', vector);
-    size_t size = vector.size();
-    for (size_t i = 0; i < size; ++i) {
-        const String& featureString = vector[i];
 
+    for (auto& featureString : vector) {
         size_t separatorPosition = featureString.find('=');
         size_t colonPosition = featureString.find(':');
         if (separatorPosition != notFound && colonPosition != notFound)
@@ -249,17 +233,19 @@ void WindowFeatures::parseDialogFeatures(const String& string, DialogFeaturesMap
         if (separatorPosition == notFound)
             separatorPosition = colonPosition;
 
-        String key = featureString.left(separatorPosition).stripWhiteSpace().lower();
+        String key = featureString.left(separatorPosition).stripWhiteSpace();
 
         // Null string for value indicates key without value.
         String value;
         if (separatorPosition != notFound) {
-            value = featureString.substring(separatorPosition + 1).stripWhiteSpace().lower();
+            value = featureString.substring(separatorPosition + 1).stripWhiteSpace();
             value = value.left(value.find(' '));
         }
 
-        map.set(key, value);
+        features.set(key, value);
     }
+
+    return features;
 }
 
 } // namespace WebCore