New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 693484 link

Starred by 16 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocking:
issue 813232



Sign in to add a comment

[Chrome][M55][Android7.0] User can't open URL when more than 20 tabs are opened

Reported by seiyon.p...@gmail.com, Feb 17 2017

Issue description

Steps to reproduce the problem:
1. Launch Chrome
2. Open 21 tabs
3. Type URL (ex, www.amazon.com) on 21th tab

What is the expected behavior?
URL (ex, www.amazon.com) should be opened.

What went wrong?
URL (ex, www.amazon.com) is not opened.

Did this work before? N/A 

Chrome version: 55.0.2883.91  Channel: stable
OS Version: 7.0
Flash Version: Shockwave Flash 24.0 r0

The same issue was discussed in https://bugs.chromium.org/p/chromium/issues/detail?id=429657 but the fix was not landed.
The root cause is Chrome relies on the system to kill the renderers way before reaching 20. For good or for bad, the system has a hard-coded limit of 8 background processes. But OEM can update the limit of background processes and for example, OEM updates it to 64 and system doesn't kill process.
So this change on the limit of background process is impacting to Chrome tab opening behavior.

And this is not reproducible on Nexus devices since the limit is 8.
But it is very common that OEMs increase limit for performance improvement.
So Chrome should not reply on system to kill process but kill process by itself.

 
bugreport.zip
3.3 MB Download
Cc: tedc...@chromium.org klo...@chromium.org
Status: Available (was: Unconfirmed)
Cc: boliu@chromium.org

Comment 4 by boliu@chromium.org, Feb 18 2017

I totally was speculating earlier today whether this is possible or not: https://bugs.chromium.org/p/chromium/issues/detail?id=689758#c5

which devices are you using? where is limit coded in android, because I don't think it's anymore on pixels..

Comment 5 by boliu@chromium.org, Feb 18 2017

> I don't think it's anymore on pixels..

err, I don't think it's *8* anymore on pixels
It is reproducible on alicee_global_com and you could see the detailed information from bugreport attached in Comment #1.
Also I can assume that it's reproducible on many devices since many OEMs change the limit of background process.

Comment 7 by boliu@chromium.org, Feb 20 2017

what's alicee_global_com? what's a more common (in north america) device where this reproduces?
It's reproducible on mph1 device.

Device : mph1
Product : mph1_vzw
OS : M OS
Cc: aelias@chromium.org
Could you take a look and fix this limitation?

Comment 11 by boliu@chromium.org, Mar 14 2017

plan is chrome will kill renderers in lru order when running out of slots

Comment 12 by boliu@chromium.org, Mar 14 2017

Cc: mariakho...@chromium.org jcivelli@chromium.org agrieve@chromium.org yfried...@chromium.org
Labels: M-59
Owner: boliu@chromium.org
Status: Assigned (was: Available)
cc some people: plan is chrome will kill renderers in lru order when running out of slots

Comment 13 by boliu@chromium.org, Mar 15 2017

My plan is to add a separate LRU list where "use" means "process becomes visible". Note: ideally, the signal should be "widget belonging to process becomes visible", but that's a few layers of code removed from ChildProcessLaucher so not easy to get; "process becomes visible" should be a pretty good approximation.
@boliu, appreciated your support and detailed plan.
Hopefully the fix will be landed in M59.

Comment 15 by boliu@chromium.org, Mar 15 2017

for reference, I am able to reproduce this on a pixel phone by bumping MAX_CACHED_APPS from 32 to 128: https://android.googlesource.com/platform/frameworks/base/+/master/services/core/java/com/android/server/am/ProcessList.java

There are a few other values are calculated from that value, so I'm not sure exactly which one is the issue here. But that's probably the number being tweaked here.
@boliu,what you mention in https://bugs.chromium.org/p/chromium/issues/detail?id=693484#c15 is what OEMs have done for increasing the limit of background process.
So if bumping MAX_CACHED_APPS, you can set the same environment on Pixel device as OEM devices. 

Comment 17 by boliu@chromium.org, Mar 16 2017

Actually, scratch the "separate LRU list" idea. Gonna try refactoring BindingManagerImpl to have it support returning the LRU connection: https://bugs.chromium.org/p/chromium/issues/detail?id=689758#c17
Thanks for keeping me posted with the fix.

