[Cronet] Adjust to Android M permissions model

[Cronet] Adjust to Android M permissions model

Handle requesting permissions at run-time as per the new Android M
permissions model.

BUG=610174

[pauljensen, 2016-05-10 12:37:23]

pauljensen@chromium.org changed reviewers:
+ xunjieli@chromium.org

[pauljensen, 2016-05-10 12:37:23]

Helen, PTAL.

https://codereview.chromium.org/1961943003/diff/40001/components/cronet.gypi
File components/cronet.gypi (right):

https://codereview.chromium.org/1961943003/diff/40001/components/cronet.gypi#...
components/cronet.gypi:243:
'../third_party/android_tools/android_tools.gyp:android_support_v13_javalib',
FYI This does not appear to increase the size of cronet.jar or cronet_api.jar

[xunjieli, 2016-05-10 14:12:07]

The requesting permission logic somewhat complicates the sample app. The app can
write to its own data storage right? Can we avoid using external storage and
just write to the app's data storage? That way we can keep the sample app as
lean as possible, since internal storage is always available.

http://developer.android.com/training/basics/data-storage/files.html

https://codereview.chromium.org/1961943003/diff/40001/components/cronet.gypi
File components/cronet.gypi (right):

https://codereview.chromium.org/1961943003/diff/40001/components/cronet.gypi#...
components/cronet.gypi:243:
'../third_party/android_tools/android_tools.gyp:android_support_v13_javalib',
On 2016/05/10 12:37:23, pauljensen wrote:
> FYI This does not appear to increase the size of cronet.jar or cronet_api.jar

Acknowledged.

[pauljensen, 2016-05-10 14:22:33]

On 2016/05/10 14:12:07, xunjieli wrote:
> The requesting permission logic somewhat complicates the sample app. The app
can
> write to its own data storage right? Can we avoid using external storage and
> just write to the app's data storage? That way we can keep the sample app as
> lean as possible, since internal storage is always available.

Netlog is generally for debugging, and the app's data storage is inaccessible to
adb on non-rooted devices, and harder to find on rooted devices.  I imagine
typical Android app developers (esp outside Google) may not have access to a
rooted device/build.  I think it's much easier to use "adb pull
/sdcard/cronet_sample_netlog.json".

I do agree this makes the sample app more complicated.  Another alternative that
might work is to adjust the targetSdkVersion of the sample app to something
lower than Marshmallow (I think this might avoid the run-time permissions issue
perhaps), but I don't like this alternative as it may lead to headaches for
developers using the sample app and I think our sample app should lead with a
good example that supports the latest Android.

[xunjieli, 2016-05-10 14:50:40]

On 2016/05/10 14:22:33, pauljensen wrote:
> On 2016/05/10 14:12:07, xunjieli wrote:
> > The requesting permission logic somewhat complicates the sample app. The app
> can
> > write to its own data storage right? Can we avoid using external storage and
> > just write to the app's data storage? That way we can keep the sample app as
> > lean as possible, since internal storage is always available.
> 
> Netlog is generally for debugging, and the app's data storage is inaccessible
to
> adb on non-rooted devices, and harder to find on rooted devices.  I imagine
> typical Android app developers (esp outside Google) may not have access to a
> rooted device/build.  I think it's much easier to use "adb pull
> /sdcard/cronet_sample_netlog.json".
> 
> I do agree this makes the sample app more complicated.  Another alternative
that
> might work is to adjust the targetSdkVersion of the sample app to something
> lower than Marshmallow (I think this might avoid the run-time permissions
issue
> perhaps), but I don't like this alternative as it may lead to headaches for
> developers using the sample app and I think our sample app should lead with a
> good example that supports the latest Android.

That is a fair point, although one might argue the privacy implication of
storing logs in a publicly accessible place. I guess it is fine for a sample
app, but we shouldn't encourage the practice.
Ideally, a Cronet embedded application should do is to log in its data
directory, and make a dialogue for user to send/save the logs.
If we use external storage, the logs still linger on disk when user uninstalls
the app, and are available for almost all other apps with the permission. I am
fine with documenting somewhere that logging to external storage is not
encouraged in production.

[pauljensen, 2016-05-10 17:51:39]

