Rename WebIntentData for backend storage to WebIntentServiceData.

Rename WebIntentData for backend storage to WebIntentServiceData.
Change startActivity IPC to use the WebIntentData struct.

R=jhawkins@chromium.org
BUG=None
TEST=None

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=102069

[Greg Billock, 2011-09-16 23:43:27]

On 2011/09/16 23:39:00, Greg Billock wrote:

This change ended up touching quite a bit of surface area. There's still a bit
of lingering terminology confusion about "Intents" vs. Services I think. I made
a start on cleaning it up a bit here, but there's more.

I switched to WebIntentServiceData rather than Registration since much of the
time it's used it isn't in a registration context, but rather figuring out what
services are available. I'm not convinced we need a filter class, so I haven't
added that.

[groby-ooo-7-16, 2011-09-16 23:49:49]

Drive-by comment :)

Very much agreeing on the terminology issue. I'm still occasionally confused by
it :)

http://codereview.chromium.org/7930002/diff/1/chrome/browser/intents/web_inte...
File chrome/browser/intents/web_intent_service_data.h (right):

http://codereview.chromium.org/7930002/diff/1/chrome/browser/intents/web_inte...
chrome/browser/intents/web_intent_service_data.h:14: // TODO(gbillock): if we
want to use this in an IPC message, it should
Since we're also using it in browser/common, I'm wondering if it belongs there.

[groby-ooo-7-16, 2011-09-20 17:55:07]

LGTM - unless you want to move WebIntentServiceData to webkit_glue in the same
changelist?

http://codereview.chromium.org/7930002/diff/1/chrome/browser/intents/web_inte...
File chrome/browser/intents/web_intent_service_data.h (right):

http://codereview.chromium.org/7930002/diff/1/chrome/browser/intents/web_inte...
chrome/browser/intents/web_intent_service_data.h:14: // TODO(gbillock): if we
want to use this in an IPC message, it should
On 2011/09/16 23:49:49, groby wrote:
> Since we're also using it in browser/common, I'm wondering if it belongs
there. 

I'm done wondering :) It definitely belongs into webkit/glue

[Greg Billock, 2011-09-20 18:20:33]

Included a couple pointers to the IPC message move.

http://codereview.chromium.org/7930002/diff/1/chrome/browser/intents/web_inte...
File chrome/browser/intents/web_intent_service_data.h (right):

http://codereview.chromium.org/7930002/diff/1/chrome/browser/intents/web_inte...
chrome/browser/intents/web_intent_service_data.h:14: // TODO(gbillock): if we
want to use this in an IPC message, it should
On 2011/09/20 17:55:07, groby wrote:
> On 2011/09/16 23:49:49, groby wrote:
> > Since we're also using it in browser/common, I'm wondering if it belongs
> there. 
> 
> I'm done wondering :) It definitely belongs into webkit/glue
> 

Agreed. Let's do that in a follow-on.

http://codereview.chromium.org/7930002/diff/1/content/browser/tab_contents/ta...
File content/browser/tab_contents/tab_contents.cc (right):

