Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(402)

Issue 940993002: Revert of Handle RenderProcessHostImpl::SetBackgrounded on Windows (Closed)

Created:
5 years, 10 months ago by Devlin
Modified:
5 years, 10 months ago
Reviewers:
gab, jam, Will Harris
CC:
chromium-reviews, darin-cc_chromium.org, nasko+codewatch_chromium.org, creis+watch_chromium.org, cpu_(ooo_6.6-7.5), ananta, DaleCurtis
Base URL:
https://chromium.googlesource.com/chromium/src.git@908473002_ps2
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Handle RenderProcessHostImpl::SetBackgrounded on Windows (patchset #4 id:60001 of https://codereview.chromium.org/926663002/) Reason for revert: This patch set re-enabled ChromeRenderProcessHostTest.Backgrounding, which is now failing on XP bots. Example logs: https://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%281%29/builds/35740/steps/browser_tests%20on%20Windows-XP-SP3/logs/ChromeRenderProcessHostTest.Backgrounding https://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%281%29/builds/35740/steps/browser_tests%20on%20Windows-XP-SP3/logs/ChromeRenderProcessHostTest.Backgrounding ChromeRenderProcessHostTest.Backgrounding (run #1): [ RUN ] ChromeRenderProcessHostTest.Backgrounding [2904:3280:0219/073309:WARNING:data_reduction_proxy_settings.cc(365)] SPDY proxy OFF at startup [2904:2900:0219/073309:WARNING:raw_channel.cc(210)] Shutting down RawChannel with write buffer nonempty [2904:2900:0219/073309:WARNING:raw_channel.cc(210)] Shutting down RawChannel with write buffer nonempty [2904:2900:0219/073310:WARNING:raw_channel.cc(210)] Shutting down RawChannel with write buffer nonempty [2904:2900:0219/073311:WARNING:raw_channel.cc(210)] Shutting down RawChannel with write buffer nonempty [2904:2900:0219/073311:WARNING:raw_channel.cc(210)] Shutting down RawChannel with write buffer nonempty [2904:2900:0219/073311:WARNING:raw_channel.cc(210)] Shutting down RawChannel with write buffer nonempty [2904:2900:0219/073311:WARNING:raw_channel.cc(210)] Shutting down RawChannel with write buffer nonempty [2904:2900:0219/073311:WARNING:raw_channel.cc(210)] Shutting down RawChannel with write buffer nonempty [2904:2900:0219/073311:WARNING:raw_channel.cc(210)] Shutting down RawChannel with write buffer nonempty [2904:2900:0219/073311:WARNING:raw_channel.cc(210)] Shutting down RawChannel with write buffer nonempty [2904:3280:0219/073311:WARNING:pref_notifier_impl.cc(27)] pref observer found at shutdown printing.enabled [2904:3280:0219/073311:WARNING:pref_notifier_impl.cc(27)] pref observer found at shutdown plugins.allow_outdated [2904:3280:0219/073311:WARNING:pref_notifier_impl.cc(27)] pref observer found at shutdown plugins.always_authorize Backtrace: (No symbol) [0x61666544] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x02DCFC78+33833672] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x029F5C04+29794900] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x02DCFB91+33833441] ovly_debug_event [0x00CA5D1A+6638122] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x034FBB93+41353699] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x034FBC4B+41353883] ovly_debug_event [0x00CEFA46+6940502] ovly_debug_event [0x00CF020A+6942490] ovly_debug_event [0x00CF031B+6942763] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x036A59F8+43098184] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x0182F2F3+11155779] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x019E8F4E+12965278] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x019E4BA4+12947956] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x019E4BE1+12948017] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x019E6482+12954322] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x00E4800A+771674] ovly_debug_event [0x0086BD9B+2206379] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01759607+10280023] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x017A2153+10577827] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x03DBD414+50535012] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x03C7FF7E+49235406] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x03C7FE36+49235078] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x03C7F3D3+49232419] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x03145348+37460376] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x00E075EB+506939] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01085C44+3121812] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01085DB9+3122185] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x01086389+3123673] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x010860D6+3122982] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x00E8BFCD+1050141] ovly_debug_event [0x00A75018+4341032] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x0313E1D8+37431336] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x00E343D6+690726] ovly_debug_event [0x00A75080+4341136] RelaunchChromeBrowserWithNewCommandLineIfNeeded [0x033E849A+40225514] RegisterWaitForInputIdle [0x7C816037+73] Original issue's description: > Handle RenderProcessHostImpl::SetBackgrounded on Windows > > This was disabled on Windows because: > > - http://crrev.com/274071 made it such that it was asynchronous via the renderer (as only > the process itself can set itself in background mode on Windows). > > - And then this was removed for Windows as part of issue 381820 as true background mode on > Windows was achieved by having the renderer itself change its process priority (which > doesn't work as the unbackgrounding request can be stuck behind other priority tasks > running in background mode). > > Also, the test for this was disabled in between (see issue 394368) -- the asynchronicity > introduced in r274071 probably causing the flakes. > > Overall there is no need to have the renderer enter PROCESS_MODE_BACKGROUND_BEGIN itself, > IDLE_PRIORITY_CLASS should be sufficient as its the same as far as CPU priority is > concerned and IO is irrelevant in the renderers. > > Thus we can get background mode for hidden renderers without running into issue 381820. > > Experiment with IDLE_PRIORITY_CLASS/BELOW_NORMAL_PRIORITY_CLASS vs the status quo. > > Watching the relevant perf bots (tab_switching.top_10 -> MPArch.RWH_TabSwitchPaintDuration) > and UMA metrics MPArch.RWH_TabSwitchPaintDuration which shouldn't regress. > And others (e.g., SessionRestore related) which will hopefully improve. > > Enable the experiment by default in the absence of Finch to get perf waterfall coverage. > > The experiment will be called "BackgroundRendererProcesses" and have 4 groups: > ["Disallow", "AllowBelowNormalFromBrowser", > "AllowIdleFromBrowser", "AllowBackgroundModeFromRenderer"] > > It will be a per-session (rather than per-profile) experiment, randomly assigning a user > to a group every new Chrome session. > > BUG=394368, 458594 > > Committed: https://crrev.com/aaf738a2cce69ce952386b48e3bc252bd307ef43 > Cr-Commit-Position: refs/heads/master@{#317042} TBR=wfh@chromium.org,jam@chromium.org,gab@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=394368, 458594 Committed: https://crrev.com/126d77b69d28f4e84117ae655f7414222982c881 Cr-Commit-Position: refs/heads/master@{#317070}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -54 lines) Patch
M base/process/process_win.cc View 2 chunks +1 line, -16 lines 0 comments Download
M chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc View 1 chunk +7 lines, -1 line 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 1 chunk +9 lines, -19 lines 0 comments Download
M content/child/child_thread_impl.cc View 2 chunks +0 lines, -18 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Devlin
Created Revert of Handle RenderProcessHostImpl::SetBackgrounded on Windows
5 years, 10 months ago (2015-02-19 17:38:37 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/940993002/1
5 years, 10 months ago (2015-02-19 17:39:09 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 10 months ago (2015-02-19 17:39:49 UTC) #3
commit-bot: I haz the power
5 years, 10 months ago (2015-02-19 17:40:51 UTC) #4
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/126d77b69d28f4e84117ae655f7414222982c881
Cr-Commit-Position: refs/heads/master@{#317070}

Powered by Google App Engine
This is Rietveld 408576698