[Webkit-unassigned] [Bug 94316] [GTK] Add webkit_uri_scheme_request_finish_error

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from Carlos Garcia Campos <cgarcia at igalia.com>  2013-04-10 01:48:06 PST ---
(From update of attachment 196962)
View in context: https://bugs.webkit.org/attachment.cgi?id=196962&action=review

Looks good to me. This is new API so you need another review from the API point of view, and also an owner should approve this.

> Source/WebKit2/UIProcess/API/gtk/tests/LoadTrackingTest.cpp:68
> +    test->m_error.clear();
> +    test->m_error.set(g_error_copy(error));

Don't need to clear before set.

> Source/WebKit2/UIProcess/API/gtk/tests/LoadTrackingTest.cpp:159
> +    m_error.clear();

This should be done in all other load methods.

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebContext.cpp:158
> +        if (equalIgnoringCase(scheme, "error")) {

Where does this came from? Why not using g_str_equal or g_strcmp0?

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebContext.cpp:161
> +            error.clear();

This is done automatically.

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebContext.cpp:225
> +    g_assert_cmpstr(test->m_error.get()->message, ==, errorMessage);

I think you can do test->m_error->message directly.

> Tools/MiniBrowser/gtk/main.c:223
> +        GError *error = g_error_new(g_quark_from_string("minibrowser"), 10, "Invalid about:%s page.", path);

This 10 looks arbitrary. You could define a global quark and error code.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.