[webkit-reviews] review requested: [Bug 118417] [GTK] FrameLoaderClient: Refactor naked pointers to use smart pointers : [Attachment 206157] Patch

Fri Jul 5 09:08:07 PDT 2013

Carlos Garcia Campos <cgarcia at igalia.com> has asked  for review:
Bug 118417: [GTK] FrameLoaderClient: Refactor naked pointers to use smart
pointers
https://bugs.webkit.org/show_bug.cgi?id=118417

Attachment 206157: Patch
https://bugs.webkit.org/attachment.cgi?id=206157&action=review

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

> Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:1083
> +    GOwnPtr<gchar> errorURI(g_filename_to_uri(errorPath.get(), 0, 0));
> +    GRefPtr<GFile> errorFile =
adoptGRef(g_file_new_for_uri(errorURI.get()));

I know the code was already that way, but I wondering why we need to convert
the path to uri instead of using g_file_new_for_path() directly, if it's
possible we could take advantage that you are changing this code to improve
this too.

> Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:1091
> +	   gboolean loaded = g_file_load_contents(errorFile.get(), 0,
&fileContent.outPtr(), 0, 0, 0);
>	   if (!loaded)
>	       content = makeString("<html><body>", webError->message,
"</body></html>");

You can probably do if (!g_file_load_contents(errorFile.get(), 0,
&fileContent.outPtr(), 0, 0, 0)) and you don't need the local variable that is
only used for this.