Changeset 207388 in webkit


Ignore:
Timestamp:
Oct 16, 2016 1:35:18 AM (7 years ago)
Author:
Carlos Garcia Campos
Message:

Document request not updated after willSendRequest is called for a redirect
https://bugs.webkit.org/show_bug.cgi?id=163436

Reviewed by Michael Catanzaro.

Source/WebCore:

The first willSendRequest happens before DocumentLoader::startLoadingMainResource(), that calls setRequest, but
the second one happens after DocumentLoader::redirectReceived() and then the request is never updated again.

Covered by GTK+ unit tests.

  • loader/DocumentLoader.cpp:

(WebCore::DocumentLoader::willContinueMainResourceLoadAfterRedirect): Set the new request.

  • loader/DocumentLoader.h:
  • loader/SubresourceLoader.cpp:

(WebCore::SubresourceLoader::willSendRequestInternal): Notify the document loader when loading the main resource
and called for a redirection.

Tools:

Update /webkit2/WebKitWebView/active-uri test to check the active URI also when modified by
WebKitPage::send-request signal in a web extension.

  • TestWebKitAPI/Tests/WebKit2Gtk/TestLoaderClient.cpp:

(testWebViewActiveURI):
(serverCallback):

  • TestWebKitAPI/Tests/WebKit2Gtk/WebExtensionTest.cpp:

(sendRequestCallback):

  • TestWebKitAPI/gtk/WebKit2Gtk/LoadTrackingTest.cpp:

