Bug 180368 - [Web App Manifest] Add SPI for applying a manifest to a top-level browsing context
Summary: [Web App Manifest] Add SPI for applying a manifest to a top-level browsing co...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-04 12:09 PST by David Quesada
Modified: 2017-12-06 19:30 PST (History)
4 users (show)

See Also:

Attachments
Patch (17.89 KB, patch)
2017-12-04 13:32 PST, David Quesada 		ggaren: review+
Details | Formatted Diff | Diff
Patch for landing (18.75 KB, patch)
2017-12-05 17:16 PST, David Quesada 		no flags Details | Formatted Diff | Diff
Patch for landing (v2) (18.78 KB, patch)
2017-12-06 14:58 PST, David Quesada 		no flags Details | Formatted Diff | Diff
Patch for landing (v3) (18.83 KB, patch)
2017-12-06 17:03 PST, David Quesada 		no flags Details | Formatted Diff | Diff
Patch for landing (v4) (18.79 KB, patch)
2017-12-06 17:07 PST, David Quesada 		no flags Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description David Quesada 2017-12-04 12:09:38 PST 
To run a web app as intended by the manifest, we need the ability to apply a manifest to the browsing context.
Comment 1 Radar WebKit Bug Importer 2017-12-04 12:10:11 PST 
<rdar://problem/35835664>
Comment 2 David Quesada 2017-12-04 12:10:57 PST 
Actually, I already had a radar for this one :/ rdar://problem/34748067
Comment 3 David Quesada 2017-12-04 13:32:54 PST 
Created attachment 328383 [details]
Patch

Patch for review. This depends on 180294.
Comment 4 Geoffrey Garen 2017-12-04 15:41:14 PST 
Comment on attachment 328383 [details]
Patch

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

r=me

> Source/WebCore/page/PageConfiguration.h:81
> +    std::optional<ApplicationManifest> applicationManifest { std::nullopt };

No need to specify nullopt here. The default constructor will do that for you.
Comment 5 David Quesada 2017-12-05 17:16:02 PST 
Created attachment 328532 [details]
Patch for landing
Comment 6 David Quesada 2017-12-06 14:58:53 PST 
Created attachment 328636 [details]
Patch for landing (v2)

Attempt to fix the EWS errors.
Comment 7 Joseph Pecoraro 2017-12-06 16:58:15 PST 
Comment on attachment 328636 [details]
Patch for landing (v2)

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

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfigurationPrivate.h:70
> +@property (nonatomic, setter=_setApplicationManifest:) _WKApplicationManifest *_applicationManifest;

This should have a API_AVAILABLE macro with TBA values. Certainly mac, maybe iOS?

    @property ... _WKApplicationManifest *_applicationManifest WK_API_AVAILABLE(macosx(WK_MAC_TBA));
Comment 8 David Quesada 2017-12-06 17:03:56 PST 
Created attachment 328655 [details]
Patch for landing (v3)

Now with API availability annotation
Comment 9 David Quesada 2017-12-06 17:07:11 PST 
Created attachment 328657 [details]
Patch for landing (v4)

Addressed one more piece of feedback from JoePeck: removed #if guards around a class forward declaration.
Comment 10 WebKit Commit Bot 2017-12-06 19:30:11 PST 
Comment on attachment 328657 [details]
Patch for landing (v4)

Clearing flags on attachment: 328657

Committed r225615: <https://trac.webkit.org/changeset/225615>
Comment 11 WebKit Commit Bot 2017-12-06 19:30:13 PST 
All reviewed patches have been landed.  Closing bug.