Bug 191602 - WebKit.ApplicationManifestBasic API test is failing when enabling PSON
Summary: WebKit.ApplicationManifestBasic API test is failing when enabling PSON
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks: 191572
  Show dependency treegraph
 
Reported: 2018-11-13 14:56 PST by Chris Dumez
Modified: 2018-11-14 08:59 PST (History)
6 users (show)

See Also:


Attachments
WIP Patch (8.10 KB, patch)
2018-11-13 15:51 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (11.06 KB, patch)
2018-11-13 16:02 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (13.31 KB, patch)
2018-11-13 18:56 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (13.34 KB, patch)
2018-11-14 08:56 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2018-11-13 14:56:41 PST
WebKit.ApplicationManifestBasic API test is failing when enabling PSON:
        /Volumes/Data/WebKit/OpenSource/Tools/TestWebKitAPI/Tests/WebKitCocoa/ApplicationManifest.mm:98
        Value of: [manifest.name isEqualToString:@"A Web Application"]
          Actual: false
        Expected: true
        
        
        /Volumes/Data/WebKit/OpenSource/Tools/TestWebKitAPI/Tests/WebKitCocoa/ApplicationManifest.mm:99
        Value of: [manifest.shortName isEqualToString:@"WebApp"]
          Actual: false
        Expected: true
        
        
        /Volumes/Data/WebKit/OpenSource/Tools/TestWebKitAPI/Tests/WebKitCocoa/ApplicationManifest.mm:100
        Value of: [manifest.applicationDescription isEqualToString:@"Hello."]
          Actual: false
        Expected: true
        
        
        /Volumes/Data/WebKit/OpenSource/Tools/TestWebKitAPI/Tests/WebKitCocoa/ApplicationManifest.mm:101
        Value of: [manifest.startURL isEqual:[NSURL URLWithString:@"http://example.com/app/start"]]
          Actual: false
        Expected: true
        
        
        /Volumes/Data/WebKit/OpenSource/Tools/TestWebKitAPI/Tests/WebKitCocoa/ApplicationManifest.mm:102
        Value of: [manifest.scope isEqual:[NSURL URLWithString:@"http://example.com/app"]]
          Actual: false
        Expected: true


The issue seems to be that we're process swapping when the client does:
    NSString *htmlString = [NSString stringWithFormat:@"<link rel=\"manifest\" href=\"data:text/plain;charset=utf-8;base64,%@\">", [[NSJSONSerialization dataWithJSONObject:manifestObject options:0 error:nil] base64EncodedStringWithOptions:0]];
    [webView loadHTMLString:htmlString baseURL:[NSURL URLWithString:@"http://example.com/app/index"]];

Could be related to substitute data and it getting dropped if the load swaps to a new process.
Comment 1 Chris Dumez 2018-11-13 15:51:25 PST
Created attachment 354716 [details]
WIP Patch
Comment 2 Chris Dumez 2018-11-13 16:02:03 PST
Created attachment 354720 [details]
Patch
Comment 3 Chris Dumez 2018-11-13 18:56:40 PST
Created attachment 354749 [details]
Patch
Comment 4 Alex Christensen 2018-11-14 06:20:34 PST
Comment on attachment 354749 [details]
Patch

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

> Source/WebKit/UIProcess/API/APINavigation.h:159
> +    std::optional<SubstituteData> m_substituteData;

Since it is uncommon it might be a little better to use std::unique_ptr so we don't have to always use the memory of the whole structure.

> Source/WebKit/UIProcess/WebNavigationState.h:54
> +    Ref<API::Navigation> createLoadDataNavigation(const IPC::DataReference&, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData);

We already have a struct.  Couldn't we just add one parameter here?
Comment 5 Chris Dumez 2018-11-14 08:56:42 PST
Created attachment 354814 [details]
Patch
Comment 6 Chris Dumez 2018-11-14 08:58:23 PST
Comment on attachment 354814 [details]
Patch

Clearing flags on attachment: 354814

Committed r238179: <https://trac.webkit.org/changeset/238179>
Comment 7 Chris Dumez 2018-11-14 08:58:25 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2018-11-14 08:59:23 PST
<rdar://problem/46064330>