Limit renderer saved frames to avoid running out of fds.

Limit renderer saved frames to avoid running out of fds.

Software delegated rendering uses one fd per tile, so with a bunch of tiles that means it can run out of fds. If it seems close to hitting the limit the browser should throw away old frames to avoid this from happening.

BUG=362603
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266383

[piman, 2014-04-23 00:32:02]

https://codereview.chromium.org/248193003/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/renderer_frame_manager.cc (right):

https://codereview.chromium.org/248193003/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/renderer_frame_manager.cc:89:
static_cast<uint32>((saved_frame_limit - 2) * fraction_to_reduce);
I'm not sure I agree with this computation. If we're right at the handle limit
with 5 frames and max_number_of_saved_frames() == 5, we'll end up with
saved_frame_limit==2, so we'll discard frames even though we fit.

How about something like:
int max_handles = base::SharedMemory::GetHandleLimit() * kHandleMaxFraction; 
(it's actually a constant, could be done in constructor)
int handles_per_frame =
HostSharedBitmapManager::current()->AllocatedBitmapCount() /
(unlocked_frames_.size() + locked_frames_.size())
int saved_frame_limit = std::max(1, std::min(max_number_of_saved_frames(),
max_handles/handles_per_frame))

https://codereview.chromium.org/248193003/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/renderer_frame_manager.cc:90: }
RenderWidgetHostViewAuraTest.DiscardDelegatedFrames/DiscardDelegatedFramesWithLocking
test current behavior, they'll have to be modified (ideally, extended to take
this into account).

[jbauman, 2014-04-25 02:27:52]

PTAL. Modified it to use your algorithm (it does seem better), and added a test
for throwing frames away based on the number of handles used. Existing tests are
fine because they don't create shared bitmaps and so won't run into any limits.

[piman, 2014-04-25 02:39:48]

lgtm

https://codereview.chromium.org/248193003/diff/20001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
(right):

https://codereview.chromium.org/248193003/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:1166:
bitmaps.push_back(cc::SharedBitmap::GenerateId());
nit: Do you need to pass those around? You're not using the filled vector
anywhere.

https://codereview.chromium.org/248193003/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:1183:
base::GetCurrentProcessHandle());
I see what you did there :)

[jbauman, 2014-04-25 18:07:31]

The CQ bit was checked by jbauman@chromium.org

[jbauman, 2014-04-25 19:43:56]

The CQ bit was unchecked by jbauman@chromium.org

[jbauman, 2014-04-25 19:44:00]

The CQ bit was checked by jbauman@chromium.org

[commit-bot: I haz the power, 2014-04-25 22:09:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbauman@chromium.org/248193003/40001

[commit-bot: I haz the power, 2014-04-25 23:19:49]

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

[commit-bot: I haz the power, 2014-04-25 23:19:49]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[jbauman, 2014-04-25 23:23:20]

The CQ bit was checked by jbauman@chromium.org

[commit-bot: I haz the power, 2014-04-25 23:26:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbauman@chromium.org/248193003/40001

[commit-bot: I haz the power, 2014-04-26 22:17:54]

Change committed as 266383

[Nico, 2014-04-28 15:49:21]

A revert of this CL has been created in
https://codereview.chromium.org/258093002/ by thakis@chromium.org.

The reason for reverting is: Somewhat speculative.
SoftwareFrameManagerTest.DoNotEvictVisible started failing reliably on the
valgrind bots after this landed. Example:
http://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20OS%20%28v...

http://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20OS%20%28v....