Change WebApk app-id prefix from 'webapk:' to 'webapk-'

Change WebApk app-id prefix from 'webapk:' to 'webapk-'

This fixes some code which uses the host of a WebApp for various processing.
Specifically, when examining history state
(https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDirectoryManager.java?type=cs&q=liveWebapps.add+package:%5Echromium$&l=111)
this would fail because 'webapk:' becomes the host name.

This also makes WebApk id's consistent with traditional A2HS ids.
Finally, this includes a change to the cleanup routine run on WebApps to age out the old entries.
We will lose install source as part of this but it's just metrics book-keeping for us and doesn't affect users.
BUG=735060, 735722
Review-Url: https://codereview.chromium.org/2952023002
Cr-Commit-Position: refs/heads/master@{#482267}
Committed: https://chromium.googlesource.com/chromium/src/+/5ff213a88961f80fab0e21a871b62eb4336f3cb0

[Yaron, 2017-06-21 18:11:39]

The CQ bit was checked by yfriedman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-21 18:12:01]

Dry run: CQ is trying da patch.

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

[Yaron, 2017-06-21 18:29:30]

Description was changed from

==========
Change WebApk app-id prefix from 'webapk:' to 'webapk-'

This fixes some code which uses the host of a WebApp for various processing.
Specifically, when examining history state
(https://cs.corp.google.com/clankium/src/chrome/android/java/src/org/chromium/...)
this would fail because 'webapk:' becomes the host name.

This also makes WebApk id's consistent with traditional A2HS ids.
Finally, this includes a change to the cleanup routine run on WebApps to age out
the old entries.
We will lose install source as part of this but it's just metrics book-keeping
for us and doesn't affect users.
BUG=735060
==========

to

==========
Change WebApk app-id prefix from 'webapk:' to 'webapk-'

This fixes some code which uses the host of a WebApp for various processing.
Specifically, when examining history state
(https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...)
this would fail because 'webapk:' becomes the host name.

This also makes WebApk id's consistent with traditional A2HS ids.
Finally, this includes a change to the cleanup routine run on WebApps to age out
the old entries.
We will lose install source as part of this but it's just metrics book-keeping
for us and doesn't affect users.
BUG=735060
==========

[commit-bot: I haz the power, 2017-06-21 19:57:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-21 19:57:29]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Yaron, 2017-06-21 20:40:45]

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

[Yaron, 2017-06-21 20:40:45]

Thoughts?
As discussed on the bug, there's nothing too sensitive about losing the shared
prefs - namely the metrics of install disclosure and possibly the pref for the
unbound webapk disclosure but that hasn't launched yet so it's irrelevant.

That said, given that "-" is valid in a hostname and ":" isn't, I realize that
it does make it so we can confuse a webapp with the path of "webapk-foobar.com"
as a webapk when it's old A2HS. Perhaps that was the original motivation for
using ":" and I could instead special-case the linked to code to work with
WebApks. I just worry that we could have similar issues elsewhere if we continue
to work this way :/

[dominickn, 2017-06-22 00:50:02]

On 2017/06/21 20:40:45, Yaron wrote:
> Thoughts?
> As discussed on the bug, there's nothing too sensitive about losing the shared
> prefs - namely the metrics of install disclosure and possibly the pref for the
> unbound webapk disclosure but that hasn't launched yet so it's irrelevant.
> 
> That said, given that "-" is valid in a hostname and ":" isn't, I realize that
> it does make it so we can confuse a webapp with the path of
"webapk-foobar.com"
> as a webapk when it's old A2HS. Perhaps that was the original motivation for
> using ":" and I could instead special-case the linked to code to work with
> WebApks. I just worry that we could have similar issues elsewhere if we
continue
> to work this way :/

This broadly seems fine to me. I don't think it's possible to have a pre-WebAPK
PWA with a webApkPackage in its WebappDataStorage, so that shouldn't pass the
prefix check you have right?

[Yaron, 2017-06-22 14:15:34]

On 2017/06/22 00:50:02, dominickn wrote:
> On 2017/06/21 20:40:45, Yaron wrote:
> > Thoughts?
> > As discussed on the bug, there's nothing too sensitive about losing the
shared
> > prefs - namely the metrics of install disclosure and possibly the pref for
the
> > unbound webapk disclosure but that hasn't launched yet so it's irrelevant.
> > 
> > That said, given that "-" is valid in a hostname and ":" isn't, I realize
that
> > it does make it so we can confuse a webapp with the path of
> "webapk-foobar.com"
> > as a webapk when it's old A2HS. Perhaps that was the original motivation for
> > using ":" and I could instead special-case the linked to code to work with
> > WebApks. I just worry that we could have similar issues elsewhere if we
> continue
> > to work this way :/
> 
> This broadly seems fine to me. I don't think it's possible to have a
pre-WebAPK
> PWA with a webApkPackage in its WebappDataStorage, so that shouldn't pass the
> prefix check you have right?

My point was that you could have a collision in package namespace between for
example:
foobar.com (as webapk) and webapk-foobar.com (as A2HS). 
Of course this is fairly contrived, and shouldn't happen in practice. It also
doesn't affect security (not a risk of spoofing/fishing) so I think this is
fine.