Comment 19 by boliu@chromium.org, Mar 30 2017

There's only 2 weeks left in m59, and I don't think there's enough time to get the proper fix (ie kill last used renderer) into m59, due to all the dependent refactors taking a long time

There's an easy workaround that should cover most of use case though I think: have chrome set kRendererProcessLimit to number of renderer slots. Probably gonna do that temporarily for m59
Project Member

Comment 20 by bugdroid1@chromium.org, Apr 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cbef3d37319db3e6153494db5f97df9412959856

commit cbef3d37319db3e6153494db5f97df9412959856
Author: boliu <boliu@chromium.org>
Date: Mon Apr 03 18:35:20 2017

android: Limit num renderer to service slots

This is a temporary workaround  crbug.com/693484 . The issue is chrome on
android expects the OS to not be able to maintain more than 20 service
processes. So the current code can only has 20 sandboxed slots, but
tells chrome that it can create as many renderer as it wants. On some
OEM devices, android has been tweaked to allow more than 20 background
processes, and this leads to a catastrophic failure case of not being
able to create new renderers.

The real fix of chrome actively killing LRU child is not ready, so this
is an imperfect workaround, to tell chrome to not create more than 20
renderers.

Note this will not cover all cases since this is a soft limit, and there
are other non-renderer sandboxed child processes beyond renderer. But
this should cover a large chunk of use cases to be address this severe
failure case in the short term.

BUG= 693484 

Review-Url: https://codereview.chromium.org/2793623002
Cr-Commit-Position: refs/heads/master@{#461482}

[modify] https://crrev.com/cbef3d37319db3e6153494db5f97df9412959856/content/browser/child_process_launcher.cc
[modify] https://crrev.com/cbef3d37319db3e6153494db5f97df9412959856/content/browser/child_process_launcher.h
[modify] https://crrev.com/cbef3d37319db3e6153494db5f97df9412959856/content/browser/child_process_launcher_helper.h
[modify] https://crrev.com/cbef3d37319db3e6153494db5f97df9412959856/content/browser/child_process_launcher_helper_android.cc
[modify] https://crrev.com/cbef3d37319db3e6153494db5f97df9412959856/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/cbef3d37319db3e6153494db5f97df9412959856/content/browser/renderer_host/render_process_host_unittest.cc
[modify] https://crrev.com/cbef3d37319db3e6153494db5f97df9412959856/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
[modify] https://crrev.com/cbef3d37319db3e6153494db5f97df9412959856/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncherHelper.java

Comment 21 by boliu@chromium.org, Aug 10 2017

Issue 515346 has been merged into this issue.
Cc: bauerb@chromium.org
 Issue 628993  has been merged into this issue.
Issue 810534 has been merged into this issue.
this issue becomes readily reproducible when strict site isolation (ie oopif) is enabled, as expected.

so this is the big blocker for enabling strict site isolation
Cc: emilyschechter@chromium.org nasko@chromium.org creis@chromium.org
Components: Internals>Sandbox>SiteIsolation
copying label/cc from the other bug
relevant doc here, especially for site isolation folks who are not familiar with child processes on android: https://docs.google.com/document/d/1PTI3udZ6TI-R0MlSsl2aflxRcnZ5blnkGGHE5JRI_Z8/edit?usp=sharing

Comment 27 by boliu@chromium.org, Feb 16 2018

Blocking: 813232
Project Member

Comment 28 by bugdroid1@chromium.org, Apr 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/08afe1ff234853691b1be4a4a8e45dc2dce2d955

commit 08afe1ff234853691b1be4a4a8e45dc2dce2d955
Author: Bo Liu <boliu@chromium.org>
Date: Mon Apr 09 23:48:08 2018

android: Don't call base::GetTerminationStatus

"Child" processes on android are not actually child processes of the
browser process, so waitpid always fails. Just return NORMAL_TERMINATION
instead.

Bug:  693484 
Change-Id: Id37bc2f562f80883a7f4b1e7a1d425becd3f853d
Reviewed-on: https://chromium-review.googlesource.com/1002538
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549325}
[modify] https://crrev.com/08afe1ff234853691b1be4a4a8e45dc2dce2d955/content/browser/child_process_launcher_helper_android.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Apr 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/261d11078d4f67a346c1ea2275d21680a7bf641d

