From b99ee319c833e72c996ff2f56b3efdef68fdf19c Mon Sep 17 00:00:00 2001 From: "carlosgc@webkit.org" Date: Fri, 21 Jul 2017 09:43:13 +0000 Subject: [PATCH] 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 --- Source/WebDriver/ChangeLog | 28 ++++++++++++++++++++++++++++ Source/WebDriver/Session.cpp | 35 +++++++++++++++++++++++++---------- Source/WebDriver/Session.h | 3 +++ 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/Source/WebDriver/ChangeLog b/Source/WebDriver/ChangeLog index 19a3d1d51f05..93aa752ee612 100644 --- a/Source/WebDriver/ChangeLog +++ b/Source/WebDriver/ChangeLog @@ -1,3 +1,31 @@ +2017-07-21 Carlos Garcia Campos + + 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 WebDriver: handle invalid selector errors diff --git a/Source/WebDriver/Session.cpp b/Source/WebDriver/Session.cpp index 35ced0e6377d..fdd24dfbbed6 100644 --- a/Source/WebDriver/Session.cpp +++ b/Source/WebDriver/Session.cpp @@ -71,7 +71,7 @@ void Session::close(Function&& 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 toplevelBrowsingContext) +{ + m_toplevelBrowsingContext = toplevelBrowsingContext; + m_browsingContext = std::nullopt; +} + +void Session::switchToBrowsingContext(std::optional 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&& completionHandler) { ASSERT(!m_toplevelBrowsingContext.value()); @@ -101,7 +116,7 @@ void Session::createTopLevelBrowsingContext(Function&& 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&& 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&& 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&& 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&& 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&& frameID, FunctionisNull()) { - m_browsingContext = std::nullopt; + switchToBrowsingContext(std::nullopt); completionHandler(CommandResult::success()); return; } @@ -381,7 +396,7 @@ void Session::switchToFrame(RefPtr&& frameID, Function&& 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 1b3c29b525f9..19d585eff04e 100644 --- a/Source/WebDriver/Session.h +++ b/Source/WebDriver/Session.h @@ -104,6 +104,9 @@ public: private: Session(std::unique_ptr&&); + void switchToTopLevelBrowsingContext(std::optional); + void switchToBrowsingContext(std::optional); + RefPtr createElement(RefPtr&&); RefPtr createElement(const String& elementID); RefPtr extractElement(Inspector::InspectorValue&); -- 2.36.0