[webkit-reviews] review granted: [Bug 89873] [WK2][GTK] Implement new API to save a web page using MHTML : [Attachment 157435] Patch Proposal + Unit Tests + Documentation

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted Mario Sanchez Prada
<msanchez at igalia.com>'s request for review:
Bug 89873: [WK2][GTK] Implement new API to save a web page using MHTML
https://bugs.webkit.org/show_bug.cgi?id=89873

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

------- Additional Comments from Carlos Garcia Campos <cgarcia at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=157435&action=review


> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:2458
> + * specified in #WebKitSaveMode.

specified in @save_mode, no?

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebView.cpp:956
> +    SaveWebViewTest()
> +    {
> +	   m_tempDirectory.set(g_dir_make_tmp("WebKit2SaveViewTest-XXXXXX",
0));

C++ initializer syntax:

SaveWebViewTest()
    : m_tempDirectory(g_dir_make_tmp("WebKit2SaveViewTest-XXXXXX", 0))

or isn't that possible with GOwnPtr?

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebView.cpp:962
> +	   g_file_delete(m_file.get(), 0, 0);

Don't assume saveAndWaitForFile() has been called, even thought it's the case
now, this class might be used in the future for other tests.

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebView.cpp:964
> +	   g_input_stream_close(m_inputStream.get(), 0, 0);

Ditto.

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebView.cpp:1037
> +    // Check that the file exists and that it contains at least the same
amount of bytes.

the same amount of bytes than the stream?