Changeset 188386 in webkit

Timestamp:

Aug 13, 2015, 10:37:27 AM (11 years ago)

Author:

andersca@apple.com

Message:

Use WTF::Optional in WindowFeatures
​https://bugs.webkit.org/show_bug.cgi?id=147956

Reviewed by Sam Weinig.

Source/WebCore:

• loader/FrameLoader.cpp:

(WebCore::createWindow):

• page/WindowFeatures.cpp:

(WebCore::WindowFeatures::WindowFeatures):
(WebCore::WindowFeatures::setWindowFeature):
(WebCore::WindowFeatures::boolFeature):
(WebCore::WindowFeatures::floatFeature):
(WebCore::WindowFeatures::parseDialogFeatures):

• page/WindowFeatures.h:

(WebCore::WindowFeatures::WindowFeatures):

Source/WebKit/mac:

• WebCoreSupport/WebChromeClient.mm:

(WebChromeClient::createWindow):

Source/WebKit/win:

• WebCoreSupport/WebChromeClient.cpp:

(createWindowFeaturesPropertyBag):

Source/WebKit2:

• Shared/WebCoreArgumentCoders.cpp:

(IPC::ArgumentCoder<WindowFeatures>::encode): Deleted.
(IPC::ArgumentCoder<WindowFeatures>::decode): Deleted.

• UIProcess/API/C/WKPage.cpp:

(WKPageSetPageUIClient):

• UIProcess/API/Cocoa/WKWindowFeatures.mm:

(-[WKWindowFeatures _initWithWindowFeatures:]):

Source/WTF:

Add new operators to WTF::Optional to make it more like std::optional.

• wtf/Optional.h:

(WTF::Optional::operator->):
(WTF::Optional::operator*):

Location:

trunk/Source

Files:

