Eliminate Launchable. Use Navigator instead.

Eliminate Launchable. Use Navigator instead.

Adds URLResponse & body stream fields to NavigationDetails struct.

** not tested as part of the main chrome integration test suite **

R=aa@chromium.org
BUG=none

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278243

[Ben Goodger (Google), 2014-06-17 05:41:17]

Hey Aaron I got rid of my Launchable interface and used Navigator instead. I
added some fields to NavigationDetails to supply the needed data. 

I'm still thinking Launcher is probably a separate interface for the time being
as it handles the content type resolution/embedding.

WDYT?

[Ben Goodger (Google), 2014-06-17 06:20:21]

https://codereview.chromium.org/331323007/diff/20001/mojo/services/navigation...
File mojo/services/navigation/navigation.mojom (right):

https://codereview.chromium.org/331323007/diff/20001/mojo/services/navigation...
mojo/services/navigation/navigation.mojom:11: mojo.URLResponse response;
I think maybe these two fields should go on a nested struct that could be NULL
in the cases where Navigate was not called as the result of a fetch. I don't
know that it makes sense for them to be individually NULL. If we do this, then
if we get the nullability support in mojom this nested struct would be nullable.

[Aaron Boodman, 2014-06-18 17:22:46]

Was on vacay. Back now, looking...

[Aaron Boodman, 2014-06-18 17:33:00]

lgtm w/ nit

https://codereview.chromium.org/331323007/diff/40001/mojo/services/navigation...
File mojo/services/navigation/navigation.mojom (right):

https://codereview.chromium.org/331323007/diff/40001/mojo/services/navigation...
mojo/services/navigation/navigation.mojom:11: mojo.URLResponse response;
I think that these should go in a separate struct, in a separate param.
NavigationDetails will eventually have other information related to the
navigation (disposition, etc). But it will always be present.

I believe that all the request stuff together either will or won't be present,
depending on whether request happened (you can verify with Darin in person if he
is handy).

Also, we will eventually want the URLRequest as well, maybe include it now.

[Aaron Boodman, 2014-06-18 17:35:07]

So like:

Navigate(uint32 node_id, NavigationDetails details, nullable RequestDetails
request);

Also, please update the comment to explain when |request| will be null.

thanks!

[Ben Goodger (Google), 2014-06-19 02:42:22]

Committed patchset #4 manually as r278243 (tree was closed).

[Ben Goodger (Google), 2014-06-19 02:50:01]

Where I ended up with this was a ResponseDetails struct and a TODO to
obtain RequestDetails, since the URLLoader doesn't stash them anywhere
convenient after the load is initiated.

-Ben


On Wed, Jun 18, 2014 at 7:42 PM, <ben@chromium.org> wrote:

> Committed patchset #4 manually as r278243 (tree was closed).
>
> https://codereview.chromium.org/331323007/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[darin (slow to review), 2014-06-19 02:57:01]

Do you wish URLLoader API was different?
On Jun 18, 2014 7:50 PM, "Ben Goodger (Google)" <ben@chromium.org> wrote:

> Where I ended up with this was a ResponseDetails struct and a TODO to
> obtain RequestDetails, since the URLLoader doesn't stash them anywhere
> convenient after the load is initiated.
>
> -Ben
>
>
> On Wed, Jun 18, 2014 at 7:42 PM, <ben@chromium.org> wrote:
>
>> Committed patchset #4 manually as r278243 (tree was closed).
>>
>> https://codereview.chromium.org/331323007/
>>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.