Simplify caller checking logic in WebApkServiceImpl

Simplify caller checking logic in WebApkServiceImpl

This CL passes the uid of the host browser instead of the package name of the
host browser. This simplifies things because:
- Several package names can have the same UID.
- There is only one UID per package name. A UID is assigned to an app at install
  time.

This CL also moves the WebApkServiceImplTests JUnit tests to instrumentation tests
(The simplification in this CL would make the JUnit tests super boring).
The instrumentation tests are useful. The behavior when an unauthorized caller
tries to use the WebAPK service was different than I expected

BUG=625448
Test=WebApkServiceImplTest.*
R=sievers
TBR=yfriedman (To get around the OWNERS checks for new DEPS files)

Committed: https://crrev.com/9af31665e1d8a11e8cef9d1d6ebb8891c1239ad4
Cr-Commit-Position: refs/heads/master@{#405399}

[pkotwicz, 2016-07-09 03:46:15]

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

[pkotwicz, 2016-07-09 03:46:15]

Yaron can you please take a look? This CL is now a prerequisite to
https://codereview.chromium.org/2114293003/

[Yaron, 2016-07-11 13:41:41]

https://codereview.chromium.org/2133923002/diff/1/chrome/android/webapk/libs/...
File
chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkUtils.java
(right):

https://codereview.chromium.org/2133923002/diff/1/chrome/android/webapk/libs/...
chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkUtils.java:67:
public static int getHostBrowserUid(Context context) {
Hmm, why is this or getHostBrowserPackageName in common? Seems like the client
should never need these fn calls and they should be in the shell apk or runtime
library (but probably the former)

[pkotwicz, 2016-07-11 13:49:59]

Yaron can you please take another look?

https://codereview.chromium.org/2133923002/diff/1/chrome/android/webapk/libs/...
File
chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkUtils.java
(right):

https://codereview.chromium.org/2133923002/diff/1/chrome/android/webapk/libs/...
chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkUtils.java:67:
public static int getHostBrowserUid(Context context) {
getHostBrowserPackageName() is in common because of
https://codereview.chromium.org/1888773004/ There is no reason for it to be in
common and we should move these functions to webapk/shell_apk. I will do this as
a followup CL

[Yaron, 2016-07-12 03:58:43]

lgtm

https://codereview.chromium.org/2133923002/diff/1/chrome/android/webapk/libs/...
File
chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkUtils.java
(right):

https://codereview.chromium.org/2133923002/diff/1/chrome/android/webapk/libs/...
chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkUtils.java:67:
public static int getHostBrowserUid(Context context) {
On 2016/07/11 13:49:58, pkotwicz wrote:
> getHostBrowserPackageName() is in common because of
> https://codereview.chromium.org/1888773004/ There is no reason for it to be in
> common and we should move these functions to webapk/shell_apk. I will do this
as
> a followup CL

oops, my bad :(

https://codereview.chromium.org/2133923002/diff/1/chrome/android/webapk/libs/...
File
chrome/android/webapk/libs/runtime_library/junit/src/org/chromium/webapk/lib/runtime_library/WebApkServiceImplTest.java
(left):

https://codereview.chromium.org/2133923002/diff/1/chrome/android/webapk/libs/...
chrome/android/webapk/libs/runtime_library/junit/src/org/chromium/webapk/lib/runtime_library/WebApkServiceImplTest.java:1:
// Copyright 2016 The Chromium Authors. All rights reserved.
can you add a mention of this to the cl description. in case the folding of
junit tests into into the instrumentation test breaks the builder, it'll be
easier to track

[pkotwicz, 2016-07-12 15:27:19]

Description was changed from

==========
Simplify caller checking logic in WebApkServiceImpl

This CL passes the uid of the host browser instead of the package name of the
host browser. This simplifies things because:
- Several package names can have the same UID.
- There is only one UID per package name. A UID is assigned to an app at install
  time.

BUG=625448
Test=WebApkServiceImplTest.*
==========

to

==========
Simplify caller checking logic in WebApkServiceImpl

This CL passes the uid of the host browser instead of the package name of the
host browser. This simplifies things because:
- Several package names can have the same UID.
- There is only one UID per package name. A UID is assigned to an app at install
  time.

This CL also moves the WebApkServiceImplTests JUnit tests to instrumentation
tests
(The simplification in this CL would make the JUnit tests super boring).
The instrumentation tests are useful. The behavior when an unauthorized caller
tries to use the WebAPK service was different than I expected

BUG=625448
Test=WebApkServiceImplTest.*
==========

[pkotwicz, 2016-07-12 15:27:19]

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

[pkotwicz, 2016-07-12 15:27:45]

rsesek@ can you please take a look at this CL from a security stand point?

[pkotwicz, 2016-07-12 15:27:46]

rsesek@ can you please take a look at this CL from a security stand point?

[pkotwicz, 2016-07-13 13:41:12]

rsesek@ Ping!

[Robert Sesek, 2016-07-13 19:32:22]

In a WebAPK the Bundle provided to the WebApkServiceImpl is from the manifest,
right? What happens if the UID of the browser changes (delete/reinstall or
backup/restore)?

[pkotwicz, 2016-07-13 19:58:28]

If this occurs WebApkServiceFactory needs to build a new WebApkServiceImpl. I
know there isn't code to do this yet

I have filed crbug.com/627950 to deal with Chrome being uninstalled / upgraded
and the WebAPK process not terminating.

[Robert Sesek, 2016-07-13 20:56:03]

lgtm

[pkotwicz, 2016-07-13 22:09:17]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2016-07-13 22:09:18]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org,
yfriedman@chromium.org
Link to the patchset: https://codereview.chromium.org/2133923002/#ps20001
(title: "Merge branch 'master' into sandbox_on_transact1")

[commit-bot: I haz the power, 2016-07-13 22:10:16]

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-07-13 22:21:44]

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

[commit-bot: I haz the power, 2016-07-13 22:21:45]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[pkotwicz, 2016-07-14 00:16:03]

Description was changed from

==========
Simplify caller checking logic in WebApkServiceImpl

This CL passes the uid of the host browser instead of the package name of the
host browser. This simplifies things because:
- Several package names can have the same UID.
- There is only one UID per package name. A UID is assigned to an app at install
  time.

This CL also moves the WebApkServiceImplTests JUnit tests to instrumentation
tests
(The simplification in this CL would make the JUnit tests super boring).
The instrumentation tests are useful. The behavior when an unauthorized caller
tries to use the WebAPK service was different than I expected

BUG=625448
Test=WebApkServiceImplTest.*
==========

to

==========
Simplify caller checking logic in WebApkServiceImpl

This CL passes the uid of the host browser instead of the package name of the
host browser. This simplifies things because:
- Several package names can have the same UID.
- There is only one UID per package name. A UID is assigned to an app at install
  time.

This CL also moves the WebApkServiceImplTests JUnit tests to instrumentation
tests
(The simplification in this CL would make the JUnit tests super boring).
The instrumentation tests are useful. The behavior when an unauthorized caller
tries to use the WebAPK service was different than I expected

BUG=625448
Test=WebApkServiceImplTest.*
R=sievers
TBR=yfriedman (To get around the OWNERS checks for new DEPS files)
==========

[pkotwicz, 2016-07-14 00:21:45]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2016-07-14 00:21:45]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org,
yfriedman@chromium.org
Link to the patchset: https://codereview.chromium.org/2133923002/#ps40001
(title: "Merge branch 'master' into sandbox_on_transact1")

[commit-bot: I haz the power, 2016-07-14 00:23:00]

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-07-14 01:59:41]

Description was changed from

==========
Simplify caller checking logic in WebApkServiceImpl

This CL passes the uid of the host browser instead of the package name of the
host browser. This simplifies things because:
- Several package names can have the same UID.
- There is only one UID per package name. A UID is assigned to an app at install
  time.

This CL also moves the WebApkServiceImplTests JUnit tests to instrumentation
tests
(The simplification in this CL would make the JUnit tests super boring).
The instrumentation tests are useful. The behavior when an unauthorized caller
tries to use the WebAPK service was different than I expected

BUG=625448
Test=WebApkServiceImplTest.*
R=sievers
TBR=yfriedman (To get around the OWNERS checks for new DEPS files)
==========

to

==========
Simplify caller checking logic in WebApkServiceImpl

This CL passes the uid of the host browser instead of the package name of the
host browser. This simplifies things because:
- Several package names can have the same UID.
- There is only one UID per package name. A UID is assigned to an app at install
  time.

This CL also moves the WebApkServiceImplTests JUnit tests to instrumentation
tests
(The simplification in this CL would make the JUnit tests super boring).
The instrumentation tests are useful. The behavior when an unauthorized caller
tries to use the WebAPK service was different than I expected

BUG=625448
Test=WebApkServiceImplTest.*
R=sievers
TBR=yfriedman (To get around the OWNERS checks for new DEPS files)
==========

[commit-bot: I haz the power, 2016-07-14 01:59:42]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-14 02:00:07]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-14 02:01:51]

