Enable "Get App Settings" in Android -> Settings -> Applications -> Chromium

Enable "Get App Settings" option in Android->Settings->Apps->Chromium.
 
It will redirect to the Settings page and allow user
for fast changing options responsible for network usage.

More about feature:
https://developer.android.com/training/basics/network-ops/managing.html

[marcin, 2017-01-01 22:51:35]

Description was changed from

==========
Revert "Patch for Ubuntu 16.10"

This reverts commit 6bf34206a1ea4e30e8024a7563556c91c2c49f48.

Revert "Add author"

This reverts commit 76e466de6bffb367e387c85e69bd63599773c0db.

Merge branch 'build1610' into settings


Adding support for "Get App Settings"


Merge branch 'master' of https://chromium.googlesource.com/chromium/src into
build1610


Add author


Patch for Ubuntu 16.10

BUG=
==========

to

==========
Patch enables "Get App Settings" option in system settings menu -> Applications
-> Chromium. It will redirect to the Settings page and allow user for fast
changing options responsible for network usage.

More about feature:
https://developer.android.com/training/basics/network-ops/managing.html
==========

[marcin, 2017-01-01 22:51:35]

marcin@mwiacek.com changed reviewers:
+ dfalcantara@chromium.org

[marcin, 2017-01-01 23:40:01]

The CQ bit was checked by marcin@mwiacek.com

[commit-bot: I haz the power, 2017-01-01 23:40:08]

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-01-01 23:40:08]

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

[commit-bot: I haz the power, 2017-01-01 23:40:09]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[marcin, 2017-01-02 08:41:32]

Description was changed from

==========
Patch enables "Get App Settings" option in system settings menu -> Applications
-> Chromium. It will redirect to the Settings page and allow user for fast
changing options responsible for network usage.

More about feature:
https://developer.android.com/training/basics/network-ops/managing.html
==========

to

==========
Enable "Get App Settings" option in Android->Settings->Apps->Chromium.
 
It will redirect to the Settings page and allow user
for fast changing options responsible for network usage.

More about feature:
https://developer.android.com/training/basics/network-ops/managing.html
==========

[gone, 2017-01-03 05:58:48]

Er, explicitly not lgtm because you tried to submit this without any review. 
What Chromium bug are you trying to fix?  Did you file one?

[marcin, 2017-01-03 09:19:58]

On 2017/01/03 05:58:48, dfalcantara (OOO 12.22-1.3) wrote:
> Er, explicitly not lgtm because you tried to submit this without any review. 
> What Chromium bug are you trying to fix?  Did you file one?

Android 4.0 with this feature - October 18, 2011

Patch enabling it for one of the most important apps - 2017

Big shame, that it cannot be applied. Is this final decision?

PS. I don't see Feature Request for it in public bugs.

[gone, 2017-01-03 17:24:55]

On 2017/01/03 09:19:58, marcin wrote:
> On 2017/01/03 05:58:48, dfalcantara (OOO 12.22-1.3) wrote:
> > Er, explicitly not lgtm because you tried to submit this without any review.

> > What Chromium bug are you trying to fix?  Did you file one?
> 
> Android 4.0 with this feature - October 18, 2011
> 
> Patch enabling it for one of the most important apps - 2017
> 
> Big shame, that it cannot be applied. Is this final decision?
> 
> PS. I don't see Feature Request for it in public bugs.

Judging by how you're wondering if this is a final decision, start here:
https://www.chromium.org/developers/contributing-code

1) You're trying to hammer this through by saying that we're six
   years late on doing something instead of asking for a proper code review

2) There's no bug filed for this in crbug.com where we can discuss
   the need for the patch at all

3) Nothing in your CL suggests that you've tested the side-effects
   of changing the intent filter of an important Chrome Activity;
   e.g. does this work with the settings activity not being exported?

[marcin, 2017-01-04 20:48:24]

On 2017/01/03 17:24:55, dfalcantara (check queue) wrote:
> On 2017/01/03 09:19:58, marcin wrote:
> > On 2017/01/03 05:58:48, dfalcantara (OOO 12.22-1.3) wrote:
> > > Er, explicitly not lgtm because you tried to submit this without any
review.
> 
> > > What Chromium bug are you trying to fix?  Did you file one?
> > 
> > Android 4.0 with this feature - October 18, 2011
> > 
> > Patch enabling it for one of the most important apps - 2017
> > 
> > Big shame, that it cannot be applied. Is this final decision?
> > 
> > PS. I don't see Feature Request for it in public bugs.
> 
> Judging by how you're wondering if this is a final decision, start here:
> https://www.chromium.org/developers/contributing-code
> 
> 1) You're trying to hammer this through by saying that we're six
>    years late on doing something instead of asking for a proper code review
> 
> 2) There's no bug filed for this in http://crbug.com where we can discuss
>    the need for the patch at all
> 
> 3) Nothing in your CL suggests that you've tested the side-effects
>    of changing the intent filter of an important Chrome Activity;
>    e.g. does this work with the settings activity not being exported?

