Bug 94623 - [WK2] Make RegisterIntentServiceForFrame and DeliverIntentToFrame messages synchronous
Summary: [WK2] Make RegisterIntentServiceForFrame and DeliverIntentToFrame messages sy...
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL: http://www.w3.org/TR/web-intents/#use...
Keywords:
Depends on:
Blocks: 90451
  Show dependency treegraph
 
Reported: 2012-08-21 11:56 PDT by Chris Dumez
Modified: 2012-10-08 01:38 PDT (History)
8 users (show)

See Also:


Attachments
Patch (9.04 KB, patch)
2012-08-22 03:05 PDT, Chris Dumez
sam: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 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.
Comment 1 Chris Dumez 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.
Comment 2 Chris Dumez 2012-08-22 03:05:51 PDT
Created attachment 159890 [details]
Patch
Comment 3 Kenneth Rohde Christiansen 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
Comment 4 Simon Hausmann 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?
Comment 5 Sam Weinig 2012-08-22 11:31:31 PDT
Comment on attachment 159890 [details]
Patch

Sync messages should be avoided. r-.