Provide a prototype for AR QuickLook to trigger processing in the originating page
<rdar://54904781>
Created attachment 377776 [details] Patch
(In reply to Dean Jackson from comment #2) > Created attachment 377776 [details] > Patch Looks like iOS is broken.
Created attachment 377804 [details] Patch
Created attachment 377806 [details] Patch
Comment on attachment 377806 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377806&action=review > Source/WebKit/ChangeLog:12 > + Provide a way for a Web page to know if a particular action in the AR scene > + was performed, if and only if the system AR library calls a delegate with > + particular parameters. Post a message to the window on the main frame > + to indicate the action was triggered. Why the mainframe? Can a AR QuickLook be triggered only from a mainframe? > Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm:161 > + NSString *javaScriptString = [NSString stringWithFormat:@"window.postMessage({_origin: '_apple_ar_quicklook' , action: 'buttonTapped'}, window.location.origin);"]; > + _previewController->page().runJavaScriptInMainFrame(javaScriptString, true, [](API::SerializedScriptValue*, bool, const WebCore::ExceptionDetails&, WebKit::ScriptValueCallback::Error) { }); > + } Is there any chance the page has navigated since quicklook started? It would be good to either defend agains that or assert that it didn't happen.
Comment on attachment 377806 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377806&action=review > Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm:73 > +- (instancetype)initWithSystemPreviewController:(WebKit::SystemPreviewController*)previewController MIMEType:(NSString*)mimeType originatingPageURL:(URL)url Nit - do we ever expect the previewController to be null? If not, perhaps this should take a reference.
Comment on attachment 377806 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377806&action=review > Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm:158 > + if ([[message[@"callToAction"] description] isEqualToString:@"buttonTapped"]) { -description is generally used for debugging and might contain all kinds of junk (addresses, class names).
Created attachment 378606 [details] Patch
Comment on attachment 378606 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378606&action=review > Source/WebCore/html/HTMLAnchorElement.cpp:489 > + systemPreviewInfo.globalFrameID.frameID = document().frame()->loader().client().frameID().valueOr(FrameIdentifier { }); > + systemPreviewInfo.globalFrameID.pageID = document().frame()->loader().client().pageID().valueOr(PageIdentifier { }); Weird flex but OK. Not sure why the PageIdentifier and FrameIdentifiers aren't on Page and Frame. Chris? > Source/WebCore/loader/FrameLoaderTypes.h:185 > + IntRect previewRect; What coordinate system is this rect in? > Source/WebKit/UIProcess/SystemPreviewController.h:65 > + void triggerSystemPreviewActionWithTarget(uint64_t frameID, uint64_t pageID); Maybe put "ForTesting" in the name
Committed r249855: <https://trac.webkit.org/changeset/249855>
(In reply to Dean Jackson from comment #11) > Committed r249855: <https://trac.webkit.org/changeset/249855> TestWebKitAPI.WebKit.SystemPreviewTriggered (added with this change) is timing out on the iOS Simulator bots: https://build.webkit.org/builders/Apple%20iOS%2012%20Simulator%20Release%20WK2%20(Tests)/builds/6364
This breaks the iOS 13 OpenSource build...perhaps it needs <https://trac.webkit.org/changeset/249859/webkit>? Or an SPI declaration for ARQuickLookWebKitItemDelegate?
Reopening to attach new patch.
Created attachment 378969 [details] Patch
Committed r249958: <https://trac.webkit.org/changeset/249958>
Comment on attachment 378606 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378606&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:508 > +- (void)_triggerSystemPreviewActionOnFrame:(uint64_t)frameID page:(uint64_t)pageID WK_API_AVAILABLE(ios(13.0)); Should this be WK_IOS_TBA?