[Yaron, 2017-06-22 14:15:53]

On 2017/06/22 14:15:34, Yaron wrote:
> On 2017/06/22 00:50:02, dominickn wrote:
> > On 2017/06/21 20:40:45, Yaron wrote:
> > > Thoughts?
> > > As discussed on the bug, there's nothing too sensitive about losing the
> shared
> > > prefs - namely the metrics of install disclosure and possibly the pref for
> the
> > > unbound webapk disclosure but that hasn't launched yet so it's irrelevant.
> > > 
> > > That said, given that "-" is valid in a hostname and ":" isn't, I realize
> that
> > > it does make it so we can confuse a webapp with the path of
> > "webapk-foobar.com"
> > > as a webapk when it's old A2HS. Perhaps that was the original motivation
for
> > > using ":" and I could instead special-case the linked to code to work with
> > > WebApks. I just worry that we could have similar issues elsewhere if we
> > continue
> > > to work this way :/
> > 
> > This broadly seems fine to me. I don't think it's possible to have a
> pre-WebAPK
> > PWA with a webApkPackage in its WebappDataStorage, so that shouldn't pass
the
> > prefix check you have right?
> 
> My point was that you could have a collision in package namespace between for
> example:
> http://foobar.com (as webapk) and http://webapk-foobar.com (as A2HS). 
> Of course this is fairly contrived, and shouldn't happen in practice. It also
> doesn't affect security (not a risk of spoofing/fishing) so I think this is
> fine.

err "package namespace" = "webapp id namespace"

[Yaron, 2017-06-22 19:09:38]

Description was changed from

==========
Change WebApk app-id prefix from 'webapk:' to 'webapk-'

This fixes some code which uses the host of a WebApp for various processing.
Specifically, when examining history state
(https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...)
this would fail because 'webapk:' becomes the host name.

This also makes WebApk id's consistent with traditional A2HS ids.
Finally, this includes a change to the cleanup routine run on WebApps to age out
the old entries.
We will lose install source as part of this but it's just metrics book-keeping
for us and doesn't affect users.
BUG=735060
==========

to

==========
Change WebApk app-id prefix from 'webapk:' to 'webapk-'

This fixes some code which uses the host of a WebApp for various processing.
Specifically, when examining history state
(https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...)
this would fail because 'webapk:' becomes the host name.

This also makes WebApk id's consistent with traditional A2HS ids.
Finally, this includes a change to the cleanup routine run on WebApps to age out
the old entries.
We will lose install source as part of this but it's just metrics book-keeping
for us and doesn't affect users.
BUG=735060,735722
==========

[Yaron, 2017-06-23 13:52:32]

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

[Yaron, 2017-06-23 13:52:33]

Still need a review on this - +Dan for webapp changes, Peter for webapk

[gone, 2017-06-23 16:59:08]

Not actually seeing anything that affects regular webapps.  Dominick's the right
reviewer for this now, though.

[pkotwicz, 2017-06-23 17:35:02]

LGTM!

[dominickn, 2017-06-26 00:21:44]

Sorry, didn't realise this was ready for a proper review!

lgtm, thanks

[Yaron, 2017-06-26 13:32:19]

The CQ bit was checked by yfriedman@chromium.org

[commit-bot: I haz the power, 2017-06-26 13:32:27]

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, 2017-06-26 14:21:35]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498483939379560,
"parent_rev": "4bd91b7809d8e61cd7ebfda075239270e53e8671", "commit_rev":
"5ff213a88961f80fab0e21a871b62eb4336f3cb0"}

[commit-bot: I haz the power, 2017-06-26 14:21:44]

Description was changed from

==========
Change WebApk app-id prefix from 'webapk:' to 'webapk-'

This fixes some code which uses the host of a WebApp for various processing.
Specifically, when examining history state
(https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...)
this would fail because 'webapk:' becomes the host name.

This also makes WebApk id's consistent with traditional A2HS ids.
Finally, this includes a change to the cleanup routine run on WebApps to age out
the old entries.
We will lose install source as part of this but it's just metrics book-keeping
for us and doesn't affect users.
BUG=735060,735722
==========

to

==========
Change WebApk app-id prefix from 'webapk:' to 'webapk-'

This fixes some code which uses the host of a WebApp for various processing.
Specifically, when examining history state
(https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...)
this would fail because 'webapk:' becomes the host name.

This also makes WebApk id's consistent with traditional A2HS ids.
Finally, this includes a change to the cleanup routine run on WebApps to age out
the old entries.
We will lose install source as part of this but it's just metrics book-keeping
for us and doesn't affect users.
BUG=735060,735722

Review-Url: https://codereview.chromium.org/2952023002
Cr-Commit-Position: refs/heads/master@{#482267}
Committed:
https://chromium.googlesource.com/chromium/src/+/5ff213a88961f80fab0e21a871b6...
==========

[commit-bot: I haz the power, 2017-06-26 14:21:46]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/5ff213a88961f80fab0e21a871b6...