Bug 191638 – WebKit.WebsiteDataStoreCustomPaths API test is failing when enabling process prewarming

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

WebKit.WebsiteDataStoreCustomPaths API test is failing when enabling process prewarming.

[Chris Dumez]

Created attachment 354824 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 354824 [details]
Patch

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

> Tools/ChangeLog:13
> +        However, process prewarming constructs the default data store and passes it to the
> +        new WebProcessProxy if WebProcessPool::m_websiteDataStore is null.

Hm... wouldn't this affect existing apps that would pre-warm processes?

[Chris Dumez]

(In reply to Ryosuke Niwa from comment #2)
> Comment on attachment 354824 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=354824&action=review
> 
> > Tools/ChangeLog:13
> > +        However, process prewarming constructs the default data store and passes it to the
> > +        new WebProcessProxy if WebProcessPool::m_websiteDataStore is null.
> 
> Hm... wouldn't this affect existing apps that would pre-warm processes?

What's the big deal about creating the default data store if it does not exist yet?

[Brady Eidson]

(In reply to Chris Dumez from comment #3)
> (In reply to Ryosuke Niwa from comment #2)
> > Comment on attachment 354824 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=354824&action=review
> > 
> > > Tools/ChangeLog:13
> > > +        However, process prewarming constructs the default data store and passes it to the
> > > +        new WebProcessProxy if WebProcessPool::m_websiteDataStore is null.
> > 
> > Hm... wouldn't this affect existing apps that would pre-warm processes?
> 
> What's the big deal about creating the default data store if it does not
> exist yet?

You can't do it because it's a huge no-no on iOS.

[Chris Dumez]

Created attachment 354862 [details]
Patch

[Chris Dumez]

Ok, I implemented an alternative approach which makes sure process prewarming tries to find a suitable data store but never creates the default one when it does not exist.

[WebKit Commit Bot]

Comment on attachment 354862 [details]
Patch

Clearing flags on attachment: 354862

Committed r238215: <https://trac.webkit.org/changeset/238215>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/46084619>

[Truitt Savell]

Looks like the changes in https://trac.webkit.org/changeset/238215/webkit

build:
https://trac.webkit.org/changeset/238215/webkit

has caused two API failures:

TestWebKitAPI.WKProcessPool.InitialWarmedProcessUsed
        
        /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessPreWarming.mm:62
        Value of: [pool _hasPrewarmedWebProcess]
          Actual: false
        Expected: true
        
        
        /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessPreWarming.mm:63
        Expected equality of these values:
          1U
            Which is: 1
          [pool _webPageContentProcessCount]
            Which is: 0
        

    TestWebKitAPI.WKProcessPool.WarmInitialProcess
        
        /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessPreWarming.mm:47
        Value of: [pool _hasPrewarmedWebProcess]
          Actual: false
        Expected: true

[Chris Dumez]

(In reply to Truitt Savell from comment #10)
> Looks like the changes in https://trac.webkit.org/changeset/238215/webkit
> 
> build:
> https://trac.webkit.org/changeset/238215/webkit
> 
> has caused two API failures:
> 
> TestWebKitAPI.WKProcessPool.InitialWarmedProcessUsed
>         
>        
> /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/
> WebKitCocoa/ProcessPreWarming.mm:62
>         Value of: [pool _hasPrewarmedWebProcess]
>           Actual: false
>         Expected: true
>         
>         
>        
> /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/
> WebKitCocoa/ProcessPreWarming.mm:63
>         Expected equality of these values:
>           1U
>             Which is: 1
>           [pool _webPageContentProcessCount]
>             Which is: 0
>         
> 
>     TestWebKitAPI.WKProcessPool.WarmInitialProcess
>         
>        
> /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/
> WebKitCocoa/ProcessPreWarming.mm:47
>         Value of: [pool _hasPrewarmedWebProcess]
>           Actual: false
>         Expected: true

Will look now, thanks. I believe the test just needs updating.

[Chris Dumez]

(In reply to Chris Dumez from comment #11)
> (In reply to Truitt Savell from comment #10)
> > Looks like the changes in https://trac.webkit.org/changeset/238215/webkit
> > 
> > build:
> > https://trac.webkit.org/changeset/238215/webkit
> > 
> > has caused two API failures:
> > 
> > TestWebKitAPI.WKProcessPool.InitialWarmedProcessUsed
> >         
> >        
> > /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/
> > WebKitCocoa/ProcessPreWarming.mm:62
> >         Value of: [pool _hasPrewarmedWebProcess]
> >           Actual: false
> >         Expected: true
> >         
> >         
> >        
> > /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/
> > WebKitCocoa/ProcessPreWarming.mm:63
> >         Expected equality of these values:
> >           1U
> >             Which is: 1
> >           [pool _webPageContentProcessCount]
> >             Which is: 0
> >         
> > 
> >     TestWebKitAPI.WKProcessPool.WarmInitialProcess
> >         
> >        
> > /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/
> > WebKitCocoa/ProcessPreWarming.mm:47
> >         Value of: [pool _hasPrewarmedWebProcess]
> >           Actual: false
> >         Expected: true
> 
> Will look now, thanks. I believe the test just needs updating.

https://bugs.webkit.org/show_bug.cgi?id=191693 Patch coming shortly.