https://www.chromium.org/developers/contributing-code:

"The reviewer will almost always have suggestions or style fixes for you,
and it's important not to take such suggestions personally or as a commentary
on your abilities or ideas. This is a process where we work together
to make sure that the highest quality code gets submitted."

Do you have any suggestions or style fixes? Do you think, that this patch
doesn't meet any requirement? Is it doing something else than described 
in your opinion ? Is accidentally clicking in web tool
("you tried to submit this without any review") connected with quality of patch?

Additionally on https://www.chromium.org/developers/contributing-code
I don't see info, that bug is mandatory. I understand, that in your opinion 
in this simple case it happens and I have opened today
https://bugs.chromium.org/p/chromium/issues/detail?id=678362.

DO you need something more ?

What proofs of checking patch quality do you need? Are "adb shell screenrecord"
videos enough ? Should I upload them to YouTube or do you prefer any other
place?

"does this work with the settings activity not being exported?" - could you
clarify
this case ? (if you speak about "android:exported=false", it doesn't change
anything and patch is still working). 

I asked for review for very small patch enabling OS functionality
available since 2011. If possible, please clarify next steps, which could
help in implementing it.

[gone, 2017-01-04 21:16:44]

On 2017/01/04 20:48:24, marcin wrote:
> On 2017/01/03 17:24:55, dfalcantara (check queue) wrote:
> > On 2017/01/03 09:19:58, marcin wrote:
> > > On 2017/01/03 05:58:48, dfalcantara (OOO 12.22-1.3) wrote:
> > > > Er, explicitly not lgtm because you tried to submit this without any
> review.
> > 
> > > > What Chromium bug are you trying to fix?  Did you file one?
> > > 
> > > Android 4.0 with this feature - October 18, 2011
> > > 
> > > Patch enabling it for one of the most important apps - 2017
> > > 
> > > Big shame, that it cannot be applied. Is this final decision?
> > > 
> > > PS. I don't see Feature Request for it in public bugs.
> > 
> > Judging by how you're wondering if this is a final decision, start here:
> > https://www.chromium.org/developers/contributing-code
> > 
> > 1) You're trying to hammer this through by saying that we're six
> >    years late on doing something instead of asking for a proper code review
> > 
> > 2) There's no bug filed for this in http://crbug.com where we can discuss
> >    the need for the patch at all
> > 
> > 3) Nothing in your CL suggests that you've tested the side-effects
> >    of changing the intent filter of an important Chrome Activity;
> >    e.g. does this work with the settings activity not being exported?
> 
> https://www.chromium.org/developers/contributing-code:
> 
> "The reviewer will almost always have suggestions or style fixes for you,
> and it's important not to take such suggestions personally or as a commentary
> on your abilities or ideas. This is a process where we work together
> to make sure that the highest quality code gets submitted."
> 
> Do you have any suggestions or style fixes? Do you think, that this patch
> doesn't meet any requirement? Is it doing something else than described 
> in your opinion ? Is accidentally clicking in web tool
> ("you tried to submit this without any review") connected with quality of
patch?
> 
> Additionally on https://www.chromium.org/developers/contributing-code
> I don't see info, that bug is mandatory. I understand, that in your opinion 
> in this simple case it happens and I have opened today
> https://bugs.chromium.org/p/chromium/issues/detail?id=678362.
> 
> DO you need something more ?
> 
> What proofs of checking patch quality do you need? Are "adb shell
screenrecord"
> videos enough ? Should I upload them to YouTube or do you prefer any other
> place?
> 
> "does this work with the settings activity not being exported?" - could you
> clarify
> this case ? (if you speak about "android:exported=false", it doesn't change
> anything and patch is still working). 
> 
> I asked for review for very small patch enabling OS functionality
> available since 2011. If possible, please clarify next steps, which could
> help in implementing it.

The reason I asked for a bug is so we can discuss what you're trying to do
before you send off a patch, even if you think that what you're changing is
"simple".  "Very small" manifest changes have resulted in several bugs that have
prevented Chrome from launching at all, so pardon my paranoia about your change.
Moreover, it might make more sense to send the user to a different part of the
Settings page, like "Data usage".  That, however, is a discussion that should
be had on the bug itself.

I'm hung up on the android:exported bit because while your CL works, the
Android documentation says that it shouldn't:
https://developer.android.com/guide/topics/manifest/activity-element.html#exp...

