Bug 52434 – [GTK] Convert use of raw pointers to GOwnPtr in FileSystemGtk.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Joone Hur]

Some functions use GOwnPtr, but others still use gchar & char pointers.

[Joone Hur]

Created attachment 78907 [details]
Proposed Patch

[Joone Hur]

Created attachment 78915 [details]
Proposed Patch2

Sorry, I have submitted the patch again..

[David Levin]

Comment on attachment 78915 [details]
Proposed Patch2

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

Now that I had a second chance to look at it. I think it could be slightly better since you are making this change. Sorry for not noticing last time.

> Source/WebCore/platform/gtk/FileSystemGtk.cpp:51
>      return string;

I wonder why you don't just do
   return String(escapedString.get());

And the same for the other three places where you did a change.

[Joone Hur]

Created attachment 78923 [details]
Proposed Patch3

Thanks for the review.

[Martin Robinson]

Thanks for the cleanup, Joone!

[Joone Hur]

(In reply to comment #5)
> Thanks for the cleanup, Joone!

You're welcome :-)

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 78923 [details]:

The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 78923 [details]
Proposed Patch3

Clearing flags on attachment: 78923

Committed r75797: <http://trac.webkit.org/changeset/75797>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[David Levin]

(In reply to comment #7)
> The commit-queue encountered the following flaky tests while processing attachment 78923 [details]:
> 
> The commit-queue is continuing to process your patch.

Eric, something went wrong here ^^^.

[Eric Seidel (no email)]

I think the python tests flaked and it got confused by that:
https://webkit-commit-queue.appspot.com/results/7492048

We need to disable that python test.  We've known it's been flaky for some time.

[Dirk Pranke]

(In reply to comment #11)
> I think the python tests flaked and it got confused by that:
> https://webkit-commit-queue.appspot.com/results/7492048
> 
> We need to disable that python test.  We've known it's been flaky for some time.

Perhaps a better solution would be to fix that test? :)