Make WebAPK use "start URL longest path" as the scope if the scope is missing

Make WebAPK use "start URL longest path" as the scope if the scope is missing

This makes WebAPKs and Webapps use the same logic to determine the scope if the
scope is not provided by the Web Manifest

BUG=624563
Committed: https://crrev.com/47136bc34e3cc1ce64d23d9c47580283c3102624
Cr-Commit-Position: refs/heads/master@{#410273}

[pkotwicz, 2016-08-03 03:55:09]

pkotwicz@chromium.org changed reviewers:
+ dominickn@chromium.org

[pkotwicz, 2016-08-03 03:55:09]

dominickn@ can you please take a look? This CL depends on
https://codereview.chromium.org/2138973002/

In some cases the call to ShortcutHelper#getScopeFromStartUrl() is a bit odd
because ShortcutHelper#EXTRA_URL may contain a URL other than the start URL. I
haven't found any cases where this matters.

[dominickn, 2016-08-03 04:03:34]

Looks fine at first glance, but I'm curious why you renamed the method (that
seems to be the bulk of this CL)? getScopeFromStartUrl seems like an unnecessary
specification (why shouldn't people use a non-start URL?).

[pkotwicz, 2016-08-03 13:50:38]

getScopeFromUrl() returns a different result for
https://tests.peter.sh//notification-generator/
and
https://tests.peter.sh//notification-generator/easter_egg/easter_egg.html

WebappDataStorage#updateFromShortcutIntent() always updates the scope stored in
WebappDataStorage whenever the WebappActivity is launched.
If EXTRA_SCOPE is not specified, the scope stored in WebappDataStorage should
not be updated each time that the webapp is launched

Thankfully, ServiceTabLauncher#launchTab()
- is the only place which specifies an EXTRA_URL != start_url
- specifies the EXTRA_SCOPE

[dominickn, 2016-08-04 00:11:54]

On 2016/08/03 13:50:38, pkotwicz wrote:
> getScopeFromUrl() returns a different result for
> https://tests.peter.sh//notification-generator/
> and
> https://tests.peter.sh//notification-generator/easter_egg/easter_egg.html
> 
> WebappDataStorage#updateFromShortcutIntent() always updates the scope stored
in
> WebappDataStorage whenever the WebappActivity is launched.
> If EXTRA_SCOPE is not specified, the scope stored in WebappDataStorage should
> not be updated each time that the webapp is launched
> 
> Thankfully, ServiceTabLauncher#launchTab()
> - is the only place which specifies an EXTRA_URL != start_url
> - specifies the EXTRA_SCOPE

This makes sense, but getScopeFromStartUrl() also returns a different result for
those URLs - and in a number of the callsites, it's passed a parameter called
|url| like you mentioned. So I don't think it makes sense to change the name of
the method because it's the responsibility of the callsites to use the method on
start URLs, there's nothing in the method itself which is specific to a start
URL.

I don't want to block on this, so lgtm. But I'd prefer the name to not change.
:)

[pkotwicz, 2016-08-04 01:06:08]

pkotwicz@chromium.org changed reviewers:
+ dfalcantara@chromium.org, hanxi@chromium.org

[pkotwicz, 2016-08-04 01:06:08]

Xi can you please take a look at the change in webapk_installer.cc
dfalcantara@: Can you please take a look at the change in ShortcutHelper.java (I
think that dominickn@ is an OWNER of all of the other files)

I switched the function name back to ShortcutHelper#getScopeFromUrl()

[pkotwicz, 2016-08-04 01:06:32]

Patchset #2 (id:20001) has been deleted

[gone, 2016-08-04 16:42:12]

lgtm

[Xi Han, 2016-08-05 14:29:21]

lgtm

[pkotwicz, 2016-08-06 23:21:39]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2016-08-06 23:21:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, hanxi@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2199413003/#ps60001
(title: "Merge branch 'master' into webapk_builder_impl2_scope")

[commit-bot: I haz the power, 2016-08-06 23:21:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-06 23:55:58]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-08-06 23:57:28]

Description was changed from

==========
Make WebAPK use "start URL longest path" as the scope if the scope is missing

This makes WebAPKs and Webapps use the same logic to determine the scope if the
scope is not provided by the Web Manifest

BUG=624563
==========

to

==========
Make WebAPK use "start URL longest path" as the scope if the scope is missing

This makes WebAPKs and Webapps use the same logic to determine the scope if the
scope is not provided by the Web Manifest

BUG=624563

Committed: https://crrev.com/47136bc34e3cc1ce64d23d9c47580283c3102624
Cr-Commit-Position: refs/heads/master@{#410273}
==========

[commit-bot: I haz the power, 2016-08-06 23:57:29]

Patchset 3 (id:??) landed as
https://crrev.com/47136bc34e3cc1ce64d23d9c47580283c3102624
Cr-Commit-Position: refs/heads/master@{#410273}