WebDriver: correctly handle main frame handles

https://bugs.webkit.org/show_bug.cgi?id=174668

Reviewed by Brian Burg.

When I switched to use std::optional instead of empty strings for the browsing contexts in WebDriver, I forgot
that automation uses empty string for frames to refer to the main frame. We should handle that case, because we
are currently considering empty strings as valid browsing context. It's not a big deal because Automation
converts back the empty string received to the main frame, though. We should also ensure we close the current
browsing context when switching to a new top level browsing context. This patch adds to helper private methods
to switch browsing contexts that deal with the special cases.

* Session.cpp:
(WebDriver::Session::close):
(WebDriver::Session::switchToTopLevelBrowsingContext):
(WebDriver::Session::switchToBrowsingContext):
(WebDriver::Session::createTopLevelBrowsingContext):
(WebDriver::Session::go):
(WebDriver::Session::back):
(WebDriver::Session::forward):
(WebDriver::Session::refresh):
(WebDriver::Session::switchToWindow):
(WebDriver::Session::switchToFrame):
(WebDriver::Session::switchToParentFrame):
* Session.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219721 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebDriver/ChangeLog b/Source/WebDriver/ChangeLog
index 19a3d1d51f05323420d5ef72a72da28ee90a0b9c..93aa752ee6126f6fcbfe00327f496b798a7acca7 100644 (file)
--- a/Source/WebDriver/ChangeLog
+++ b/Source/WebDriver/ChangeLog
@@ -1,3 +1,31 @@
+2017-07-21  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        WebDriver: correctly handle main frame handles
+        https://bugs.webkit.org/show_bug.cgi?id=174668
+
+        Reviewed by Brian Burg.
+
+        When I switched to use std::optional instead of empty strings for the browsing contexts in WebDriver, I forgot
+        that automation uses empty string for frames to refer to the main frame. We should handle that case, because we
+        are currently considering empty strings as valid browsing context. It's not a big deal because Automation
+        converts back the empty string received to the main frame, though. We should also ensure we close the current
+        browsing context when switching to a new top level browsing context. This patch adds to helper private methods
+        to switch browsing contexts that deal with the special cases.
+
+        * Session.cpp:
+        (WebDriver::Session::close):
+        (WebDriver::Session::switchToTopLevelBrowsingContext):
+        (WebDriver::Session::switchToBrowsingContext):
+        (WebDriver::Session::createTopLevelBrowsingContext):
+        (WebDriver::Session::go):
+        (WebDriver::Session::back):
+        (WebDriver::Session::forward):
+        (WebDriver::Session::refresh):
+        (WebDriver::Session::switchToWindow):
+        (WebDriver::Session::switchToFrame):
+        (WebDriver::Session::switchToParentFrame):
+        * Session.h:
+
 2017-07-18  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         WebDriver: handle invalid selector errors

diff --git a/Source/WebDriver/Session.cpp b/Source/WebDriver/Session.cpp
index 35ced0e6377d67051770d18093803d013ceaa6d4..fdd24dfbbed63422a7c5955d94786efbb1c387ad 100644 (file)
--- a/Source/WebDriver/Session.cpp
+++ b/Source/WebDriver/Session.cpp
@@ -71,7 +71,7 @@ void Session::close(Function<void (CommandResult&&)>&& completionHandler)
             completionHandler(CommandResult::fail(WTFMove(response.responseObject)));
             return;
         }
-        m_toplevelBrowsingContext = std::nullopt;
+        switchToTopLevelBrowsingContext(std::nullopt);
         completionHandler(CommandResult::success());
     });
 }
@@ -87,6 +87,21 @@ void Session::setTimeouts(const Timeouts& timeouts, Function<void (CommandResult
     completionHandler(CommandResult::success());
 }
 
+void Session::switchToTopLevelBrowsingContext(std::optional<String> toplevelBrowsingContext)
+{
+    m_toplevelBrowsingContext = toplevelBrowsingContext;
+    m_browsingContext = std::nullopt;
+}
+
+void Session::switchToBrowsingContext(std::optional<String> browsingContext)
+{
+    // Automation sends empty strings for main frame.
+    if (!browsingContext || browsingContext.value().isEmpty())
+        m_browsingContext = std::nullopt;
+    else
+        m_browsingContext = browsingContext;
+}
+
 void Session::createTopLevelBrowsingContext(Function<void (CommandResult&&)>&& completionHandler)
 {
     ASSERT(!m_toplevelBrowsingContext.value());
@@ -101,7 +116,7 @@ void Session::createTopLevelBrowsingContext(Function<void (CommandResult&&)>&& c
                 completionHandler(CommandResult::fail(CommandResult::ErrorCode::UnknownError));
                 return;
             }
-            m_toplevelBrowsingContext = handle;
+            switchToTopLevelBrowsingContext(handle);
             completionHandler(CommandResult::success());
         });
     });