commit 261d11078d4f67a346c1ea2275d21680a7bf641d
Author: Bo Liu <boliu@chromium.org>
Date: Fri Apr 13 20:22:51 2018

android: Add method to kill child process

Add a kill method to ChildProcessConnection to force kill the service.
Also add a new field that keeps track of whether the process is killed
intentionally. Add a junit test to verify this.

Bug:  693484 
Change-Id: Iaae9e82e611be97b9694fc8d67b23ba090220840
Reviewed-on: https://chromium-review.googlesource.com/1003635
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550744}
[modify] https://crrev.com/261d11078d4f67a346c1ea2275d21680a7bf641d/base/android/java/src/org/chromium/base/process_launcher/ChildProcessConnection.java
[modify] https://crrev.com/261d11078d4f67a346c1ea2275d21680a7bf641d/base/android/java/src/org/chromium/base/process_launcher/ChildProcessService.java
[modify] https://crrev.com/261d11078d4f67a346c1ea2275d21680a7bf641d/base/android/java/src/org/chromium/base/process_launcher/IChildProcessService.aidl
[modify] https://crrev.com/261d11078d4f67a346c1ea2275d21680a7bf641d/base/android/junit/src/org/chromium/base/process_launcher/ChildProcessConnectionTest.java

Project Member

Comment 30 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/261d11078d4f67a346c1ea2275d21680a7bf641d

commit 261d11078d4f67a346c1ea2275d21680a7bf641d
Author: Bo Liu <boliu@chromium.org>
Date: Fri Apr 13 20:22:51 2018

android: Add method to kill child process

Add a kill method to ChildProcessConnection to force kill the service.
Also add a new field that keeps track of whether the process is killed
intentionally. Add a junit test to verify this.

Bug:  693484 
Change-Id: Iaae9e82e611be97b9694fc8d67b23ba090220840
Reviewed-on: https://chromium-review.googlesource.com/1003635
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550744}
[modify] https://crrev.com/261d11078d4f67a346c1ea2275d21680a7bf641d/base/android/java/src/org/chromium/base/process_launcher/ChildProcessConnection.java
[modify] https://crrev.com/261d11078d4f67a346c1ea2275d21680a7bf641d/base/android/java/src/org/chromium/base/process_launcher/ChildProcessService.java
[modify] https://crrev.com/261d11078d4f67a346c1ea2275d21680a7bf641d/base/android/java/src/org/chromium/base/process_launcher/IChildProcessService.aidl
[modify] https://crrev.com/261d11078d4f67a346c1ea2275d21680a7bf641d/base/android/junit/src/org/chromium/base/process_launcher/ChildProcessConnectionTest.java

Comment 31 by boliu@chromium.org, Apr 17 2018

Cc: wnwen@chromium.org
I'm taking this opportunity to clean up child process UMA on android once and for all!! (until someone else tries to add something else)

For a particular child death record, we want to know:
1) process type: renderer, gpu, utility (only supported process types on android)
2) app status: running, paused, background (= stopped or destroyed)
3) crash report: whether process death generated a crash report
4) whether child has oom protection binding
5) base::TerminationStatus: only two applies here, NORMAL and LAUNCH_FAILED. LAUNCH_FAILED happens when the child service dies before setupConnection succeeds
6) whether process was killed intentionally by the new thing in this bug
7) for renderer only: whether renderer is hosting a main frame (vs subframe, new for OOPIF)

Current status:
* Tab.RendererDetailedExitStatus/Tab.RendererDetailedExitStatusUnbound covers 2, 3, 4 for renderer
* GPU.GPUProcessDetailedExitStatus covers 2, 3 and 4 (GPU always has oom bindings) for GPU
* utility processes are ignored
* 5 is ignored


My idea:
An enum histogram for each process type. And log a *single enum* for all possible combinations for the rest of the states. So if you are counting, for states from 2-7, that's 96 possible combinations, which is pretty ridiculous.