[marcin, 2017-01-04 21:33:54]

On 2017/01/04 21:16:44, dfalcantara (check queue) wrote:
> On 2017/01/04 20:48:24, marcin wrote:
> > On 2017/01/03 17:24:55, dfalcantara (check queue) wrote:
> > > On 2017/01/03 09:19:58, marcin wrote:
> > > > On 2017/01/03 05:58:48, dfalcantara (OOO 12.22-1.3) wrote:
> > > > > Er, explicitly not lgtm because you tried to submit this without any
> > review.
> > > 
> > > > > What Chromium bug are you trying to fix?  Did you file one?
> > > > 
> > > > Android 4.0 with this feature - October 18, 2011
> > > > 
> > > > Patch enabling it for one of the most important apps - 2017
> > > > 
> > > > Big shame, that it cannot be applied. Is this final decision?
> > > > 
> > > > PS. I don't see Feature Request for it in public bugs.
> > > 
> > > Judging by how you're wondering if this is a final decision, start here:
> > > https://www.chromium.org/developers/contributing-code
> > > 
> > > 1) You're trying to hammer this through by saying that we're six
> > >    years late on doing something instead of asking for a proper code
review
> > > 
> > > 2) There's no bug filed for this in http://crbug.com where we can discuss
> > >    the need for the patch at all
> > > 
> > > 3) Nothing in your CL suggests that you've tested the side-effects
> > >    of changing the intent filter of an important Chrome Activity;
> > >    e.g. does this work with the settings activity not being exported?
> > 
> > https://www.chromium.org/developers/contributing-code:
> > 
> > "The reviewer will almost always have suggestions or style fixes for you,
> > and it's important not to take such suggestions personally or as a
commentary
> > on your abilities or ideas. This is a process where we work together
> > to make sure that the highest quality code gets submitted."
> > 
> > Do you have any suggestions or style fixes? Do you think, that this patch
> > doesn't meet any requirement? Is it doing something else than described 
> > in your opinion ? Is accidentally clicking in web tool
> > ("you tried to submit this without any review") connected with quality of
> patch?
> > 
> > Additionally on https://www.chromium.org/developers/contributing-code
> > I don't see info, that bug is mandatory. I understand, that in your opinion 
> > in this simple case it happens and I have opened today
> > https://bugs.chromium.org/p/chromium/issues/detail?id=678362.
> > 
> > DO you need something more ?
> > 
> > What proofs of checking patch quality do you need? Are "adb shell
> screenrecord"
> > videos enough ? Should I upload them to YouTube or do you prefer any other
> > place?
> > 
> > "does this work with the settings activity not being exported?" - could you
> > clarify
> > this case ? (if you speak about "android:exported=false", it doesn't change
> > anything and patch is still working). 
> > 
> > I asked for review for very small patch enabling OS functionality
> > available since 2011. If possible, please clarify next steps, which could
> > help in implementing it.
> 
> The reason I asked for a bug is so we can discuss what you're trying to do
> before you send off a patch, even if you think that what you're changing is
> "simple".  "Very small" manifest changes have resulted in several bugs that
have
> prevented Chrome from launching at all, so pardon my paranoia about your
change.
> Moreover, it might make more sense to send the user to a different part of the
> Settings page, like "Data usage".  That, however, is a discussion that should
> be had on the bug itself.
> 
> I'm hung up on the android:exported bit because while your CL works, the
> Android documentation says that it shouldn't:
>
https://developer.android.com/guide/topics/manifest/activity-element.html#exp...

1. My development/QA experience say, that every patch should be checked and 
I have done it in first place (Samsung / Android 6). I understand, that there 
can be concerns and thank you, that you raised them.

2. In this concrete case - we can disable for example JavaScript and there can
be
less files read (no AJAX calls, etc.), we can also switch other options and it
can
change amount of data exchanged. In this situation it seems, that opening 
main Settings page is the most sensible.

3. Maybe for android:exported there should be bug opened for Android team. Could
you
maybe address it correctly?

4. I understand, that I should wait for approval now - I will not comment more
unless you will ask for it.

[gone, 2017-01-04 22:01:56]

