Being able to hit this indicates a pretty bad bug. It means that a renderer processing visible content was running at background process priority which is bad for performance ( issue 560446 ).

The widget visible/hidden heuristic needs to be improved, it has too many failure modes..

Removing the DCHECK is hiding a bad issue IMO and I would advise against it.

Unassigning from me, then...  I don't have time to learn this code.  It does seem really easy to hit, and dealing with it on every branch is decreasing my productivity.

A minimized repro use case would help, I've never hit this and always run local builds with DCHECK_ALWAYS_ON.. So it's probably got something to do with the specific use case you put your local build through?

Are you sure it means the process is running at lower priority?  We haven't even called UpdateProcessPriority() yet, and I don't think we start processes at lower priority, right?  So is_process_backgrounded_ is true, but that's just because it's the default value.  Or do we start processes at lower priority, and I just didn't realize it?

A minimized repro:  Run Chrome on Windows.  Run any single browser test on Windows.  If I don't attach a debugger on start, I'm seeing it 100% of the time.

The default value is false : https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_process_host_impl.cc?type=cs&q=%22is_process_backgrounded_(false),%22+file:render_process_host_impl.cc&sq=package:chromium&l=668

And to be clear - I'm seeing it without any local changes at all.  I am using a component build without nacl, but that's about it, in terms of build-time magic.

Ah, you're right, I was thinking it was true.  Just calling UpdateProcessPriority() on the process creation callback fixes the issue (But only if we successfully create a process), but I assume we're not doing that by default for a reason.

This is indeed intentional, see  https://crbug.com/560446#c32 .

But I'm still wondering why you're getting |is_process_backgrounded_ == true| early if UpdateProcessPriority() isn't called?

Also I just tried running browser_tests.exe from a build like yours [1] on Windows and everything seems fine..?! 

[1]
   is_component_build = true
   is_debug = false
   is_chrome_branded = true
   dcheck_always_on = true
   is_win_fastlink = true
   enable_nacl = false

One possibility:

The only place other than UpdateProcessPriority() where is_process_backgrounded_ is updated is in RenderProcessHostImpl::OnProcessLaunched:

    is_process_backgrounded_ =
        child_process_launcher_->GetProcess().IsProcessBackgrounded();

which means that if your browser_tests.exe process is running at background priority for whatever reason on your OS, then so will its child processes and this very call will detect itself as a background process.

Oh....That's probably it.  I run command prompt at reduced priority, so MSVC intellisense takes priority over building.

And I've confirmed that's exactly the issue I'm running into.

Aaaah ;-). Then I guess back to you to decide whether you care to fix this or not (e.g. can |is_process_backgrounded_| be initialized another way than through the very process' priority).

This doesn't affect the product so P3.

The DCHECK is incorrect, given that way of setting whether a process is backgrounded or not.  Is there any compelling reason for that logic?  Since it's not correct, I'm inclined to just default to false, but maybe I'm missing something (In all 30 seconds I've thought about the issue).

I see. Yes the current code simply doesn't anticipate that case. Now that we understand it, I think it should be possible to trigger this by add a browsertest that changes the the browser process priority before launching a child process.

IIRC polling the actual process state was something we decided to do to avoid having to write code that anticipates os-specific behavior (e.g. on android processes are expected to launch at low priority). One way to circumvent the DCHECK in this case, would be to go back to having |is_process_backgrounded_| reflect "the priority we wish this process would have, modulo outside interference."

I.e., on windows, we would effectively change this line:

is_process_backgrounded_ =
        child_process_launcher_->GetProcess().IsProcessBackgrounded();

to:

is_process_backgrounded_ = false;

and so forth, for other platforms.

Yea, that seems like the right fix to me.  I'll dig through the code a bit more before sending out a CL, out of paranoia.

Assuming the process' initial priority introduces an implicit dependency on the process launching code (which is outside of this module) though, e.g. if Android changes its approach per  issue 601184  :-(.