Only 5 appears to be dependent on other apps (if launch failed, then a lot of the other states don't apply), so probably should be pulled out. That still leaves 48 combinations..

Anyone has thoughts on more things that could be pulled out?

Comment 32 by boliu@chromium.org, Apr 17 2018

More "fun" things while looking into this..

BrowserChildProcessCrashed is never called right now. That's only called on base::TERMINATION_STATUS_PROCESS_CRASHED or base::TERMINATION_STATUS_ABNORMAL_TERMINATION, and neither happens on android.

GPU (and non-renderer child process) crash reporting depends on BrowserChildProcessHostDisconnected, which on desktop is called *in addition* to BrowserChildProcessCrashed.

So should really fix some of the callbacks on BrowserChildProcessObserver for android as well, or at least document all the weird behavior on android.

Comment 33 by boliu@chromium.org, Apr 17 2018

Question for anyone knowledgeable for how crash reports are generated on android. Is there any synchronization between browser start reading the minidump files and child process writing to the minidump fd?

I'm questioning how correct it is to check that the minidump file is size 0 on the browser side, if there is no synchronization here.
re: UMA

I think we should limit logging to the states that we care about. Here is the set of stuff I can conceivably use:

# of {renderer, gpu} crashes in {top, child} frame in running or paused state (just the sum of those is fine, no split) that had an oom protection binding and did not generate a crash report {killed by Android, ourselves}
=> that's 2^3 = 8 states

IMO that's sufficient for us to figure out important things:
1) Foreground OOM rate for renderer/gpu processes
2) Whether the new connection kill code ever triggers in foreground renderers
3) How often do OOPIF frames oom in foreground

These are specific questions I'd like to be able to answer.

Project Member

Comment 35 by bugdroid1@chromium.org, Apr 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/514c0abad9f0f75b97acfeca62612fb49a52539b

commit 514c0abad9f0f75b97acfeca62612fb49a52539b
Author: Bo Liu <boliu@chromium.org>
Date: Wed Apr 18 13:14:38 2018

android: Add ChildProcessRanking

Add ChildProcessRanking class which keeps a ranked list of connections
for a particular ChildConnectionAllocator. Add unit tests to test basic
functionality.

This will be used for killing the lowest ranked to free up a service
slot when all slots are full. For now, just integrate
ChildProcessRanking into ChildProcessLauncherHelper without using it for
anything.

Current implementation still relies on stability to for reasonable
behavior. Ideally in the future connections will be ranked explicitly
without relying on stability.

BindingManagerImpl and ChildProcessRanking probably should be merged at
some point in the future as well.

Bug:  693484 
Change-Id: I4401a29f198a63898424d7b78c8ef2b423b23cc9
Reviewed-on: https://chromium-review.googlesource.com/1013292
Commit-Queue: Bo <boliu@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551650}
[modify] https://crrev.com/514c0abad9f0f75b97acfeca62612fb49a52539b/content/public/android/BUILD.gn
[modify] https://crrev.com/514c0abad9f0f75b97acfeca62612fb49a52539b/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncherHelper.java
[add] https://crrev.com/514c0abad9f0f75b97acfeca62612fb49a52539b/content/public/android/java/src/org/chromium/content/browser/ChildProcessRanking.java
[add] https://crrev.com/514c0abad9f0f75b97acfeca62612fb49a52539b/content/public/android/junit/src/org/chromium/content/browser/ChildProcessRankingTest.java

Comment 36 by boliu@chromium.org, Apr 18 2018

> in running or paused state (just the sum of those is fine, no split)

That's good to know. Is that what happens now on the server side, that we combine the running + paused state to calculate the stability metric?

Also you probably meant to include oom protection bindings here as well here?


If that's all that's happening with this data on the server side, then yeah, not too bad I think. (Simple enough that that existing histogram that's already broken down by process type don't need any further break down)
For foreground oom computation I think we only care about the cases where oom protection bindings were present. And yes, in our oom rate graphs we add up running and paused stats today.

The question about protection binding does bring up a question of whether we have enough data to evaluate how often OOPIF subframes oom in foreground? Would those have protection bindings?

Comment 38 by boliu@chromium.org, Apr 18 2018

> The question about protection binding does bring up a question of whether we have enough data to evaluate how often OOPIF subframes oom in foreground? Would those have protection bindings?

Yes, we have the same data for main frames.

And yes. Current plan is visible subframes get moderate binding. Plus everything is still subject to moderate binding management.

