Changeset 238174 in webkit


Ignore:
Timestamp:
Nov 14, 2018 7:50:05 AM (5 years ago)
Author:
Chris Dumez
Message:

WebKit.WKHTTPCookieStoreWithoutProcessPool API test is failing with process prewarming is enabled
https://bugs.webkit.org/show_bug.cgi?id=191624

Reviewed by Alex Christensen.

Source/WebKit:

Stop setting the WebProcessPool's primary data store (m_websiteDataStore) to the default one in
WebProcessPool::prewarmProcess(). We did not really need to, we can pass the default data store
to the new WebPageProxy without having to set m_websiteDataStore. m_websiteDataStore only gets
set upon constructor if thr default data store already exists or later on when creating a WebPage
that uses the default data store.

In the case of the API test, the following was happening:

  1. Create an ephemeral data store EDS
  2. Create a WebView V1 using datastore EDS
  3. Do a load in V1
  4. Process prewarming would kick in and wrongly associated V1's process pool PP1 with the default data store
  5. Create/Get the default datastore and set a few cookies on it
  6. Create a WebView V2 using default datastore and a fresh new process pool PP2
  7. Do a load in V2 and expect the cookies to be there

In HTTPCookieStore::setCookie() that is called at step 5, we call:
m_owningDataStore->processPoolForCookieStorageOperations()

In this case, m_owningDataStore is the default datastore and this call would previously return null because
there is no WebProcessPool yet associated with the default datastore. However, with the process prewarming
bug at step 4, the process pool PP1 would be returned since it was wrongly associated with the default
data store. As a result, we would call setCookie() on PP1's WebCookieManagerProxy and this would fail
because PP1's network process knows nothing about this session ID (it was only ever used with an ephemeral
session).

  • UIProcess/WebProcessPool.cpp:

(WebKit::WebProcessPool::prewarmProcess):

Tools:

Add API test coverage.

  • TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:

(runWKHTTPCookieStoreWithoutProcessPool):
(TEST):

