[webkit-reviews] review denied: [Bug 69870] [Chromium] Empty API for web intents : [Attachment 114381] Patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Greg Billock
<gbillock at google.com>'s request for review:
Bug 69870: [Chromium] Empty API for web intents
https://bugs.webkit.org/show_bug.cgi?id=69870

Attachment 114381: Patch
https://bugs.webkit.org/attachment.cgi?id=114381&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=114381&action=review


> Source/WebKit/chromium/public/WebIntentsController.h:42
> +    WEBKIT_EXPORT void webIntentReply(ReplyType, const WebString&, int);

this looks like it should just be a method on WebFrame.  it should probably
have a name that mirrors startActivity, like didCompleteActivity.  For bonus
points, the name should really say something about intents.

> Source/WebKit/chromium/public/WebViewClient.h:323
> +    virtual void registerIntentHandler(const WebString& action,

it seems like the request to register an intent handler should be relative to
the WebFrame so that the embedder can tell what origin is requesting the intent
handler.

it would probably be good to bundle up these request parameters into a separate
structure.

> Source/WebKit/chromium/public/WebViewClient.h:330
> +    virtual void startActivity(const WebString& action,

it seems like activities are started by script in the context of a document. 
therefore this should also be a WebFrame-relative method (i.e., it should be
declared on WebFrameClient).  perhaps the parameters should be bottled up into
a WebIntent structure (or some other structure)?

what is the intentId?  is that supposed to be passed back to WebKit as the
third parameter to webIntentReply?  is there really any need for an intent
identifier?  can there be multiple overlapping startActivity calls made by a
single script context?