I was thinking we should have a metric for how often user sees sad frame, but that should be a separate metric, with the source of truth in the renderer.
Project Member

Comment 39 by bugdroid1@chromium.org, Apr 19 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0d2a232ec87d7448213157cec1d5c494c146e100

commit 0d2a232ec87d7448213157cec1d5c494c146e100
Author: Bo Liu <boliu@chromium.org>
Date: Thu Apr 19 00:18:09 2018

Add ChildProcessTerminationInfo

Combine termination status and exit code into a single struct. And then
add two Android-only fields: oom protection binding, and intentional
kill.

Update the code in ChildProcessLauncher and ChildProcessLauncherHelper
to populate Info instead, including renaming methods.

Other than populating the new fields on Android, this is a no-op change
on all platforms.

Bug:  693484 
Change-Id: I1ce01cbf1e81a125b34604657d6abcb6a02e0556
Reviewed-on: https://chromium-review.googlesource.com/1013225
Reviewed-by: Mark Seaborn <mseaborn@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551894}
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/components/nacl/browser/nacl_process_host.cc
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/browser/browser_child_process_host_impl.cc
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/browser/browser_child_process_host_impl.h
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/browser/child_process_launcher.cc
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/browser/child_process_launcher.h
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/browser/child_process_launcher_helper.h
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/browser/child_process_launcher_helper_android.cc
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/browser/child_process_launcher_helper_fuchsia.cc
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/browser/child_process_launcher_helper_linux.cc
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/browser/child_process_launcher_helper_mac.cc
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/browser/child_process_launcher_helper_win.cc
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncherHelper.java
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/public/browser/BUILD.gn
[modify] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/public/browser/browser_child_process_host.h
[add] https://crrev.com/0d2a232ec87d7448213157cec1d5c494c146e100/content/public/browser/child_process_termination_info.h

Project Member

Comment 40 by bugdroid1@chromium.org, Apr 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e06143919fdd3780f43c2526da442201584d53b5

commit e06143919fdd3780f43c2526da442201584d53b5
Author: Bo Liu <boliu@chromium.org>
Date: Fri Apr 20 20:42:30 2018

Use ChildProcessLauncher::Terminate

Instead of base::Process::Terminate. This only makes a difference on
Android where "child" processes are not actually forked from the browser
process. Browser may or may not have permission to to kill a "child"
process.

In practice, chrome on android only supports unsandboxed GPU and utility
processes (for now) which are in the same uid as the browser, so sending
kill signal generally works.

Bug:  693484 
Change-Id: I830b92b079fa4a52a6c8c7512939ffad2bd72cfb
Reviewed-on: https://chromium-review.googlesource.com/1020445
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552461}
[modify] https://crrev.com/e06143919fdd3780f43c2526da442201584d53b5/content/browser/browser_child_process_host_impl.cc

Project Member

Comment 41 by bugdroid1@chromium.org, Apr 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1

commit 2a4894018ccf333aeb5ef18ab6d4691de4fb20a1
Author: Bo Liu <boliu@chromium.org>
Date: Tue Apr 24 23:41:27 2018

Use ChildProcessTerminationInfo more widely

Use ChildProcessTerminationInfo in these content APIs:
* BrowserChildProcessObserver::BrowserChildProcessCrashed/Killed
* NOTIFICATION_RENDERER_PROCESS_CLOSED (instead of
  RendererClosedDetails)
* RenderProcessHostObserver::RenderProcessExited

On desktop platforms, ChildProcessTerminationInfo replaces
base::TerminationStatus and exit_code. On Android,
ChildProcessTerminationInfo also contains two additional fields, since
"child" process launching is fairly different on Android.

There is one non-refactor change:
RenderProcessHostImpl::Cleanup now calls
ChildProcessLauncher::GetTerminationStatus.
On Android, this is important to populate the Android-only fields.
And for all platforms, this has the side effect of closing the
base::Process earlier, which is visible through
RenderProcessHost::GetProcess.

Everything else is a pure refactor.

chromecast/ review tbr
TBR=slan@chromium.org

