Mojo: teach launcher about mojo:// URLs.

Mojo: teach launcher about mojo:// URLs.

Now you can type, e.g., mojo://mojo_embedded_app/ffff00ff into the box
and get a purple square.

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

[Aaron Boodman, 2014-06-19 20:27:33]

https://codereview.chromium.org/345773003/diff/20001/mojo/examples/window_man...
File mojo/examples/window_manager/window_manager.cc (right):

https://codereview.chromium.org/345773003/diff/20001/mojo/examples/window_man...
mojo/examples/window_manager/window_manager.cc:116: app_url =
"mojo://mojo_embedded_app";
Both work, but this form is canonical.

[Ben Goodger (Google), 2014-06-19 21:09:47]

lgtm

https://codereview.chromium.org/345773003/diff/20001/mojo/examples/window_man...
File mojo/examples/window_manager/window_manager.cc (right):

https://codereview.chromium.org/345773003/diff/20001/mojo/examples/window_man...
mojo/examples/window_manager/window_manager.cc:156: nav_details->url =
requested_url;
redundant with response->url?

https://codereview.chromium.org/345773003/diff/20001/mojo/services/launcher/l...
File mojo/services/launcher/launcher.cc (right):

https://codereview.chromium.org/345773003/diff/20001/mojo/services/launcher/l...
mojo/services/launcher/launcher.cc:127: if (url.is_valid() &&
url.SchemeIs("mojo")) {
I think this is fine for now, but note that when a mojo URL is defined as
resolving to a HTTP URL, we'll have to fall back to the other branch. Really we
need the shell to expose its resolver perhaps.

[Aaron Boodman, 2014-06-19 21:25:12]

https://codereview.chromium.org/345773003/diff/20001/mojo/examples/window_man...
File mojo/examples/window_manager/window_manager.cc (right):

https://codereview.chromium.org/345773003/diff/20001/mojo/examples/window_man...
mojo/examples/window_manager/window_manager.cc:156: nav_details->url =
requested_url;
On 2014/06/19 21:09:47, Ben Goodger (Google) wrote:
> redundant with response->url?

No, because |response| might be null.

We could avoid needing this by changing the Launcher API to be like:

Launch(string url) => (foo, bar, baz)

That way, we could pass a closure for the callback containing the original URL.

But then the caller will need to be sure to check whether response->url is
present and prefer it to requested_url. I think it is safer to just have one
code path and have launcher always tell you the final URL you should be
navigating to.

https://codereview.chromium.org/345773003/diff/20001/mojo/services/launcher/l...
File mojo/services/launcher/launcher.cc (right):

https://codereview.chromium.org/345773003/diff/20001/mojo/services/launcher/l...
mojo/services/launcher/launcher.cc:127: if (url.is_valid() &&
url.SchemeIs("mojo")) {
On 2014/06/19 21:09:47, Ben Goodger (Google) wrote:
> I think this is fine for now, but note that when a mojo URL is defined as
> resolving to a HTTP URL, we'll have to fall back to the other branch. Really
we
> need the shell to expose its resolver perhaps.

Hm, ok.

I didn't realize/remember we wanted mojo URLs to fall back to http. Why is that?
I mean, why not just reference them directly at the HTTP URL in the first place?

[Ben Goodger (Google), 2014-06-19 21:29:56]

> But then the caller will need to be sure to check whether response->url is
> present and prefer it to requested_url. I think it is safer to just have one
> code path and have launcher always tell you the final URL you should be
> navigating to.

ok
> 
> Hm, ok.
> 
> I didn't realize/remember we wanted mojo URLs to fall back to http. Why is
that?
> I mean, why not just reference them directly at the HTTP URL in the first
place?

I think the idea was that they would serve as an alias for the default
implementation of Foo. I think this may be less important with Dave's changes in
which we decouple interface from application URL.

[Aaron Boodman, 2014-06-20 00:08:43]

The CQ bit was checked by aa@chromium.org

[commit-bot: I haz the power, 2014-06-20 00:11:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/aa@chromium.org/345773003/40001

[commit-bot: I haz the power, 2014-06-20 12:57:02]

Change committed as 278686

[darin (slow to review), 2014-06-20 15:12:37]

https://codereview.chromium.org/345773003/diff/40001/mojo/services/public/int...
File mojo/services/public/interfaces/launcher/launcher.mojom (right):

https://codereview.chromium.org/345773003/diff/40001/mojo/services/public/int...
mojo/services/public/interfaces/launcher/launcher.mojom:32: OnLaunch(string
handler_url, string view_url,
The final URL of the redirect chain is also stored in URLResponse.url. That
should always be the same as view_url, right?

[Aaron Boodman, 2014-06-20 16:09:32]

https://codereview.chromium.org/345773003/diff/40001/mojo/services/public/int...
File mojo/services/public/interfaces/launcher/launcher.mojom (right):

https://codereview.chromium.org/345773003/diff/40001/mojo/services/public/int...
mojo/services/public/interfaces/launcher/launcher.mojom:32: OnLaunch(string
handler_url, string view_url,
On 2014/06/20 15:12:37, darin wrote:
> The final URL of the redirect chain is also stored in URLResponse.url. That
> should always be the same as view_url, right?

In the case where a request was performed, yes. But a request might not be
performed, so I thought it was better to always pass this URL so clients could
use the API consistently.

I admit it's a bit awkward though. Suggestions welcome.

[darin (slow to review), 2014-06-20 16:14:35]

What is the case in which a request is not performed?
On Jun 20, 2014 9:09 AM, <aa@chromium.org> wrote:

>
> https://codereview.chromium.org/345773003/diff/40001/mojo/
> services/public/interfaces/launcher/launcher.mojom
> File mojo/services/public/interfaces/launcher/launcher.mojom (right):
>
> https://codereview.chromium.org/345773003/diff/40001/mojo/
> services/public/interfaces/launcher/launcher.mojom#newcode32
> mojo/services/public/interfaces/launcher/launcher.mojom:32:
> OnLaunch(string handler_url, string view_url,
> On 2014/06/20 15:12:37, darin wrote:
>
>> The final URL of the redirect chain is also stored in URLResponse.url.
>>
> That
>
>> should always be the same as view_url, right?
>>
>
> In the case where a request was performed, yes. But a request might not
> be performed, so I thought it was better to always pass this URL so
> clients could use the API consistently.
>
> I admit it's a bit awkward though. Suggestions welcome.
>
> https://codereview.chromium.org/345773003/
>

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

[Aaron Boodman, 2014-06-20 16:52:43]

Well one current case is mojo:// URLs.

But some future cases include:
- an app that has already been cached and can handle the destination URL
- app caches
- service workers


On Fri, Jun 20, 2014 at 9:14 AM, Darin Fisher <darin@chromium.org> wrote:

> What is the case in which a request is not performed?
> On Jun 20, 2014 9:09 AM, <aa@chromium.org> wrote:
>
>>
>> https://codereview.chromium.org/345773003/diff/40001/mojo/
>> services/public/interfaces/launcher/launcher.mojom
>> File mojo/services/public/interfaces/launcher/launcher.mojom (right):
>>
>> https://codereview.chromium.org/345773003/diff/40001/mojo/
>> services/public/interfaces/launcher/launcher.mojom#newcode32
>> mojo/services/public/interfaces/launcher/launcher.mojom:32:
>> OnLaunch(string handler_url, string view_url,
>> On 2014/06/20 15:12:37, darin wrote:
>>
>>> The final URL of the redirect chain is also stored in URLResponse.url.
>>>
>> That
>>
>>> should always be the same as view_url, right?
>>>
>>
>> In the case where a request was performed, yes. But a request might not
>> be performed, so I thought it was better to always pass this URL so
>> clients could use the API consistently.
>>
>> I admit it's a bit awkward though. Suggestions welcome.
>>
>> https://codereview.chromium.org/345773003/
>>
>

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-23 17:10:47]

On Fri, Jun 20, 2014 at 9:52 AM, Aaron Boodman <aa@chromium.org> wrote:

> Well one current case is mojo:// URLs.
>
> But some future cases include:
> - an app that has already been cached and can handle the destination URL
> - app caches
> - service workers
>

I don't immediately grok the implications for these.

-Darin



>
>
> On Fri, Jun 20, 2014 at 9:14 AM, Darin Fisher <darin@chromium.org> wrote:
>
>> What is the case in which a request is not performed?
>> On Jun 20, 2014 9:09 AM, <aa@chromium.org> wrote:
>>
>>>
>>> https://codereview.chromium.org/345773003/diff/40001/mojo/
>>> services/public/interfaces/launcher/launcher.mojom
>>> File mojo/services/public/interfaces/launcher/launcher.mojom (right):
>>>
>>> https://codereview.chromium.org/345773003/diff/40001/mojo/
>>> services/public/interfaces/launcher/launcher.mojom#newcode32
>>> mojo/services/public/interfaces/launcher/launcher.mojom:32:
>>> OnLaunch(string handler_url, string view_url,
>>> On 2014/06/20 15:12:37, darin wrote:
>>>
>>>> The final URL of the redirect chain is also stored in URLResponse.url.
>>>>
>>> That
>>>
>>>> should always be the same as view_url, right?
>>>>
>>>
>>> In the case where a request was performed, yes. But a request might not
>>> be performed, so I thought it was better to always pass this URL so
>>> clients could use the API consistently.
>>>
>>> I admit it's a bit awkward though. Suggestions welcome.
>>>
>>> https://codereview.chromium.org/345773003/
>>>
>>
>

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