Bug 204618 - [GTK][WebInspector] Support for saving remote data
Summary: [GTK][WebInspector] Support for saving remote data
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
Depends on: 202894
  Show dependency treegraph
Reported: 2019-11-26 06:54 PST by Philippe Normand
Modified: 2019-12-18 04:49 PST (History)
3 users (show)

See Also:

Patch (3.19 KB, patch)
2019-11-26 06:59 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (6.54 KB, patch)
2019-12-18 04:32 PST, Philippe Normand
cgarcia: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2019-11-26 06:54:50 PST
+++ This bug was initially created as a clone of Bug #202894 +++

Implementation of RemoteWebInspectorProxy::platformSave() is missing.
Comment 1 Philippe Normand 2019-11-26 06:59:35 PST
Created attachment 384351 [details]
Comment 2 Carlos Garcia Campos 2019-11-26 07:24:02 PST
Comment on attachment 384351 [details]

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

> Source/WebKit/UIProcess/gtk/RemoteWebInspectorProxyGtk.cpp:127
> +        data = out.data();

You can't do this, out is going to be freed after the if, so data will point to freed data.

> Source/WebKit/UIProcess/gtk/RemoteWebInspectorProxyGtk.cpp:130
> +        data = content.utf8().data();

More ore less the same here, because the CString returned by utf8() is a temporary.

> Source/WebKit/UIProcess/gtk/RemoteWebInspectorProxyGtk.cpp:131
> +        dataLength = content.utf8().length();

And we don't want to convert to utf8 twice either.

> Source/WebKit/UIProcess/gtk/RemoteWebInspectorProxyGtk.cpp:137
> +    if (g_file_set_contents(path.get(), data, dataLength, nullptr))
> +        m_inspectorPage->process().send(Messages::RemoteWebInspectorUI::DidSave(path.get()), m_inspectorPage->webPageID());

You can use g_file_replace_contents_async(), instead of doing this synchronously.
Comment 3 Philippe Normand 2019-11-26 07:27:11 PST
I literally copied this from the non-remote implementation.
Comment 4 Carlos Garcia Campos 2019-11-26 08:25:12 PST
(In reply to Philippe Normand from comment #3)
> I literally copied this from the non-remote implementation.

I missed it there, let's fix that first then.
Comment 5 Philippe Normand 2019-12-18 04:32:43 PST
Created attachment 385957 [details]
Comment 6 Philippe Normand 2019-12-18 04:49:11 PST
Committed r253678: <https://trac.webkit.org/changeset/253678>