Bug:  693484 
Change-Id: I64d40b19797483918d9777352fd9c4eec4e398f9
Reviewed-on: https://chromium-review.googlesource.com/1017386
Commit-Queue: Bo <boliu@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553367}
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/android/compositor/compositor_view.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/android/compositor/compositor_view.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/apps/guest_view/app_view_browsertest.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/chrome_child_process_watcher.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/chrome_child_process_watcher.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/chromeos/power/renderer_freezer.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/chromeos/power/renderer_freezer.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/media/webrtc/webrtc_event_log_manager.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/media/webrtc/webrtc_event_log_manager.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/metrics/chrome_stability_metrics_provider.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/metrics/chrome_stability_metrics_provider.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/metrics/chrome_stability_metrics_provider_unittest.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/metrics/plugin_metrics_provider.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/metrics/plugin_metrics_provider.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/metrics/plugin_metrics_provider_unittest.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/metrics/subprocess_metrics_provider.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/metrics/subprocess_metrics_provider.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/resource_coordinator/browser_child_process_watcher.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/resource_coordinator/browser_child_process_watcher.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/ui/cocoa/hung_renderer_controller.mm
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/ui/views/hung_renderer_view.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chrome/browser/ui/views/hung_renderer_view.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chromecast/browser/metrics/cast_stability_metrics_provider.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/chromecast/browser/metrics/cast_stability_metrics_provider.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/components/crash/content/browser/crash_dump_observer_android.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/components/crash/content/browser/crash_dump_observer_android.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/android/content_view_statics.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/browser_child_process_host_impl.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/download/mhtml_generation_manager.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/indexed_db/indexed_db_dispatcher_host.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/indexed_db/indexed_db_dispatcher_host.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/isolated_origin_browsertest.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/mach_broker_mac.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/mach_broker_mac.mm
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/renderer_host/render_process_host_browsertest.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/renderer_host/render_view_host_impl.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/renderer_host/render_view_host_impl.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/site_instance_impl.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/site_instance_impl.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/webrtc/webrtc_internals.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/browser/webrtc/webrtc_internals.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/public/browser/browser_child_process_observer.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/public/browser/child_process_termination_info.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/public/browser/notification_types.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/public/browser/render_process_host.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/public/browser/render_process_host_observer.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/public/test/browser_test_utils.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/public/test/mock_render_process_host.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/shell/browser/layout_test/blink_test_controller.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/content/shell/browser/layout_test/blink_test_controller.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/extensions/browser/event_router.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/extensions/browser/event_router.h
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/extensions/browser/extension_function_dispatcher.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/headless/lib/browser/headless_web_contents_impl.cc
[modify] https://crrev.com/2a4894018ccf333aeb5ef18ab6d4691de4fb20a1/headless/lib/browser/headless_web_contents_impl.h

Project Member

Comment 42 by bugdroid1@chromium.org, Apr 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e9624612b00bca20c5706800cf06b0201f07a23b

commit e9624612b00bca20c5706800cf06b0201f07a23b
Author: Bo Liu <boliu@chromium.org>
Date: Sat Apr 28 00:39:08 2018

android: Fix inputs into CrashDumpObserver

Overall goal is to switch Crash related classes on Android to use
content::ChildProcessTerminationInfo, which has more relevant
termination information than is currently available.

This is a set of related changes / fixes in order to make intentions and
behaviors on Android clearer.

* Change BrowserChildProcessHostImpl::OnChildDisconnected to always call
  BrowserChildProcessKilled (and BrowserChildProcessHostDelegate
  OnProcessCrashed), and explicitly document this behavior. This is
  because Android cannot distinguish between crashes and kills, and
  basing this decision on whether there was oom protection makes very
  little sense. Generally base::TerminationStatus does not convey much
  information.
  One concrete behavior change is that BrowserChildProcessKilled (and
  OnProcessCrashed) will now be called even when chrome app is in the
  background.
* Corresponding to above, have CrashDumpObserver listen to
  BrowserChildProcessKilled instead of BrowserChildProcessCrashed.
  Crashed was never called anyway. Listening to Killed allows populating
  additional termination info.
* Stop using base::TerminationStatus in crash, and use bool
  was_oom_protected_status instead, which is the only useful bit of
  information conveyed by TerminationStatus on Android.
  This also forces code to stop abusing other values of
  TerminationStatus for other purposes, ie added an explicit bool
  normal_exit instead of relying on NORMAL_EXIT.
* FastShutdownStarted case is treated as a normal exit. Previously it
  was treated as a "normal crash", although with other fields
  unpopulated.