• WTF/ChangeLog (modified) (1 diff)
• WTF/wtf/Optional.h (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/loader/FrameLoader.cpp (modified) (2 diffs)
• WebCore/page/WindowFeatures.cpp (modified) (8 diffs)
• WebCore/page/WindowFeatures.h (modified) (4 diffs)
• WebKit/mac/ChangeLog (modified) (1 diff)
• WebKit/mac/WebCoreSupport/WebChromeClient.mm (modified) (1 diff)
• WebKit/win/ChangeLog (modified) (1 diff)
• WebKit/win/WebCoreSupport/WebChromeClient.cpp (modified) (1 diff)
• WebKit2/ChangeLog (modified) (1 diff)
• WebKit2/Shared/WebCoreArgumentCoders.cpp (modified) (2 diffs)
• WebKit2/UIProcess/API/C/WKPage.cpp (modified) (1 diff)
• WebKit2/UIProcess/API/Cocoa/WKWindowFeatures.mm (modified) (1 diff)

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2015-08-12  Anders Carlsson  <andersca@apple.com>
+
+        Use WTF::Optional in WindowFeatures
+        https://bugs.webkit.org/show_bug.cgi?id=147956
+
+        Reviewed by Sam Weinig.
+
+        Add new operators to WTF::Optional to make it more like std::optional.
+
+        * wtf/Optional.h:
+        (WTF::Optional::operator->):
+        (WTF::Optional::operator*):
+
 2015-08-12  Filip Pizlo  <fpizlo@apple.com>
 

trunk/Source/WTF/wtf/Optional.h

@@ -134 +134 @@
     explicit operator bool() const { return m_isEngaged; }
 
+    const T* operator->() const
+    {
+        ASSERT(m_isEngaged);
+        return asPtr()->operator->();
+    }
+
+    T* operator->()
+    {
+        ASSERT(m_isEngaged);
+        return asPtr()->operator->();
+    }
+
+    const T& operator*() const { return value(); }
+    T& operator*() { return value(); }
+
     T& value()
     {

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-08-12  Anders Carlsson  <andersca@apple.com>
+
+        Use WTF::Optional in WindowFeatures
+        https://bugs.webkit.org/show_bug.cgi?id=147956
+
+        Reviewed by Sam Weinig.
+
+        * loader/FrameLoader.cpp:
+        (WebCore::createWindow):
+        * page/WindowFeatures.cpp:
+        (WebCore::WindowFeatures::WindowFeatures):
+        (WebCore::WindowFeatures::setWindowFeature):
+        (WebCore::WindowFeatures::boolFeature):
+        (WebCore::WindowFeatures::floatFeature):
+        (WebCore::WindowFeatures::parseDialogFeatures):
+        * page/WindowFeatures.h:
+        (WebCore::WindowFeatures::WindowFeatures):
+
 2015-08-13  Matthew Daiter  <mdaiter@apple.com>
 

trunk/Source/WebCore/loader/FrameLoader.cpp

@@ -3552 +3552 @@
     FloatSize viewportSize = page->chrome().pageRect().size();
     FloatRect windowRect = page->chrome().windowRect();
-    if (features.xSet)
-        windowRect.setX(features.x);
-    if (features.ySet)
-        windowRect.setY(features.y);
+    if (features.x)
+        windowRect.setX(*features.x);
+    if (features.y)
+        windowRect.setY(*features.y);
     // Zero width and height mean using default size, not minumum one.
-    if (features.widthSet && features.width)
-        windowRect.setWidth(features.width + (windowRect.width() - viewportSize.width()));
-    if (features.heightSet && features.height)
-        windowRect.setHeight(features.height + (windowRect.height() - viewportSize.height()));
+    if (features.width && *features.width)
+        windowRect.setWidth(*features.width + (windowRect.width() - viewportSize.width()));
+    if (features.height && *features.height)
+        windowRect.setHeight(*features.height + (windowRect.height() - viewportSize.height()));
 
     // Ensure non-NaN values, minimum size as well as being within valid screen area.
@@ -3572 +3572 @@
     ViewportArguments arguments;
     // Zero width and height mean using default size, not minimum one.
-    if (features.widthSet && features.width)
-        arguments.width = features.width;
-    if (features.heightSet && features.height)
-        arguments.height = features.height;
+    if (features.width && *features.width)
+        arguments.width = *features.width;
+    if (features.height && *features.height)
+        arguments.height = *features.height;
     frame->setViewportArguments(arguments);
 #endif

trunk/Source/WebCore/page/WindowFeatures.cpp

@@ -38 +38 @@
 
 WindowFeatures::WindowFeatures(const String& features)
-    : x(0)
-    , xSet(false)
-    , y(0)
-    , ySet(false)
-    , width(0)
-    , widthSet(false)
-    , height(0)
-    , heightSet(false)
-    , resizable(true)
+    : resizable(true)
     , fullscreen(false)
     , dialog(false)
@@ -135 +127 @@
     // This is consistent with Firefox, but could also be handled at another level.
 
-    if (keyString == "left" || keyString == "screenx") {
-        xSet = true;
+    if (keyString == "left" || keyString == "screenx")
         x = value;
-    } else if (keyString == "top" || keyString == "screeny") {
-        ySet = true;
+    else if (keyString == "top" || keyString == "screeny")
         y = value;
-    } else if (keyString == "width" || keyString == "innerwidth") {
-        widthSet = true;
+    else if (keyString == "width" || keyString == "innerwidth")
         width = value;
-    } else if (keyString == "height" || keyString == "innerheight") {
-        heightSet = true;
+    else if (keyString == "height" || keyString == "innerheight")
         height = value;
-    } else if (keyString == "menubar")
+    else if (keyString == "menubar")
         menuBarVisible = value;
     else if (keyString == "toolbar")
@@ -164 +152 @@
 
 WindowFeatures::WindowFeatures(const String& dialogFeaturesString, const FloatRect& screenAvailableRect)
-    : widthSet(true)
-    , heightSet(true)
-    , menuBarVisible(false)
+    : menuBarVisible(false)
     , toolBarVisible(false)
     , locationBarVisible(false)
@@ -172 +158 @@
     , dialog(true)
 {
-    DialogFeaturesMap features;
-    parseDialogFeatures(dialogFeaturesString, features);
+    auto features = parseDialogFeatures(dialogFeaturesString);
 
     const bool trusted = false;
@@ -188 +173 @@
     height = floatFeature(features, "dialogheight", 100, screenAvailableRect.height(), 450); // default here came from frame size of dialog in MacIE
 
-    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;
+    auto dialogLeft = floatFeature(features, "dialogleft", screenAvailableRect.x(), screenAvailableRect.maxX() - *width, -1);
+    if (dialogLeft > 0)
+        x = dialogLeft;
+
+    auto dialogTop = floatFeature(features, "dialogtop", screenAvailableRect.y(), screenAvailableRect.maxY() - *height, -1);
+    if (dialogTop > 0)
+        y = dialogTop;
 
     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;
-        }
+        if (!x)
+            x = screenAvailableRect.x() + (screenAvailableRect.width() - *width) / 2;
+
+        if (!y)
+            y = screenAvailableRect.y() + (screenAvailableRect.height() - *height) / 2;
     }
 
@@ -209 +194 @@
 }
 
