[Webkit-unassigned] [Bug 180398] WebDriver: implement maximize, minimize and fullscreen window commands

[bugzilla-daemon at webkit.org]

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

Michael Catanzaro <mcatanzaro at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mcatanzaro at igalia.com

--- Comment #8 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 340956
  --> https://bugs.webkit.org/attachment.cgi?id=340956
Try to fix iOS builds

View in context: https://bugs.webkit.org/attachment.cgi?id=340956&action=review

The bottom portions of webkitWebViewMaximizeWindow and webkitWebViewRestoreWindow feel pretty crowded, I would add some more blank lines in there.

> Source/WebKit/UIProcess/API/glib/WebKitAutomationSession.cpp:117
> +    void requestMaximizeWindowOfPage(WebAutomationSession&, WebPageProxy& page, CompletionHandler<void()>&& completionHandler) override
> +    {
> +        auto* webView = webkitWebContextGetWebViewForPage(m_session->priv->webContext, &page);
> +        if (!webView)
> +            completionHandler();
> +        webkitWebViewMaximizeWindow(webView, WTFMove(completionHandler));
> +    }
> +
> +    void requestHideWindowOfPage(WebAutomationSession&, WebPageProxy& page, CompletionHandler<void()>&& completionHandler) override
> +    {
> +        auto* webView = webkitWebContextGetWebViewForPage(m_session->priv->webContext, &page);
> +        if (!webView)
> +            completionHandler();
> +        webkitWebViewMinimizeWindow(webView, WTFMove(completionHandler));
> +    }
> +
> +    void requestRestoreWindowOfPage(WebAutomationSession&, WebPageProxy& page, CompletionHandler<void()>&& completionHandler) override
> +    {
> +        auto* webView = webkitWebContextGetWebViewForPage(m_session->priv->webContext, &page);
> +        if (!webView)
> +            completionHandler();
> +        webkitWebViewRestoreWindow(webView, WTFMove(completionHandler));
> +    }

These all look wrong to me, shouldn't it be something like:

if (auto* webView = webkitWebContextGetWebViewForPage(m_session->priv->webContext, &page))
    webkitWebViewMaximizeWindow(webView, WTFMove(completionHandler));
else
    completionHandler();

Otherwise you're calling WebKitWebView methods with a NULL webview, and the completion handler could be called twice.

r=me on the rest of the GTK bits.

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewGtk.cpp:117
> +        // Complete the event if not done after 1 second.

Nit: "one second."

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewGtk.cpp:134
> +    Type type;
> +    CompletionHandler<void()> completionHandler;
> +    RunLoop::Timer<WindowStateEvent> completeTimer;

How about some m_ for these?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180523/f36e3e48/attachment.html>