android: Increase NUM_PRIVILEGED_SERVICES to 20

android: Increase NUM_PRIVILEGED_SERVICES to 20

This works in conjunction with
https://codereview.chromium.org/2557273004/ to workaround
crbug.com/664341. That CL ramdomizes the order of the named services to
avoid collision, which is not actually effective unless there is a large
number of named services to choose from.

Sandboxed service already has 20 named services, so up the privileged
(gpu) count to 20 as well.

BUG=664341
Committed: https://crrev.com/9b853cfcf82d8c57487cb0dd8d1eeaa5ca40260f
Cr-Commit-Position: refs/heads/master@{#437634}

[boliu, 2016-12-08 20:28:26]

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

[commit-bot: I haz the power, 2016-12-08 20:29:04]

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

[boliu, 2016-12-08 20:30:14]

boliu@chromium.org changed reviewers:
+ halliwell@chromium.org, tedchoc@chromium.org

[boliu, 2016-12-08 20:30:15]

ptal

halliwell for chromecast
tedchoc for everything else

Note I intend to merge this back to m56. If chromecast thinks that's not a safe
thing to do, I can split off the chromecast part to a different CL.

[commit-bot: I haz the power, 2016-12-08 21:10:23]

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

[commit-bot: I haz the power, 2016-12-08 21:10:24]

Dry run: This issue passed the CQ dry run.

[halliwell, 2016-12-09 00:01:27]

Description was changed from

==========
android: Increase NUM_PRIVILEGED_SERVICES to 20

This works in conjunction with
https://codereview.chromium.org/2557273004/ to workaround
crbug.com/664341. That CL ramdomizes the order of the named services to
avoid collision, which is not actually effective unless there is a large
number of named services to choose from.

Sandboxed service already has 20 named services, so up the privileged
(gpu) count to 20 as well.

BUG=664341
==========

to

==========
android: Increase NUM_PRIVILEGED_SERVICES to 20

This works in conjunction with
https://codereview.chromium.org/2557273004/ to workaround
crbug.com/664341. That CL ramdomizes the order of the named services to
avoid collision, which is not actually effective unless there is a large
number of named services to choose from.

Sandboxed service already has 20 named services, so up the privileged
(gpu) count to 20 as well.

BUG=664341
==========

[halliwell, 2016-12-09 00:01:28]

halliwell@chromium.org changed reviewers:
+ sanfin@chromium.org, tsunghung@chromium.org

[halliwell, 2016-12-09 00:02:16]

On 2016/12/08 20:30:15, boliu wrote:
> ptal
> 
> halliwell for chromecast
> tedchoc for everything else
> 
> Note I intend to merge this back to m56. If chromecast thinks that's not a
safe
> thing to do, I can split off the chromecast part to a different CL.

sanfin/tsunghung: could you please review this for ATV Cast?
We'll use M56 for our next platform release.

[Simeon, 2016-12-09 17:28:56]

On 2016/12/09 00:02:16, halliwell wrote:
> On 2016/12/08 20:30:15, boliu wrote:
> > ptal
> > 
> > halliwell for chromecast
> > tedchoc for everything else
> > 
> > Note I intend to merge this back to m56. If chromecast thinks that's not a
> safe
> > thing to do, I can split off the chromecast part to a different CL.
> 
> sanfin/tsunghung: could you please review this for ATV Cast?
> We'll use M56 for our next platform release.

Do we need to create Java files for all the new PrivilegedProcessServices? There
are only 3 Java classes for PrivilegedProcessServices according to
https://cs.chromium.org/search/?q=PrivilegedPRocessService&sq=package:chromiu...

[boliu, 2016-12-09 17:30:23]

On 2016/12/09 17:28:56, Simeon wrote:
> On 2016/12/09 00:02:16, halliwell wrote:
> > On 2016/12/08 20:30:15, boliu wrote:
> > > ptal
> > > 
> > > halliwell for chromecast
> > > tedchoc for everything else
> > > 
> > > Note I intend to merge this back to m56. If chromecast thinks that's not a
> > safe
> > > thing to do, I can split off the chromecast part to a different CL.
> > 
> > sanfin/tsunghung: could you please review this for ATV Cast?
> > We'll use M56 for our next platform release.
> 
> Do we need to create Java files for all the new PrivilegedProcessServices?
There
> are only 3 Java classes for PrivilegedProcessServices according to
>
https://cs.chromium.org/search/?q=PrivilegedPRocessService&sq=package:chromiu...

Oh yeah good point. But looking at the green cq bots, this probably means some
other number needs to be updated as well that I missed, hmm..

[boliu, 2016-12-09 18:07:23]

The CQ bit was checked by boliu@chromium.org

[boliu, 2016-12-09 18:07:26]

The CQ bit was unchecked by boliu@chromium.org

[boliu, 2016-12-09 18:07:30]

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

[commit-bot: I haz the power, 2016-12-09 18:07:52]

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

[boliu, 2016-12-09 18:10:10]

On 2016/12/09 17:30:23, boliu wrote:
> On 2016/12/09 17:28:56, Simeon wrote:
> > On 2016/12/09 00:02:16, halliwell wrote:
> > > On 2016/12/08 20:30:15, boliu wrote:
> > > > ptal
> > > > 
> > > > halliwell for chromecast
> > > > tedchoc for everything else
> > > > 
> > > > Note I intend to merge this back to m56. If chromecast thinks that's not
a
> > > safe
> > > > thing to do, I can split off the chromecast part to a different CL.
> > > 
> > > sanfin/tsunghung: could you please review this for ATV Cast?
> > > We'll use M56 for our next platform release.
> > 
> > Do we need to create Java files for all the new PrivilegedProcessServices?
> There
> > are only 3 Java classes for PrivilegedProcessServices according to
> >
>
https://cs.chromium.org/search/?q=PrivilegedPRocessService&sq=package:chromiu...
> 
> Oh yeah good point. But looking at the green cq bots, this probably means some
> other number needs to be updated as well that I missed, hmm..

Added those.

There was no other number to update. It's just cq tests did not catch this.
First, no test actually is testing rendering works, or that the gpu process even
started successfully. The crash is in the gpu process, not the browser process,
and browser will wait for up to 20 seconds before timing out waiting for gpu
process, so if a test finishes before that timeout, it will pass just fine.

[Simeon, 2016-12-09 18:28:15]

chromecast/ lgtm % nit

https://codereview.chromium.org/2555303004/diff/20001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/app/PrivilegedProcessService10.java
(right):

https://codereview.chromium.org/2555303004/diff/20001/content/public/android/...
content/public/android/java/src/org/chromium/content/app/PrivilegedProcessService10.java:9:
* more than one sandboxed process.
*privileged* process

Ditto all the others.

[halliwell, 2016-12-09 18:28:44]

On 2016/12/09 18:28:15, Simeon wrote:
> chromecast/ lgtm % nit
> 
>
https://codereview.chromium.org/2555303004/diff/20001/content/public/android/...
> File
>
content/public/android/java/src/org/chromium/content/app/PrivilegedProcessService10.java
> (right):
> 
>
https://codereview.chromium.org/2555303004/diff/20001/content/public/android/...
>
content/public/android/java/src/org/chromium/content/app/PrivilegedProcessService10.java:9:
> * more than one sandboxed process.
> *privileged* process
> 
> Ditto all the others.

rs-lgtm for chromecast

[boliu, 2016-12-09 18:41:17]

https://codereview.chromium.org/2555303004/diff/20001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/app/PrivilegedProcessService10.java
(right):

https://codereview.chromium.org/2555303004/diff/20001/content/public/android/...
content/public/android/java/src/org/chromium/content/app/PrivilegedProcessService10.java:9:
* more than one sandboxed process.
On 2016/12/09 18:28:15, Simeon wrote:
> *privileged* process
> 
> Ditto all the others.

heh, copied from existing ones :p

sed -i for the win!

[boliu, 2016-12-09 18:41:37]

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

[commit-bot: I haz the power, 2016-12-09 18:42:19]

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

[Ted C, 2016-12-09 18:42:34]

On 2016/12/09 18:41:17, boliu wrote:
>
https://codereview.chromium.org/2555303004/diff/20001/content/public/android/...
> File
>
content/public/android/java/src/org/chromium/content/app/PrivilegedProcessService10.java
> (right):
> 
>
https://codereview.chromium.org/2555303004/diff/20001/content/public/android/...
>
content/public/android/java/src/org/chromium/content/app/PrivilegedProcessService10.java:9:
> * more than one sandboxed process.
> On 2016/12/09 18:28:15, Simeon wrote:
> > *privileged* process
> > 
> > Ditto all the others.
> 
> heh, copied from existing ones :p
> 
> sed -i for the win!

lgtm

[boliu, 2016-12-09 18:44:00]

The CQ bit was unchecked by boliu@chromium.org

[boliu, 2016-12-09 18:44:04]

The CQ bit was checked by boliu@chromium.org

[boliu, 2016-12-09 18:44:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
halliwell@chromium.org, sanfin@chromium.org
Link to the patchset: https://codereview.chromium.org/2555303004/#ps40001
(title: "sed")

[commit-bot: I haz the power, 2016-12-09 18:44:48]

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-12-09 20:43:16]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481309044016570,
"parent_rev": "2acb702891c534846b7013e44b0f873a508a366c", "commit_rev":
"994b32da9288afc2a96521adfea7bce96cf4fe48"}