-bool WindowFeatures::boolFeature(const DialogFeaturesMap& features, const char* key, bool defaultValue)
-{
-    DialogFeaturesMap::const_iterator it = features.find(key);
+bool WindowFeatures::boolFeature(const HashMap<String, String>& features, const char* key, bool defaultValue)
+{
+    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";
 }
 
-float WindowFeatures::floatFeature(const DialogFeaturesMap& features, const char* key, float min, float max, float defaultValue)
-{
-    DialogFeaturesMap::const_iterator it = features.find(key);
+float WindowFeatures::floatFeature(const HashMap<String, String>& features, const char* key, float min, float max, float defaultValue)
+{
+    auto it = features.find(key);
     if (it == features.end())
         return defaultValue;
+
     // 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.
@@ -233 +220 @@
     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)
-{
+HashMap<String, String> WindowFeatures::parseDialogFeatures(const String& string)
+{
+    HashMap<String, String> features;
+
     Vector<String> vector;
     string.split(';', vector);
+
     for (auto& featureString : vector) {
         size_t separatorPosition = featureString.find('=');
@@ -258 +249 @@
         }
 
-        map.set(key, value);
-    }
+        features.set(key, value);
+    }
+
+    return features;
 }
 

trunk/Source/WebCore/page/WindowFeatures.h

@@ -30 +30 @@
 #define WindowFeatures_h
 
+#include <wtf/Forward.h>
 #include <wtf/HashMap.h>
-#include <wtf/text/WTFString.h>
+#include <wtf/Optional.h>
+#include <wtf/Vector.h>
+#include <wtf/text/StringHash.h>
 
 namespace WebCore {
@@ -39 +42 @@
 struct WindowFeatures {
     WindowFeatures()
-        : x(0)
-        , xSet(false)
-        , y(0)
-        , ySet(false)
-        , width(0)
-        , widthSet(false)
-        , height(0)
-        , heightSet(false)
-        , menuBarVisible(true)
+        : menuBarVisible(true)
         , statusBarVisible(true)
         , toolBarVisible(true)
@@ -60 +55 @@
     WindowFeatures(const String& dialogFeaturesString, const FloatRect& screenAvailableRect);
 
-    float x;
-    bool xSet;
-    float y;
-    bool ySet;
-    float width;
-    bool widthSet;
-    float height;
-    bool heightSet;
+    Optional<float> x;
+    Optional<float> y;
+    Optional<float> width;
+    Optional<float> height;
 
     bool menuBarVisible;
@@ -82 +73 @@
 
 private:
-    typedef HashMap<String, String> DialogFeaturesMap;
-    static void parseDialogFeatures(const String&, HashMap<String, String>&);
-    static bool boolFeature(const DialogFeaturesMap&, const char* key, bool defaultValue = false);
-    static float floatFeature(const DialogFeaturesMap&, const char* key, float min, float max, float defaultValue);
+    static HashMap<String, String> parseDialogFeatures(const String&);
+    static bool boolFeature(const HashMap<String, String>&, const char* key, bool defaultValue = false);
+    static float floatFeature(const HashMap<String, String>&, const char* key, float min, float max, float defaultValue);
     void setWindowFeature(const String& keyString, const String& valueString);
 };

trunk/Source/WebKit/mac/ChangeLog

@@ +1 @@
+2015-08-12  Anders Carlsson  <andersca@apple.com>
+
+        Use WTF::Optional in WindowFeatures
+        https://bugs.webkit.org/show_bug.cgi?id=147956
+
+        Reviewed by Sam Weinig.
+
+        * WebCoreSupport/WebChromeClient.mm:
+        (WebChromeClient::createWindow):
+
 2015-08-13  Matthew Daiter  <mdaiter@apple.com>
 

trunk/Source/WebKit/mac/WebCoreSupport/WebChromeClient.mm

@@ -241 +241 @@
     
     if ([delegate respondsToSelector:@selector(webView:createWebViewWithRequest:windowFeatures:)]) {
-        NSNumber *x = features.xSet ? [[NSNumber alloc] initWithFloat:features.x] : nil;
-        NSNumber *y = features.ySet ? [[NSNumber alloc] initWithFloat:features.y] : nil;
-        NSNumber *width = features.widthSet ? [[NSNumber alloc] initWithFloat:features.width] : nil;
-        NSNumber *height = features.heightSet ? [[NSNumber alloc] initWithFloat:features.height] : nil;
+        NSNumber *x = features.x ? [[NSNumber alloc] initWithFloat:*features.x] : nil;
+        NSNumber *y = features.y ? [[NSNumber alloc] initWithFloat:*features.y] : nil;
+        NSNumber *width = features.width ? [[NSNumber alloc] initWithFloat:*features.width] : nil;
+        NSNumber *height = features.height ? [[NSNumber alloc] initWithFloat:*features.height] : nil;
         NSNumber *menuBarVisible = [[NSNumber alloc] initWithBool:features.menuBarVisible];
         NSNumber *statusBarVisible = [[NSNumber alloc] initWithBool:features.statusBarVisible];

trunk/Source/WebKit/win/ChangeLog

@@ +1 @@
+2015-08-12  Anders Carlsson  <andersca@apple.com>
+
+        Use WTF::Optional in WindowFeatures
+        https://bugs.webkit.org/show_bug.cgi?id=147956
+
+        Reviewed by Sam Weinig.
+
+        * WebCoreSupport/WebChromeClient.cpp:
+        (createWindowFeaturesPropertyBag):
+
 2015-08-10  Per Arne Vollan  <peavo@outlook.com>
 

trunk/Source/WebKit/win/WebCoreSupport/WebChromeClient.cpp

@@ -171 +171 @@
 {
     HashMap<String, COMVariant> map;
-    if (features.xSet)
-        map.set(WebWindowFeaturesXKey, features.x);
-    if (features.ySet)
-        map.set(WebWindowFeaturesYKey, features.y);
-    if (features.widthSet)
-        map.set(WebWindowFeaturesWidthKey, features.width);
-    if (features.heightSet)
-        map.set(WebWindowFeaturesHeightKey, features.height);
+    if (features.x)
+        map.set(WebWindowFeaturesXKey, *features.x);
+    if (features.y)
+        map.set(WebWindowFeaturesYKey, *features.y);
+    if (features.width)
+        map.set(WebWindowFeaturesWidthKey, *features.width);
+    if (features.height)
+        map.set(WebWindowFeaturesHeightKey, *features.height);
     map.set(WebWindowFeaturesMenuBarVisibleKey, features.menuBarVisible);
     map.set(WebWindowFeaturesStatusBarVisibleKey, features.statusBarVisible);

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2015-08-12  Anders Carlsson  <andersca@apple.com>
+
+        Use WTF::Optional in WindowFeatures
+        https://bugs.webkit.org/show_bug.cgi?id=147956
+
+        Reviewed by Sam Weinig.
+
+        * Shared/WebCoreArgumentCoders.cpp:
+        (IPC::ArgumentCoder<WindowFeatures>::encode): Deleted.
+        (IPC::ArgumentCoder<WindowFeatures>::decode): Deleted.
+        * UIProcess/API/C/WKPage.cpp:
+        (WKPageSetPageUIClient):
+        * UIProcess/API/Cocoa/WKWindowFeatures.mm:
+        (-[WKWindowFeatures _initWithWindowFeatures:]):
+
 2015-08-13  Matthew Daiter  <mdaiter@apple.com>
 

trunk/Source/WebKit2/Shared/WebCoreArgumentCoders.cpp

@@ -957 +957 @@
     encoder << windowFeatures.width;
     encoder << windowFeatures.height;
-    encoder << windowFeatures.xSet;
-    encoder << windowFeatures.ySet;
-    encoder << windowFeatures.widthSet;
-    encoder << windowFeatures.heightSet;
     encoder << windowFeatures.menuBarVisible;
     encoder << windowFeatures.statusBarVisible;
@@ -980 +976 @@
         return false;
     if (!decoder.decode(windowFeatures.height))
-        return false;
-    if (!decoder.decode(windowFeatures.xSet))
-        return false;
-    if (!decoder.decode(windowFeatures.ySet))
-        return false;
-    if (!decoder.decode(windowFeatures.widthSet))
-        return false;
-    if (!decoder.decode(windowFeatures.heightSet))
         return false;
     if (!decoder.decode(windowFeatures.menuBarVisible))

trunk/Source/WebKit2/UIProcess/API/C/WKPage.cpp

@@ -1351 +1351 @@
 
             API::Dictionary::MapType map;
-            if (windowFeatures.xSet)
-                map.set("x", API::Double::create(windowFeatures.x));
-            if (windowFeatures.ySet)
-                map.set("y", API::Double::create(windowFeatures.y));
-            if (windowFeatures.widthSet)
-                map.set("width", API::Double::create(windowFeatures.width));
-            if (windowFeatures.heightSet)
-                map.set("height", API::Double::create(windowFeatures.height));
+            if (windowFeatures.x)
+                map.set("x", API::Double::create(*windowFeatures.x));
+            if (windowFeatures.y)
+                map.set("y", API::Double::create(*windowFeatures.y));
+            if (windowFeatures.width)
+                map.set("width", API::Double::create(*windowFeatures.width));
+            if (windowFeatures.height)
+                map.set("height", API::Double::create(*windowFeatures.height));
             map.set("menuBarVisible", API::Boolean::create(windowFeatures.menuBarVisible));
             map.set("statusBarVisible", API::Boolean::create(windowFeatures.statusBarVisible));

trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWindowFeatures.mm

@@ -56 +56 @@
     _allowsResizing = @(windowFeatures.resizable);
 
-    if (windowFeatures.xSet)
-        _x = @(windowFeatures.x);
-    if (windowFeatures.ySet)
-        _y = @(windowFeatures.y);
-    if (windowFeatures.widthSet)
-        _width = @(windowFeatures.width);
-    if (windowFeatures.heightSet)
-        _height = @(windowFeatures.height);
+    if (windowFeatures.x)
+        _x = @(*windowFeatures.x);
+    if (windowFeatures.y)
+        _y = @(*windowFeatures.y);
+    if (windowFeatures.width)
+        _width = @(*windowFeatures.width);
+    if (windowFeatures.height)
+        _height = @(*windowFeatures.height);
 
     return self;