(loadChangedCallback):

Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r207386 r207388  
     12016-10-16  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        Document request not updated after willSendRequest is called for a redirect
     4        https://bugs.webkit.org/show_bug.cgi?id=163436
     5
     6        Reviewed by Michael Catanzaro.
     7
     8        The first willSendRequest happens before DocumentLoader::startLoadingMainResource(), that calls setRequest, but
     9        the second one happens after DocumentLoader::redirectReceived() and then the request is never updated again.
     10
     11        Covered by GTK+ unit tests.
     12
     13        * loader/DocumentLoader.cpp:
     14        (WebCore::DocumentLoader::willContinueMainResourceLoadAfterRedirect): Set the new request.
     15        * loader/DocumentLoader.h:
     16        * loader/SubresourceLoader.cpp:
     17        (WebCore::SubresourceLoader::willSendRequestInternal): Notify the document loader when loading the main resource
     18        and called for a redirection.
     19
    1202016-10-15  Said Abou-Hallawa  <sabouhallawa@apple.com>
    221
  • trunk/Source/WebCore/loader/DocumentLoader.cpp

    r207325 r207388  
    16301630}
    16311631
     1632void DocumentLoader::willContinueMainResourceLoadAfterRedirect(const ResourceRequest& newRequest)
     1633{
     1634    setRequest(newRequest);
     1635}
     1636
    16321637void DocumentLoader::clearMainResource()
    16331638{
  • trunk/Source/WebCore/loader/DocumentLoader.h

    r207151 r207388  
    214214        void startLoadingMainResource();
    215215        WEBCORE_EXPORT void cancelMainResourceLoad(const ResourceError&);
    216        
     216        void willContinueMainResourceLoadAfterRedirect(const ResourceRequest&);
     217
    217218        // Support iconDatabase in synchronous mode.
    218219        void iconLoadDecisionAvailable();
  • trunk/Source/WebCore/loader/SubresourceLoader.cpp

    r206869 r207388  
    230230
    231231    ResourceLoader::willSendRequestInternal(newRequest, redirectResponse);
    232     if (newRequest.isNull())
     232    if (newRequest.isNull()) {
    233233        cancel();
     234        return;
     235    }
     236
     237    if (m_resource->type() == CachedResource::MainResource && !redirectResponse.isNull())
     238        m_documentLoader->willContinueMainResourceLoadAfterRedirect(newRequest);
    234239}
    235240
  • trunk/Tools/ChangeLog

    r207385 r207388  
     12016-10-16  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        Document request not updated after willSendRequest is called for a redirect
     4        https://bugs.webkit.org/show_bug.cgi?id=163436
     5
     6        Reviewed by Michael Catanzaro.
     7
     8        Update /webkit2/WebKitWebView/active-uri test to check the active URI also when modified by
     9        WebKitPage::send-request signal in a web extension.
     10
     11        * TestWebKitAPI/Tests/WebKit2Gtk/TestLoaderClient.cpp:
     12        (testWebViewActiveURI):
     13        (serverCallback):
     14        * TestWebKitAPI/Tests/WebKit2Gtk/WebExtensionTest.cpp:
     15        (sendRequestCallback):
     16        * TestWebKitAPI/gtk/WebKit2Gtk/LoadTrackingTest.cpp:
     17        (loadChangedCallback):
     18
    1192016-10-15  Dan Bernstein  <mitz@apple.com>
    220
  • trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestLoaderClient.cpp

    r194480 r207388  
    2828#include <gtk/gtk.h>
    2929#include <libsoup/soup.h>
     30#include <wtf/Vector.h>
    3031#include <wtf/text/CString.h>
    3132
     
    210211    static void uriChanged(GObject*, GParamSpec*, ViewURITrackingTest* test)
    211212    {
    212         g_assert_cmpstr(test->m_activeURI.data(), !=, webkit_web_view_get_uri(test->m_webView));
    213         test->m_activeURI = webkit_web_view_get_uri(test->m_webView);
     213        g_assert_cmpstr(test->m_currentURI.data(), !=, webkit_web_view_get_uri(test->m_webView));
     214        test->m_currentURI = webkit_web_view_get_uri(test->m_webView);
    214215    }
    215216
    216217    ViewURITrackingTest()
    217         : m_activeURI(webkit_web_view_get_uri(m_webView))
    218     {
    219         g_assert(m_activeURI.isNull());
     218        : m_currentURI(webkit_web_view_get_uri(m_webView))
     219    {
     220        g_assert(m_currentURI.isNull());
     221        m_currentURIList.grow(m_currentURIList.capacity());
    220222        g_signal_connect(m_webView, "notify::uri", G_CALLBACK(uriChanged), this);
    221223    }
    222224
     225    enum State { Provisional, ProvisionalAfterRedirect, Commited, Finished };
     226
     227    void loadURI(const char* uri)
     228    {
     229        reset();
     230        LoadTrackingTest::loadURI(uri);
     231    }
     232
    223233    void provisionalLoadStarted()
    224234    {
    225         checkActiveURI("/redirect");
     235        m_currentURIList[Provisional] = m_currentURI;
    226236    }
    227237
    228238    void provisionalLoadReceivedServerRedirect()
    229239    {
    230         checkActiveURI("/normal");
     240        m_currentURIList[ProvisionalAfterRedirect] = m_currentURI;
    231241    }
    232242
    233243    void loadCommitted()
    234244    {
    235         checkActiveURI("/normal");
     245        m_currentURIList[Commited] = m_currentURI;
    236246    }
    237247
    238248    void loadFinished()
    239249    {
    240         checkActiveURI("/normal");
     250        m_currentURIList[Finished] = m_currentURI;
    241251        LoadTrackingTest::loadFinished();
    242252    }
    243253
    244     CString m_activeURI;
     254    void checkURIAtState(State state, const char* path)
     255    {
     256        if (path)
     257            ASSERT_CMP_CSTRING(m_currentURIList[state], ==, kServer->getURIForPath(path));
     258        else
     259            g_assert(m_currentURIList[state].isNull());
     260    }
    245261
    246262private:
    247     void checkActiveURI(const char* uri)
    248     {
    249         ASSERT_CMP_CSTRING(m_activeURI, ==, kServer->getURIForPath(uri));
    250     }
     263    void reset()
     264    {
     265        m_currentURI = CString();
     266        m_currentURIList.clear();
     267        m_currentURIList.grow(m_currentURIList.capacity());
     268    }
     269
     270    CString m_currentURI;
     271    Vector<CString, 4> m_currentURIList;
    251272};
    252273
    253274static void testWebViewActiveURI(ViewURITrackingTest* test, gconstpointer)
    254275{
     276    // Normal load, the URL doesn't change.
     277    test->loadURI(kServer->getURIForPath("/normal1").data());
     278    test->waitUntilLoadFinished();
     279    test->checkURIAtState(ViewURITrackingTest::State::Provisional, "/normal1");
     280    test->checkURIAtState(ViewURITrackingTest::State::ProvisionalAfterRedirect, nullptr);
     281    test->checkURIAtState(ViewURITrackingTest::State::Commited, "/normal1");
     282    test->checkURIAtState(ViewURITrackingTest::State::Finished, "/normal1");
     283
     284    // Redirect, the URL changes after the redirect.
    255285    test->loadURI(kServer->getURIForPath("/redirect").data());
    256286    test->waitUntilLoadFinished();
     287    test->checkURIAtState(ViewURITrackingTest::State::Provisional, "/redirect");
     288    test->checkURIAtState(ViewURITrackingTest::State::ProvisionalAfterRedirect, "/normal");
     289    test->checkURIAtState(ViewURITrackingTest::State::Commited, "/normal");
     290    test->checkURIAtState(ViewURITrackingTest::State::Finished, "/normal");
     291
     292    // Normal load, URL changed by WebKitPage::send-request.
     293    test->loadURI(kServer->getURIForPath("/normal-change-request").data());
     294    test->waitUntilLoadFinished();
     295    test->checkURIAtState(ViewURITrackingTest::State::Provisional, "/normal-change-request");
     296    test->checkURIAtState(ViewURITrackingTest::State::ProvisionalAfterRedirect, nullptr);
     297    test->checkURIAtState(ViewURITrackingTest::State::Commited, "/request-changed");
     298    test->checkURIAtState(ViewURITrackingTest::State::Finished, "/request-changed");
     299
     300    // Redirect, URL changed by WebKitPage::send-request.
     301    test->loadURI(kServer->getURIForPath("/redirect-to-change-request").data());
     302    test->waitUntilLoadFinished();
     303    test->checkURIAtState(ViewURITrackingTest::State::Provisional, "/redirect-to-change-request");
     304    test->checkURIAtState(ViewURITrackingTest::State::ProvisionalAfterRedirect, "/normal-change-request");
     305    test->checkURIAtState(ViewURITrackingTest::State::Commited, "/request-changed-on-redirect");
     306    test->checkURIAtState(ViewURITrackingTest::State::Finished, "/request-changed-on-redirect");
    257307}
    258308
     
    485535        soup_message_set_status(message, SOUP_STATUS_MOVED_PERMANENTLY);
    486536        soup_message_headers_append(message->response_headers, "Location", "/normal");
     537    } else if (g_str_equal(path, "/redirect-to-change-request")) {
     538        soup_message_set_status(message, SOUP_STATUS_MOVED_PERMANENTLY);
     539        soup_message_headers_append(message->response_headers, "Location", "/normal-change-request");
    487540    } else if (g_str_equal(path, "/cancelled")) {
    488541        soup_message_headers_set_encoding(message->response_headers, SOUP_ENCODING_CHUNKED);
  • trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/WebExtensionTest.cpp

    r205540 r207388  
    155155        g_assert(headers);
    156156        soup_message_headers_append(headers, "DNT", "1");
     157    } else if (g_str_has_suffix(requestURI, "/normal-change-request")) {
     158        GUniquePtr<char> prefix(g_strndup(requestURI, strlen(requestURI) - strlen("/normal-change-request")));
     159        GUniquePtr<char> newURI(g_strdup_printf("%s/request-changed%s", prefix.get(), redirectResponse ? "-on-redirect" : ""));
     160        webkit_uri_request_set_uri(request, newURI.get());
    157161    } else if (g_str_has_suffix(requestURI, "/http-get-method")) {
    158162        g_assert_cmpstr(webkit_uri_request_get_http_method(request), ==, "GET");
  • trunk/Tools/TestWebKitAPI/gtk/WebKit2Gtk/LoadTrackingTest.cpp

    r193815 r207388  
    4040    case WEBKIT_LOAD_COMMITTED: {
    4141        g_assert(webkit_web_view_is_loading(webView));
    42         g_assert_cmpstr(test->m_activeURI.data(), ==, webkit_web_view_get_uri(webView));
     42        test->m_activeURI = webkit_web_view_get_uri(webView);
    4343
    4444        // Check that on committed we always have a main resource with a response.
Note: See TracChangeset for help on using the changeset viewer.