Location:
trunk
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebKit/ChangeLog

    r238166 r238174  
     12018-11-14  Chris Dumez  <cdumez@apple.com>
     2
     3        WebKit.WKHTTPCookieStoreWithoutProcessPool API test is failing with process prewarming is enabled
     4        https://bugs.webkit.org/show_bug.cgi?id=191624
     5
     6        Reviewed by Alex Christensen.
     7
     8        Stop setting the WebProcessPool's primary data store (m_websiteDataStore) to the default one in
     9        WebProcessPool::prewarmProcess(). We did not really need to, we can pass the default data store
     10        to the new WebPageProxy without having to set m_websiteDataStore. m_websiteDataStore only gets
     11        set upon constructor if thr default data store already exists or later on when creating a WebPage
     12        that uses the default data store.
     13
     14        In the case of the API test, the following was happening:
     15        1. Create an ephemeral data store EDS
     16        2. Create a WebView V1 using datastore EDS
     17        3. Do a load in V1
     18        4. Process prewarming would kick in and wrongly associated V1's process pool PP1 with the default data store
     19        5. Create/Get the default datastore and set a few cookies on it
     20        6. Create a WebView V2 using default datastore and a fresh new process pool PP2
     21        7. Do a load in V2 and expect the cookies to be there
     22
     23        In HTTPCookieStore::setCookie() that is called at step 5, we call:
     24        m_owningDataStore->processPoolForCookieStorageOperations()
     25
     26        In this case, m_owningDataStore is the default datastore and this call would previously return null because
     27        there is no WebProcessPool yet associated with the default datastore. However, with the process prewarming
     28        bug at step 4, the process pool PP1 would be returned since it was wrongly associated with the default
     29        data store. As a result, we would call setCookie() on PP1's WebCookieManagerProxy and this would fail
     30        because PP1's network process knows nothing about this session ID (it was only ever used with an ephemeral
     31        session).
     32
     33        * UIProcess/WebProcessPool.cpp:
     34        (WebKit::WebProcessPool::prewarmProcess):
     35
    1362018-11-13  Jiewen Tan  <jiewen_tan@apple.com>
    237
  • trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

    r238061 r238174  
    956956        return;
    957957
    958     if (!m_websiteDataStore)
    959         m_websiteDataStore = API::WebsiteDataStore::defaultDataStore().ptr();
     958    auto* websiteDataStore = m_websiteDataStore.get();
     959    if (!websiteDataStore)
     960        websiteDataStore = API::WebsiteDataStore::defaultDataStore().ptr();
    960961
    961962    RELEASE_LOG(PerformanceLogging, "Prewarming a WebProcess for performance");
    962     createNewWebProcess(m_websiteDataStore->websiteDataStore(), WebProcessProxy::IsPrewarmed::Yes);
     963    createNewWebProcess(websiteDataStore->websiteDataStore(), WebProcessProxy::IsPrewarmed::Yes);
    963964}
    964965
  • trunk/Tools/ChangeLog

    r238166 r238174  
     12018-11-14  Chris Dumez  <cdumez@apple.com>
     2
     3        WebKit.WKHTTPCookieStoreWithoutProcessPool API test is failing with process prewarming is enabled
     4        https://bugs.webkit.org/show_bug.cgi?id=191624
     5
     6        Reviewed by Alex Christensen.
     7
     8        Add API test coverage.
     9
     10        * TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:
     11        (runWKHTTPCookieStoreWithoutProcessPool):
     12        (TEST):
     13
    1142018-11-13  Jiewen Tan  <jiewen_tan@apple.com>
    215
  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm

    r237607 r238174  
    3232#import <WebKit/WKProcessPoolPrivate.h>
    3333#import <WebKit/WKWebsiteDataStorePrivate.h>
     34#import <WebKit/_WKProcessPoolConfiguration.h>
    3435#import <WebKit/_WKWebsiteDataStoreConfiguration.h>
    3536#import <wtf/ProcessPrivilege.h>
     
    479480// FIXME: on iOS, UI process should be using the same cookie file as the network process for default session.
    480481#if PLATFORM(MAC)
    481 TEST(WebKit, WKHTTPCookieStoreWithoutProcessPool)
     482enum class ShouldEnableProcessPrewarming { No, Yes };
     483void runWKHTTPCookieStoreWithoutProcessPool(ShouldEnableProcessPrewarming shouldEnableProcessPrewarming)
    482484{
    483485    RetainPtr<NSHTTPCookie> sessionCookie = [NSHTTPCookie cookieWithProperties:@{
     
    517519    }];
    518520    TestWebKitAPI::Util::run(&finished);
    519 
    520     finished = false;
     521    finished = false;
     522
     523    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
     524    processPoolConfiguration.get().prewarmsProcessesAutomatically = shouldEnableProcessPrewarming == ShouldEnableProcessPrewarming::Yes;
     525    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
     526
    521527    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
    522528    configuration.get().websiteDataStore = ephemeralStoreWithCookies.get();
     529    [configuration setProcessPool:processPool.get()];
    523530    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
    524531    auto delegate = adoptNS([[CookieUIDelegate alloc] init]);
     
    570577    TestWebKitAPI::Util::run(&finished);
    571578}
     579
     580TEST(WebKit, WKHTTPCookieStoreWithoutProcessPoolWithoutPrewarming)
     581{
     582    runWKHTTPCookieStoreWithoutProcessPool(ShouldEnableProcessPrewarming::No);
     583}
     584
     585TEST(WebKit, WKHTTPCookieStoreWithoutProcessPoolWithPrewarming)
     586{
     587    runWKHTTPCookieStoreWithoutProcessPool(ShouldEnableProcessPrewarming::Yes);
     588}
     589
    572590#endif // PLATFORM(MAC)
    573591#endif
Note: See TracChangeset for help on using the changeset viewer.