http://codereview.chromium.org/7930002/diff/1/content/browser/tab_contents/ta...
content/browser/tab_contents/tab_contents.cc:282:
IPC_MESSAGE_HANDLER(IntentsHostMsg_WebIntentDispatch,
Everything else here is ViewHostMsg, which makes this look like it doesn't
belong. I considered putting it in a delegate TabContentsObserver, but that's a
pain because it's so thin, and the TabContentsDelegate doesn't get any message
traffic.

The downside of keeping the message a ViewHostMsg is just that it uses the
struct in intents_messages, so it makes sense to reside there. Is that just
overthinking this?

http://codereview.chromium.org/7930002/diff/1/content/common/intents_messages.h
File content/common/intents_messages.h (right):

http://codereview.chromium.org/7930002/diff/1/content/common/intents_messages...
content/common/intents_messages.h:51:
IPC_MESSAGE_ROUTED2(IntentsHostMsg_WebIntentDispatch,
Reminder: Have a look here and changed the signature a bit. Look OK?

[Greg Billock, 2011-09-20 18:46:48]

+jam for content/* John, can you comment on the IPC message move to
intents_messages? It seems suspiciously likely to be a bad idea, but less icky
than alternatives (at least, the ones I thought of :-)).

On 2011/09/20 18:20:33, Greg Billock wrote:
> Included a couple pointers to the IPC message move.
> 
>
http://codereview.chromium.org/7930002/diff/1/chrome/browser/intents/web_inte...
> File chrome/browser/intents/web_intent_service_data.h (right):
> 
>
http://codereview.chromium.org/7930002/diff/1/chrome/browser/intents/web_inte...
> chrome/browser/intents/web_intent_service_data.h:14: // TODO(gbillock): if we
> want to use this in an IPC message, it should
> On 2011/09/20 17:55:07, groby wrote:
> > On 2011/09/16 23:49:49, groby wrote:
> > > Since we're also using it in browser/common, I'm wondering if it belongs
> > there. 
> > 
> > I'm done wondering :) It definitely belongs into webkit/glue
> > 
> 
> Agreed. Let's do that in a follow-on.
> 
>
http://codereview.chromium.org/7930002/diff/1/content/browser/tab_contents/ta...
> File content/browser/tab_contents/tab_contents.cc (right):
> 
>
http://codereview.chromium.org/7930002/diff/1/content/browser/tab_contents/ta...
> content/browser/tab_contents/tab_contents.cc:282:
> IPC_MESSAGE_HANDLER(IntentsHostMsg_WebIntentDispatch,
> Everything else here is ViewHostMsg, which makes this look like it doesn't
> belong. I considered putting it in a delegate TabContentsObserver, but that's
a
> pain because it's so thin, and the TabContentsDelegate doesn't get any message
> traffic.
> 
> The downside of keeping the message a ViewHostMsg is just that it uses the
> struct in intents_messages, so it makes sense to reside there. Is that just
> overthinking this?
> 
>
http://codereview.chromium.org/7930002/diff/1/content/common/intents_messages.h
> File content/common/intents_messages.h (right):
> 
>
http://codereview.chromium.org/7930002/diff/1/content/common/intents_messages...
> content/common/intents_messages.h:51:
> IPC_MESSAGE_ROUTED2(IntentsHostMsg_WebIntentDispatch,
> Reminder: Have a look here and changed the signature a bit. Look OK?

[jam, 2011-09-20 23:10:22]

it's fine to have TabContents dispatch non ViewHostMsgs. In fact, I'd also move
ViewHostMsg_RegisterIntentHandler to intents_messages.h so that all the
intent-related IPCs are together.

[Greg Billock, 2011-09-20 23:16:29]

Yeah, I plan on doing that in a follow-up change. (There's a class to move
to webkit/glue and that surgery as well...)

Can you approve for content/*? Thanks!

On Tue, Sep 20, 2011 at 4:10 PM, <jam@chromium.org> wrote:

> it's fine to have TabContents dispatch non ViewHostMsgs. In fact, I'd also
> move
> ViewHostMsg_**RegisterIntentHandler to intents_messages.h so that all the
> intent-related IPCs are together.
>
>
>
http://codereview.chromium.**org/7930002/<http://codereview.chromium.org/7930...
>

[jam, 2011-09-20 23:42:16]

content lgtm

http://codereview.chromium.org/7930002/diff/1/content/renderer/render_view.cc
File content/renderer/render_view.cc (right):

http://codereview.chromium.org/7930002/diff/1/content/renderer/render_view.cc...
content/renderer/render_view.cc:4511: RenderThread::current()->Send(new
IntentsHostMsg_WebIntentDispatch(
nit: no need for "RenderThread::current()->"

[Greg Billock, 2011-09-21 00:25:57]

http://codereview.chromium.org/7930002/diff/1/content/renderer/render_view.cc
File content/renderer/render_view.cc (right):

http://codereview.chromium.org/7930002/diff/1/content/renderer/render_view.cc...
content/renderer/render_view.cc:4511: RenderThread::current()->Send(new
IntentsHostMsg_WebIntentDispatch(
On 2011/09/20 23:42:16, John Abd-El-Malek wrote:
> nit: no need for "RenderThread::current()->"

Done.

[commit-bot: I haz the power, 2011-09-21 01:49:18]

Can't apply patch for file chrome/browser/intents/web_intent_service_data.cc.
While running patch -p1 --forward --force;
patching file chrome/browser/intents/web_intent_service_data.cc
Hunk #1 FAILED at 3.
Hunk #2 FAILED at 21.
2 out of 2 hunks FAILED -- saving rejects to file
chrome/browser/intents/web_intent_service_data.cc.rej

[groby-ooo-7-16, 2011-09-21 17:36:22]

On 2011/09/21 01:49:18, I haz the power (commit-bot) wrote:
> Can't apply patch for file chrome/browser/intents/web_intent_service_data.cc.
> While running patch -p1 --forward --force;
> patching file chrome/browser/intents/web_intent_service_data.cc
> Hunk #1 FAILED at 3.
> Hunk #2 FAILED at 21.
> 2 out of 2 hunks FAILED -- saving rejects to file
> chrome/browser/intents/web_intent_service_data.cc.rej

Since Greg is out for this week, hijacking this patch. Will post new issue ASAP