Bug 104894 – [GTK] Remove deprecated API usage

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

[Claudio Saavedra]

GTK+ and soup have deprecated a few functions.

[Claudio Saavedra]

Created attachment 179245 [details]
2012-12-13  Claudio Saavedra  <[e-mail]>

[GTK] Remove deprecated API usage
        https://bugs.webkit.org/show_bug.cgi?id=104894

        Reviewed by NOBODY (OOPS!).

        Remove deprecated GTK+ API usage.

        * platform/gtk/RenderThemeGtk3.cpp:
        (WebCore::spinButtonArrowSize): Do not use deprecated
        gtk_style_context_get_font().

[Claudio Saavedra]

Created attachment 179246 [details]
        [GTK] Remove deprecated API usage         https://bugs.webkit.org/show_bug.cgi?id=104894

Reviewed by NOBODY (OOPS!).

        Remove deprecated libsoup API usage.

        * platform/network/soup/ResourceHandleSoup.cpp:
        (WebCore::ensureSessionIsInitialized): No need
        to add the requester feature.
        (WebCore::createSoupRequestAndMessageForHandle): SoupSession
        has a requester API, use it.

[Sergio Villar Senin]

(In reply to comment #2)
> Created an attachment (id=179246) [details]
>         [GTK] Remove deprecated API usage         https://bugs.webkit.org/show_bug.cgi?id=104894
> 
> Reviewed by NOBODY (OOPS!).
> 
>         Remove deprecated libsoup API usage.
> 
>         * platform/network/soup/ResourceHandleSoup.cpp:
>         (WebCore::ensureSessionIsInitialized): No need
>         to add the requester feature.
>         (WebCore::createSoupRequestAndMessageForHandle): SoupSession
>         has a requester API, use it.

LGTM but we have to bump the libsoup dependency as well if we integrate this.

[Claudio Saavedra]

Created attachment 179271 [details]
[GTK] Remove deprecated API usage         https://bugs.webkit.org/show_bug.cgi?id=104894

Reviewed by NOBODY (OOPS!).

        Remove deprecated libsoup API usage.

        * platform/network/soup/ResourceHandleSoup.cpp:
        (WebCore::ensureSessionIsInitialized): No need
        to add the requester feature.
        (WebCore::createSoupRequestAndMessageForHandle): SoupSession
        has a requester API, use it.

[Martin Robinson]

Comment on attachment 179271 [details]
[GTK] Remove deprecated API usage         https://bugs.webkit.org/show_bug.cgi?id=104894

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

> Source/WebCore/ChangeLog:24
>  
> +        Remove deprecated libsoup API usage.
> +
> +        * platform/network/soup/ResourceHandleSoup.cpp:
> +        (WebCore::ensureSessionIsInitialized): No need
> +        to add the requester feature.
> +        (WebCore::createSoupRequestAndMessageForHandle): SoupSession
> +        has a requester API, use it.
> +
> [phone]  Claudio Saavedra  <[e-mail]>
> +
> +        [GTK] Remove deprecated API usage
> +        https://bugs.webkit.org/show_bug.cgi?id=104894
> +
> +        Reviewed by NOBODY (OOPS!).
> +
>          Remove deprecated GTK+ API usage.
>  

Your changelog here is in the middle of the file so it could possibly make the bots sick if we land this via the commit queue.

[Claudio Saavedra]

Created attachment 179277 [details]
[GTK] Remove deprecated API usage         https://bugs.webkit.org/show_bug.cgi?id=104894

Reviewed by NOBODY (OOPS!).

        Remove deprecated libsoup API usage.

        * platform/network/soup/ResourceHandleSoup.cpp:
        (WebCore::ensureSessionIsInitialized): No need
        to add the requester feature.
        (WebCore::createSoupRequestAndMessageForHandle): SoupSession
        has a requester API, use it.

https://bugs.webkit.org/show_bug.cgi?id=104896

[Claudio Saavedra]

Comment on attachment 179277 [details]
[GTK] Remove deprecated API usage         https://bugs.webkit.org/show_bug.cgi?id=104894

Fixed that.

[WebKit Review Bot]

Comment on attachment 179245 [details]
2012-12-13  Claudio Saavedra  <[e-mail]>

Clearing flags on attachment: 179245

Committed r137602: <http://trac.webkit.org/changeset/137602>

[Claudio Saavedra]

So we've agreed with the EFL guys to hold on the libsoup patch until there's a tarball.

[EFL EWS Bot]

Comment on attachment 179277 [details]
[GTK] Remove deprecated API usage         https://bugs.webkit.org/show_bug.cgi?id=104894

Attachment 179277 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15309357

[Manuel Rego Casasnovas]

Created attachment 196449 [details]
Patch

Since libsoup version has been bumped in r147547 it seems that now this patch could be applied at the end. I'm uploading a new patch as the deprecated API is used from WK2 so we should remove it too. Moreover the configure.ac stuff seems not needed now.

[EFL EWS Bot]

Comment on attachment 196449 [details]
Patch

Attachment 196449 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17470039

[Dan Winship]

Comment on attachment 196449 [details]
Patch

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

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:58
>  #define LIBSOUP_USE_UNSTABLE_REQUEST_API
>  #include <libsoup/soup-multipart-input-stream.h>

Once GTK and EFL both depend on 2.42, you should not need to define LIBSOUP_USE_UNSTABLE_REQUEST_API anywhere, and you should not need to include any libsoup headers other than <libsoup/soup.h> anywhere.

> Source/WebKit2/WebProcess/soup/WebSoupRequestManager.cpp:-107
> -    soup_session_feature_add_feature(SOUP_SESSION_FEATURE(requester.get()), WEBKIT_TYPE_SOUP_REQUEST_GENERIC);

You don't want to remove this, you just want to change it to

  soup_session_add_feature_by_type (session, WEBKIT_TYPE_SOUP_REQUEST_GENERIC);

[Build Bot]

Comment on attachment 196449 [details]
Patch

Attachment 196449 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17519134

[Manuel Rego Casasnovas]

Created attachment 196593 [details]
Patch

[EFL EWS Bot]

Comment on attachment 196593 [details]
Patch

Attachment 196593 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17438170

[Sergio Villar Senin]

Comment on attachment 196593 [details]
Patch

The patch LGTM but you would need to update EFL's jhbuild dependencies.

[Manuel Rego Casasnovas]

(In reply to comment #17)
> (From update of attachment 196593 [details])
> The patch LGTM but you would need to update EFL's jhbuild dependencies.

Yes, we already talked about that and the patch for bumping the EFL dependencies has already r+ (see bug #113927).

So, we're waiting for r+ here and then we should land both patches manually one after the other to avoid breaking EFL port.

[Manuel Rego Casasnovas]

Created attachment 198035 [details]
Patch

Rebased patch as it doesn't apply in current trunk, due to a small conflict in WebKit2/WebProcess/soup/WebProcessSoup.cpp.

[EFL EWS Bot]

Comment on attachment 198035 [details]
Patch

Attachment 198035 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/161046

[Philippe Normand]

Comment on attachment 198035 [details]
Patch

Clearing flags on attachment: 198035

Committed r148507: <http://trac.webkit.org/changeset/148507>

[Philippe Normand]

All reviewed patches have been landed.  Closing bug.