Description was changed from

==========
Simplify caller checking logic in WebApkServiceImpl

This CL passes the uid of the host browser instead of the package name of the
host browser. This simplifies things because:
- Several package names can have the same UID.
- There is only one UID per package name. A UID is assigned to an app at install
  time.

This CL also moves the WebApkServiceImplTests JUnit tests to instrumentation
tests
(The simplification in this CL would make the JUnit tests super boring).
The instrumentation tests are useful. The behavior when an unauthorized caller
tries to use the WebAPK service was different than I expected

BUG=625448
Test=WebApkServiceImplTest.*
R=sievers
TBR=yfriedman (To get around the OWNERS checks for new DEPS files)
==========

to

==========
Simplify caller checking logic in WebApkServiceImpl

This CL passes the uid of the host browser instead of the package name of the
host browser. This simplifies things because:
- Several package names can have the same UID.
- There is only one UID per package name. A UID is assigned to an app at install
  time.

This CL also moves the WebApkServiceImplTests JUnit tests to instrumentation
tests
(The simplification in this CL would make the JUnit tests super boring).
The instrumentation tests are useful. The behavior when an unauthorized caller
tries to use the WebAPK service was different than I expected

BUG=625448
Test=WebApkServiceImplTest.*
R=sievers
TBR=yfriedman (To get around the OWNERS checks for new DEPS files)

Committed: https://crrev.com/9af31665e1d8a11e8cef9d1d6ebb8891c1239ad4
Cr-Commit-Position: refs/heads/master@{#405399}
==========

[commit-bot: I haz the power, 2016-07-14 02:01:52]

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