components/data_reduction_proxy TBR
TBR=tbansal@chromium.org

Bug:  693484 
Change-Id: I887dcff000ccbc38bbb9e4159c1c5bb27a0c8876
Reviewed-on: https://chromium-review.googlesource.com/1025042
Commit-Queue: Bo <boliu@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554581}
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/android_webview/browser/aw_browser_terminator.cc
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/android_webview/browser/aw_browser_terminator.h
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/chrome/browser/android/compositor/compositor_view.cc
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/chrome/browser/android/compositor/compositor_view.h
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/chrome/browser/metrics/chrome_stability_metrics_provider.cc
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/chrome/browser/metrics/oom/out_of_memory_reporter_unittest.cc
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/components/crash/content/browser/child_process_crash_observer_android.cc
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/components/crash/content/browser/child_process_crash_observer_android.h
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/components/crash/content/browser/crash_dump_manager_android.cc
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/components/crash/content/browser/crash_dump_manager_android.h
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/components/crash/content/browser/crash_dump_manager_android_unittest.cc
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/components/crash/content/browser/crash_dump_observer_android.cc
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/components/crash/content/browser/crash_dump_observer_android.h
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/components/data_reduction_proxy/content/browser/data_reduction_proxy_pingback_client_impl_unittest.cc
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/content/browser/browser_child_process_host_impl.cc
[modify] https://crrev.com/e9624612b00bca20c5706800cf06b0201f07a23b/content/public/browser/browser_child_process_observer.h

Project Member

Comment 43 by bugdroid1@chromium.org, May 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/26ceee596207001df90f711242123d37167a304e

commit 26ceee596207001df90f711242123d37167a304e
Author: Bo Liu <boliu@chromium.org>
Date: Tue May 01 17:22:23 2018

android: Reload invisible crashed tab

Instead of using oom protection binding, which can cause invisible tab
to show a sad tab too, which is undesired.

Bug:  693484 
Change-Id: I0b59b8b04e048453fc62771272f569884d0be405
Reviewed-on: https://chromium-review.googlesource.com/1036844
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555087}
[modify] https://crrev.com/26ceee596207001df90f711242123d37167a304e/chrome/android/java/src/org/chromium/chrome/browser/tab/TabWebContentsObserver.java
[modify] https://crrev.com/26ceee596207001df90f711242123d37167a304e/chrome/android/javatests/src/org/chromium/chrome/browser/tab/SadTabTest.java

Project Member

Comment 44 by bugdroid1@chromium.org, May 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/81f03c973e4c127598773ceea08d6af5286cf4b1

commit 81f03c973e4c127598773ceea08d6af5286cf4b1
Author: Bo Liu <boliu@chromium.org>
Date: Tue May 01 19:11:11 2018

android: Log UMA for intentional kill

Log intentional kill UMAs yet a new histogram entry.
This time, instead of logging all possible combinations of states that
we might care about, aggregate them into counts on the client side
directly. So it's not really a "histogram" exactly. The up side is
that this will be easily extensible in the future.

Add renderer visibility to TerminationInfo which is needed for one of
the new UMAs.

Bug:  693484 
Change-Id: I9bf4cc8cfa969a8098bcdabf002585f0bf4a713a
Reviewed-on: https://chromium-review.googlesource.com/1032093
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Gayane Petrosyan <gayane@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Commit-Queue: Maria Khomenko <mariakhomenko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555132}
[modify] https://crrev.com/81f03c973e4c127598773ceea08d6af5286cf4b1/components/crash/content/browser/crash_dump_manager_android.cc
[modify] https://crrev.com/81f03c973e4c127598773ceea08d6af5286cf4b1/components/crash/content/browser/crash_dump_observer_android.cc
[modify] https://crrev.com/81f03c973e4c127598773ceea08d6af5286cf4b1/components/crash/content/browser/crash_dump_observer_android.h
[modify] https://crrev.com/81f03c973e4c127598773ceea08d6af5286cf4b1/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/81f03c973e4c127598773ceea08d6af5286cf4b1/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Assigned)
Forgot to add bug number for this last CL: https://chromium-review.googlesource.com/c/chromium/src/+/1002193

This is done.

Sign in to add a comment