[webkit-reviews] review granted: [Bug 78491] [GTK] Implement WebUIClient's runOpenPanel in WebKit2GTK+ : [Attachment 138072] Patch Proposal + Unit Tests + Docs

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Mario Sanchez Prada
<msanchez at igalia.com>'s request for review:
Bug 78491: [GTK] Implement WebUIClient's runOpenPanel in WebKit2GTK+
https://bugs.webkit.org/show_bug.cgi?id=78491

Attachment 138072: Patch Proposal + Unit Tests + Docs
https://bugs.webkit.org/attachment.cgi?id=138072&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=138072&action=review


> Source/WebKit2/UIProcess/API/gtk/WebKitFileChooserRequest.cpp:336
> + * value will reflect whatever file are given.

whatever file are -> whatever files are

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebView.cpp:228
> +    static gboolean runFileChooser(WebKitWebView*, WebKitFileChooserRequest*
request, UIClientTest* test)
> +    {
> +	   test->assertObjectIsDeletedWhenTestFinishes(G_OBJECT(request));
> +	   test->m_fileChooserRequest = request;
> +	   g_main_loop_quit(test->m_mainLoop);
> +	   return TRUE;
> +    }
> +

I think it's good to keep separate unrelated test infrastructure. The
UIClientTest class is huge already and I think we should instead be trying to
be making it smaller and more modular. When landing this, do you think you can
create a subclass of UIClientTest called FileChooserTest? It will look
something like this:

class FileChooserTest: public UIClientTest {
public:
    MAKE_GLIB_TEST_FIXTURE(UIClientTest);

    FileChooserTest()
    {
	g_signal_connect(m_webView, "run-file-chooser",
G_CALLBACK(runFileChooser), this);
    }

     static gboolean runFileChooserCallback(WebKitWebView*,
WebKitFileChooserRequest* request, UIClientTest* test)
    {
	test->runFileChooser(request);
	return TRUE;
    }

    void runFileChooser(WebKitFileChooserRequest* request)
    {
	assertObjectIsDeletedWhenTestFinishes(G_OBJECT(request));
	m_fileChooserRequest = request;
	g_main_loop_quit(test->m_mainLoop);
   }

     WebKitFileChooserRequest* clickMouseButtonAndWaitForFileChooserRequest(int
x, int y)
    {
	clickMouseButton(x, y);
	g_main_loop_run(m_mainLoop);
	return m_fileChooserRequest.get();
    }

private:
    GRefPtr<WebKitFileChooserRequest> m_fileChooserRequest;
};