Bug 94623

Summary: [WK2] Make RegisterIntentServiceForFrame and DeliverIntentToFrame messages synchronous
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED INVALID    
Severity: Normal CC: andersca, gbillock, gyuyoung.kim, hausmann, kenneth, sam, tmpsantos, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://www.w3.org/TR/web-intents/#user-agent-behavior
Bug Depends on:    
Bug Blocks: 90451    
Attachments:
Description Flags
Patch sam: review-

Chris Dumez
Reported 2012-08-21 11:56:41 PDT
We currently test Web intent requests, Web intent service registration but not yet Web intent delivery. We need to test this functionality to fully cover Web Intents.
Attachments
Patch (9.04 KB, patch)
2012-08-22 03:05 PDT, Chris Dumez 		sam: review-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2012-08-22 01:41:27 PDT
According to the spec (http://www.w3.org/TR/web-intents/#user-agent-behavior): "When the User Agent delivers an intent payload to a web application, it must make the window.intent object available as the document is loaded and parsed, so that scripts on the page may process the intent data as they load. User agents must not place a window.intent object in the scope of pages which do not have registration metadata declaring themselves as intent handlers." We don't currently have a reliable way of delivering a Intent to a page before the "onload" event since the related messages are asynchronous in WebKit2. Since according to the spec, we not deliver the intent to the page until it declares itself as an intent handler, I propose to make the two following messages synchronous: RegisterIntentServiceForFrame: The page registered itself as an intent handler (i.e. the <intent> tag was parsed) DeliverIntentToFrame: Intent delivery to the page (exposing it via window.intent). This way, we have a reliable way of delivering the Intent *while* the page is loaded but *after* the page registered itself as an intent handler.
Chris Dumez
Comment 2 2012-08-22 03:05:51 PDT
Created attachment 159890 [details] Patch
Kenneth Rohde Christiansen
Comment 3 2012-08-22 03:27:29 PDT
Comment on attachment 159890 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159890&action=review > Source/WebKit2/ChangeLog:4 > + [WK2] Make RegisterIntentServiceForFrame and DeliverIntentToFrame messages synchronous > + https://bugs.webkit.org/show_bug.cgi?id=94623 We really would like to avoid sync calls during load. Is there any way the spec can be changed to accomplish this? > Source/WebKit2/ChangeLog:12 > + Web intent is delivered in time. According to the spec, > + the intent delivery (exposing it via window.intent) should > + happen as the document is loaded and parsed, so that scripts can't this be an event? > Source/WebKit2/UIProcess/API/efl/tests/resources/intent-service.html:1 > <html> html5 doctype? > Source/WebKit2/UIProcess/API/efl/tests/resources/intent-service.html:4 > +<script type="text/javascript"> no reason to specify that it is javascript
Simon Hausmann
Comment 4 2012-08-22 03:36:22 PDT
Comment on attachment 159890 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159890&action=review > Source/WebKit2/UIProcess/WebPageProxy.cpp:1641 > - m_process->send(Messages::WebPage::DeliverIntentToFrame(frame->frameID(), webIntentData->store()), m_pageID); > + m_process->sendSync(Messages::WebPage::DeliverIntentToFrame(frame->frameID(), webIntentData->store()), Messages::WebPage::DeliverIntentToFrame::Reply(), m_pageID); Sending a synchronous message from the ui process to the web process is asking for trouble and I suggest it should be avoided at all cost. We've had many problems/crashes doing synchronous calls in the Qt port going in that direction, when entering slightly more complex situations. So: Why exactly does the ui process have to block here?
Sam Weinig
Comment 5 2012-08-22 11:31:31 PDT
Comment on attachment 159890 [details] Patch Sync messages should be avoided. r-.
Note You need to log in before you can comment on or make changes to this bug.