On 2016/05/10 14:50:40, xunjieli wrote:
> On 2016/05/10 14:22:33, pauljensen wrote:
> > On 2016/05/10 14:12:07, xunjieli wrote:
> > > The requesting permission logic somewhat complicates the sample app. The
app
> > can
> > > write to its own data storage right? Can we avoid using external storage
and
> > > just write to the app's data storage? That way we can keep the sample app
as
> > > lean as possible, since internal storage is always available.
> > 
> > Netlog is generally for debugging, and the app's data storage is
inaccessible
> to
> > adb on non-rooted devices, and harder to find on rooted devices.  I imagine
> > typical Android app developers (esp outside Google) may not have access to a
> > rooted device/build.  I think it's much easier to use "adb pull
> > /sdcard/cronet_sample_netlog.json".
> > 
> > I do agree this makes the sample app more complicated.  Another alternative
> that
> > might work is to adjust the targetSdkVersion of the sample app to something
> > lower than Marshmallow (I think this might avoid the run-time permissions
> issue
> > perhaps), but I don't like this alternative as it may lead to headaches for
> > developers using the sample app and I think our sample app should lead with
a
> > good example that supports the latest Android.
> 
> That is a fair point, although one might argue the privacy implication of
> storing logs in a publicly accessible place. I guess it is fine for a sample
> app, but we shouldn't encourage the practice.
> Ideally, a Cronet embedded application should do is to log in its data
> directory, and make a dialogue for user to send/save the logs.
> If we use external storage, the logs still linger on disk when user uninstalls
> the app, and are available for almost all other apps with the permission. I am
> fine with documenting somewhere that logging to external storage is not
> encouraged in production.

I propose the following:
1. I change the sample app to use app data directory
2. I add a comment:
   If you are considering saving netlogs to external storage (e.g. /sdcard)
   there are some issues to consider:
       1. For your app to work on Android Marshmallow and newer releases
          your app will need to request access to external storage at
          run-time.  Here is an example of how to do this:
              https://codereview.chromium.org/1961943003/#ps40001
       2. External storage is accessible to all apps.  Netlogs can contain
          private information you may not want all other apps to see.
       3. Files in external storage are persisted forever, even after an
          app is uninstalled.  Since netlogs can be large, if your app is
          not careful to delete these file they may waste considerable
          space.

[xunjieli, 2016-05-10 17:58:58]

On 2016/05/10 17:51:39, pauljensen wrote:
> On 2016/05/10 14:50:40, xunjieli wrote:
> > On 2016/05/10 14:22:33, pauljensen wrote:
> > > On 2016/05/10 14:12:07, xunjieli wrote:
> > > > The requesting permission logic somewhat complicates the sample app. The
> app
> > > can
> > > > write to its own data storage right? Can we avoid using external storage
> and
> > > > just write to the app's data storage? That way we can keep the sample
app
> as
> > > > lean as possible, since internal storage is always available.
> > > 
> > > Netlog is generally for debugging, and the app's data storage is
> inaccessible
> > to
> > > adb on non-rooted devices, and harder to find on rooted devices.  I
imagine
> > > typical Android app developers (esp outside Google) may not have access to
a
> > > rooted device/build.  I think it's much easier to use "adb pull
> > > /sdcard/cronet_sample_netlog.json".
> > > 
> > > I do agree this makes the sample app more complicated.  Another
alternative
> > that
> > > might work is to adjust the targetSdkVersion of the sample app to
something
> > > lower than Marshmallow (I think this might avoid the run-time permissions
> > issue
> > > perhaps), but I don't like this alternative as it may lead to headaches
for
> > > developers using the sample app and I think our sample app should lead
with
> a
> > > good example that supports the latest Android.
> > 
> > That is a fair point, although one might argue the privacy implication of
> > storing logs in a publicly accessible place. I guess it is fine for a sample
> > app, but we shouldn't encourage the practice.
> > Ideally, a Cronet embedded application should do is to log in its data
> > directory, and make a dialogue for user to send/save the logs.
> > If we use external storage, the logs still linger on disk when user
uninstalls
> > the app, and are available for almost all other apps with the permission. I
am
> > fine with documenting somewhere that logging to external storage is not
> > encouraged in production.
> 
> I propose the following:
> 1. I change the sample app to use app data directory
> 2. I add a comment:
>    If you are considering saving netlogs to external storage (e.g. /sdcard)
>    there are some issues to consider:
>        1. For your app to work on Android Marshmallow and newer releases
>           your app will need to request access to external storage at
>           run-time.  Here is an example of how to do this:
>               https://codereview.chromium.org/1961943003/#ps40001
>        2. External storage is accessible to all apps.  Netlogs can contain
>           private information you may not want all other apps to see.
>        3. Files in external storage are persisted forever, even after an
>           app is uninstalled.  Since netlogs can be large, if your app is
>           not careful to delete these file they may waste considerable
>           space.

I am not sure. Either works for me. Your call :)

[pauljensen, 2016-05-11 02:06:44]

I closed this CL.  New CL is https://codereview.chromium.org/1971643002