@@ -122,7 +137,7 @@ void Session::go(const String& url, Function<void (CommandResult&&)>&& completio
             completionHandler(CommandResult::fail(WTFMove(response.responseObject)));
             return;
         }
-        m_browsingContext = std::nullopt;
+        switchToBrowsingContext(std::nullopt);
         completionHandler(CommandResult::success());
     });
 }
@@ -169,7 +184,7 @@ void Session::back(Function<void (CommandResult&&)>&& completionHandler)
             completionHandler(CommandResult::fail(WTFMove(response.responseObject)));
             return;
         }
-        m_browsingContext = std::nullopt;
+        switchToBrowsingContext(std::nullopt);
         completionHandler(CommandResult::success());
     });
 }
@@ -188,7 +203,7 @@ void Session::forward(Function<void (CommandResult&&)>&& completionHandler)
             completionHandler(CommandResult::fail(WTFMove(response.responseObject)));
             return;
         }
-        m_browsingContext = std::nullopt;
+        switchToBrowsingContext(std::nullopt);
         completionHandler(CommandResult::success());
     });
 }
@@ -207,7 +222,7 @@ void Session::refresh(Function<void (CommandResult&&)>&& completionHandler)
             completionHandler(CommandResult::fail(WTFMove(response.responseObject)));
             return;
         }
-        m_browsingContext = std::nullopt;
+        switchToBrowsingContext(std::nullopt);
         completionHandler(CommandResult::success());
     });
 }
@@ -289,7 +304,7 @@ void Session::switchToWindow(const String& windowHandle, Function<void (CommandR
             completionHandler(CommandResult::fail(WTFMove(response.responseObject)));
             return;
         }
-        m_toplevelBrowsingContext = windowHandle;
+        switchToTopLevelBrowsingContext(windowHandle);
         completionHandler(CommandResult::success());
     });
 }
@@ -340,7 +355,7 @@ void Session::switchToFrame(RefPtr<InspectorValue>&& frameID, Function<void (Com
     }
 
     if (frameID->isNull()) {
-        m_browsingContext = std::nullopt;
+        switchToBrowsingContext(std::nullopt);
         completionHandler(CommandResult::success());
         return;
     }
@@ -381,7 +396,7 @@ void Session::switchToFrame(RefPtr<InspectorValue>&& frameID, Function<void (Com
             completionHandler(CommandResult::fail(CommandResult::ErrorCode::UnknownError));
             return;
         }
-        m_browsingContext = frameHandle;
+        switchToBrowsingContext(frameHandle);
         completionHandler(CommandResult::success());
     });
 }
@@ -411,7 +426,7 @@ void Session::switchToParentFrame(Function<void (CommandResult&&)>&& completionH
             completionHandler(CommandResult::fail(CommandResult::ErrorCode::UnknownError));
             return;
         }
-        m_browsingContext = frameHandle;
+        switchToBrowsingContext(frameHandle);
         completionHandler(CommandResult::success());
     });
 }

diff --git a/Source/WebDriver/Session.h b/Source/WebDriver/Session.h
index 1b3c29b525f975fd57931a4d5c70ee9f809fe80b..19d585eff04ef606c70034ad36945737ed1b0e3b 100644 (file)
--- a/Source/WebDriver/Session.h
+++ b/Source/WebDriver/Session.h
@@ -104,6 +104,9 @@ public:
 private:
     Session(std::unique_ptr<SessionHost>&&);
 
+    void switchToTopLevelBrowsingContext(std::optional<String>);
+    void switchToBrowsingContext(std::optional<String>);
+
     RefPtr<Inspector::InspectorObject> createElement(RefPtr<Inspector::InspectorValue>&&);
     RefPtr<Inspector::InspectorObject> createElement(const String& elementID);
     RefPtr<Inspector::InspectorObject> extractElement(Inspector::InspectorValue&);