Early return in {Suspend,Resume}Renderer() if not backgrounded

Early return in {Suspend,Resume}Renderer() if not backgrounded

After crrev.com/2564833002, RenderThreadImpl calls SuspendRenderer() and
ResumeRenderer() unconditionally. This could cause assertion failures
when the renderer isn't backgrounded.  Rather than reverting
crrev.com/2564833002, do early return when not backgrounded because
having multiple flags to check whether a renderer is suspended doesn't
sound a good idea.

BUG=674784
Committed: https://crrev.com/0a7c6fb8848b1a0100cbafae461a143eb2c83362
Cr-Commit-Position: refs/heads/master@{#439436}

[bashi, 2016-12-19 07:29:13]

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

[commit-bot: I haz the power, 2016-12-19 07:29:27]

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

[bashi, 2016-12-19 07:30:24]

bashi@chromium.org changed reviewers:
+ haraken@chromium.org, skyostil@chromium.org, tasak@google.com

[bashi, 2016-12-19 07:30:24]

PTAL

[haraken, 2016-12-19 07:32:18]

LGTM

[commit-bot: I haz the power, 2016-12-19 08:47:16]

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

[commit-bot: I haz the power, 2016-12-19 08:47:17]

Dry run: This issue passed the CQ dry run.

[bashi, 2016-12-19 08:47:31]

I'm going to submit this.

Sami, please let me know if you have concerns and/or feedback on this CL.

[bashi, 2016-12-19 08:47:36]

The CQ bit was checked by bashi@chromium.org

[commit-bot: I haz the power, 2016-12-19 08:47: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-19 08:51:33]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1482137256478220, "parent_rev":
"1b7ad89891159167d4e957884459ff8fd302f4d1", "commit_rev":
"7e5614acdc7f34823af4256b855437439aae27ea"}

[commit-bot: I haz the power, 2016-12-19 08:52:00]

Description was changed from

==========
Early return in {Suspend,Resume}Renderer() if not backgrounded

After crrev.com/2564833002, RenderThreadImpl calls SuspendRenderer() and
ResumeRenderer() unconditionally. This could cause assertion failures
when the renderer isn't backgrounded.  Rather than reverting
crrev.com/2564833002, do early return when not backgrounded because
having multiple flags to check whether a renderer is suspended doesn't
sound a good idea.

BUG=674784
==========

to

==========
Early return in {Suspend,Resume}Renderer() if not backgrounded

After crrev.com/2564833002, RenderThreadImpl calls SuspendRenderer() and
ResumeRenderer() unconditionally. This could cause assertion failures
when the renderer isn't backgrounded.  Rather than reverting
crrev.com/2564833002, do early return when not backgrounded because
having multiple flags to check whether a renderer is suspended doesn't
sound a good idea.

BUG=674784

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

[commit-bot: I haz the power, 2016-12-19 08:52:01]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-12-19 08:54:07]

Description was changed from

==========
Early return in {Suspend,Resume}Renderer() if not backgrounded

After crrev.com/2564833002, RenderThreadImpl calls SuspendRenderer() and
ResumeRenderer() unconditionally. This could cause assertion failures
when the renderer isn't backgrounded.  Rather than reverting
crrev.com/2564833002, do early return when not backgrounded because
having multiple flags to check whether a renderer is suspended doesn't
sound a good idea.

BUG=674784

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

to

==========
Early return in {Suspend,Resume}Renderer() if not backgrounded

After crrev.com/2564833002, RenderThreadImpl calls SuspendRenderer() and
ResumeRenderer() unconditionally. This could cause assertion failures
when the renderer isn't backgrounded.  Rather than reverting
crrev.com/2564833002, do early return when not backgrounded because
having multiple flags to check whether a renderer is suspended doesn't
sound a good idea.

BUG=674784

Committed: https://crrev.com/0a7c6fb8848b1a0100cbafae461a143eb2c83362
Cr-Commit-Position: refs/heads/master@{#439436}
==========

[commit-bot: I haz the power, 2016-12-19 08:54:08]

Patchset 1 (id:??) landed as
https://crrev.com/0a7c6fb8848b1a0100cbafae461a143eb2c83362
Cr-Commit-Position: refs/heads/master@{#439436}

[Sami, 2016-12-19 10:47:49]

lgtm, thanks for the fix. My only wish would have been a small test case to go
with it :)

At some point we might want to turn these bits into an explicit state machine
with better defined transitions to make it obvious how the various modes are
related.

[bashi, 2016-12-20 00:58:17]

On 2016/12/19 10:47:49, Sami wrote:
> lgtm, thanks for the fix. My only wish would have been a small test case to go
> with it :)
Definitely I should have added tests. Uploaded a follow-up CL.
https://codereview.chromium.org/2583193003
> 
> At some point we might want to turn these bits into an explicit state machine
> with better defined transitions to make it obvious how the various modes are
> related.