[commit-bot: I haz the power, 2016-12-09 20:43:43]

Description was changed from

==========
android: Increase NUM_PRIVILEGED_SERVICES to 20

This works in conjunction with
https://codereview.chromium.org/2557273004/ to workaround
crbug.com/664341. That CL ramdomizes the order of the named services to
avoid collision, which is not actually effective unless there is a large
number of named services to choose from.

Sandboxed service already has 20 named services, so up the privileged
(gpu) count to 20 as well.

BUG=664341
==========

to

==========
android: Increase NUM_PRIVILEGED_SERVICES to 20

This works in conjunction with
https://codereview.chromium.org/2557273004/ to workaround
crbug.com/664341. That CL ramdomizes the order of the named services to
avoid collision, which is not actually effective unless there is a large
number of named services to choose from.

Sandboxed service already has 20 named services, so up the privileged
(gpu) count to 20 as well.

BUG=664341

Review-Url: https://codereview.chromium.org/2555303004
==========

[commit-bot: I haz the power, 2016-12-09 20:43:45]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-12 14:40:12]

Description was changed from

==========
android: Increase NUM_PRIVILEGED_SERVICES to 20

This works in conjunction with
https://codereview.chromium.org/2557273004/ to workaround
crbug.com/664341. That CL ramdomizes the order of the named services to
avoid collision, which is not actually effective unless there is a large
number of named services to choose from.

Sandboxed service already has 20 named services, so up the privileged
(gpu) count to 20 as well.

BUG=664341

Review-Url: https://codereview.chromium.org/2555303004
==========

to

==========
android: Increase NUM_PRIVILEGED_SERVICES to 20

This works in conjunction with
https://codereview.chromium.org/2557273004/ to workaround
crbug.com/664341. That CL ramdomizes the order of the named services to
avoid collision, which is not actually effective unless there is a large
number of named services to choose from.

Sandboxed service already has 20 named services, so up the privileged
(gpu) count to 20 as well.

BUG=664341

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

[commit-bot: I haz the power, 2016-12-12 14:40:13]

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

[boliu, 2016-12-13 00:45:31]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2574593002/ by boliu@chromium.org.

The reason for reverting is: Android keeps the intent for a long time, so
randomize doesn't help at all. More details:
crbug.com/664341#c91.