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

Issue 10879050: Revert 139366 - [OSX/Pepper] Don't have browser cache shmem ref behind ImageData. (Closed)

Created:
8 years, 4 months ago by sail
Modified:
8 years, 4 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam
Visibility:
Public.

Description

Revert 139366 - [OSX/Pepper] Don't have browser cache shmem ref behind ImageData. On OSX the browser has to create shmem blocks for sandboxed processes. The browser process can keep a handle for synchronizing between processes by id, but pepper's implementation just passes the handle between processes so that is not necessary. Before, the browser ran out of handles which breaks all sorts of stuff, but everything kept running, with various visual glitches (and no fds in browser is really bad). With this change, the renderer crashes. That sounds bad, but is actually better in terms of resolving the underlying problems. Also, histogram sizes for all platforms. Using a TransportDIB for small images is probably a bad idea. The histograms should show how things are being used in the wild. BUG=129430, 141541 TEST=flapper works, histograms appear. Review URL: https://chromiumcodereview.appspot.com/10424007 TBR=shess@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=153136

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -5 lines) Patch
M content/renderer/pepper/pepper_platform_image_2d_impl.cc View 3 chunks +11 lines, -5 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
sail
8 years, 4 months ago (2012-08-23 20:40:38 UTC) #1
sail
Hey Scott, I can't reproduce the crash anymore. I think it may have been due ...
8 years, 4 months ago (2012-08-23 20:42:49 UTC) #2
Scott Hess - ex-Googler
lgtm. thx
8 years, 4 months ago (2012-08-23 20:51:25 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sail@chromium.org/10879050/1
8 years, 4 months ago (2012-08-23 21:38:20 UTC) #4
commit-bot: I haz the power
Try job failure for 10879050-1 (retry) on linux_rel for step "interactive_ui_tests". It's a second try, ...
8 years, 4 months ago (2012-08-23 23:16:19 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sail@chromium.org/10879050/1
8 years, 4 months ago (2012-08-23 23:47:46 UTC) #6
commit-bot: I haz the power
8 years, 4 months ago (2012-08-24 01:28:08 UTC) #7
Change committed as 153136

Powered by Google App Engine
This is Rietveld 408576698