On 2017/01/04 21:33:54, marcin wrote:
> On 2017/01/04 21:16:44, dfalcantara (check queue) wrote:
> > On 2017/01/04 20:48:24, marcin wrote:
> > > On 2017/01/03 17:24:55, dfalcantara (check queue) wrote:
> > > > On 2017/01/03 09:19:58, marcin wrote:
> > > > > On 2017/01/03 05:58:48, dfalcantara (OOO 12.22-1.3) wrote:
> > > > > > Er, explicitly not lgtm because you tried to submit this without any
> > > review.
> > > > 
> > > > > > What Chromium bug are you trying to fix?  Did you file one?
> > > > > 
> > > > > Android 4.0 with this feature - October 18, 2011
> > > > > 
> > > > > Patch enabling it for one of the most important apps - 2017
> > > > > 
> > > > > Big shame, that it cannot be applied. Is this final decision?
> > > > > 
> > > > > PS. I don't see Feature Request for it in public bugs.
> > > > 
> > > > Judging by how you're wondering if this is a final decision, start here:
> > > > https://www.chromium.org/developers/contributing-code
> > > > 
> > > > 1) You're trying to hammer this through by saying that we're six
> > > >    years late on doing something instead of asking for a proper code
> review
> > > > 
> > > > 2) There's no bug filed for this in http://crbug.com where we can
discuss
> > > >    the need for the patch at all
> > > > 
> > > > 3) Nothing in your CL suggests that you've tested the side-effects
> > > >    of changing the intent filter of an important Chrome Activity;
> > > >    e.g. does this work with the settings activity not being exported?
> > > 
> > > https://www.chromium.org/developers/contributing-code:
> > > 
> > > "The reviewer will almost always have suggestions or style fixes for you,
> > > and it's important not to take such suggestions personally or as a
> commentary
> > > on your abilities or ideas. This is a process where we work together
> > > to make sure that the highest quality code gets submitted."
> > > 
> > > Do you have any suggestions or style fixes? Do you think, that this patch
> > > doesn't meet any requirement? Is it doing something else than described 
> > > in your opinion ? Is accidentally clicking in web tool
> > > ("you tried to submit this without any review") connected with quality of
> > patch?
> > > 
> > > Additionally on https://www.chromium.org/developers/contributing-code
> > > I don't see info, that bug is mandatory. I understand, that in your
opinion 
> > > in this simple case it happens and I have opened today
> > > https://bugs.chromium.org/p/chromium/issues/detail?id=678362.
> > > 
> > > DO you need something more ?
> > > 
> > > What proofs of checking patch quality do you need? Are "adb shell
> > screenrecord"
> > > videos enough ? Should I upload them to YouTube or do you prefer any other
> > > place?
> > > 
> > > "does this work with the settings activity not being exported?" - could
you
> > > clarify
> > > this case ? (if you speak about "android:exported=false", it doesn't
change
> > > anything and patch is still working). 
> > > 
> > > I asked for review for very small patch enabling OS functionality
> > > available since 2011. If possible, please clarify next steps, which could
> > > help in implementing it.
> > 
> > The reason I asked for a bug is so we can discuss what you're trying to do
> > before you send off a patch, even if you think that what you're changing is
> > "simple".  "Very small" manifest changes have resulted in several bugs that
> have
> > prevented Chrome from launching at all, so pardon my paranoia about your
> change.
> > Moreover, it might make more sense to send the user to a different part of
the
> > Settings page, like "Data usage".  That, however, is a discussion that
should
> > be had on the bug itself.
> > 
> > I'm hung up on the android:exported bit because while your CL works, the
> > Android documentation says that it shouldn't:
> >
>
https://developer.android.com/guide/topics/manifest/activity-element.html#exp...
> 
> 1. My development/QA experience say, that every patch should be checked and 
> I have done it in first place (Samsung / Android 6). I understand, that there 
> can be concerns and thank you, that you raised them.

Yar.  The problem is that I've seen manufacturer specific bugs involving
Manifest changes, like this one involving Sony's launcher: crbug.com/412405

> 2. In this concrete case - we can disable for example JavaScript and there can
> be
> less files read (no AJAX calls, etc.), we can also switch other options and it
> can
> change amount of data exchanged. In this situation it seems, that opening 
> main Settings page is the most sensible.

Yeah, that makes sense to me, too.  The reason we prefer bugs for this kind of
discussion is that reasoning is significantly easier to dig up later, and that
follow up discussions are kept in the same place.

> 3. Maybe for android:exported there should be bug opened for Android team.
Could
> you
> maybe address it correctly?

I fired off an email to the Android team after I responded here.  Just got this
back:

"""
What tooling / IDE are you using?  As you say, some part of your
development/build
infrastructure is rewriting your manifest. Presumably it's being overly clever
about
guessing that you might want to provide network usage preferences in your app,
and
is "helpfully" inserting the necessary infrastructure for it, a la
<https://developer.android.com/training/basics/network-ops/managing.html>.

Properly, an app's MANAGE_NETWORK_USAGE activity should of course be exported
(as
should any <activity> declaration that publishes an intent filter).
"""

> 4. I understand, that I should wait for approval now - I will not comment more
> unless you will ask for it.

Your patch seems to require at least exporting the settings Activity; can you
update that?  I'll kick off parallel discussion on the bug.