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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 184743002: don't create SkDevice directly, use SkBitmap or (better) SkCanvas::NewRaster factory (Closed)

Created:
6 years, 9 months ago by reed1
Modified:
6 years, 9 months ago
CC:
chromium-reviews, extensions-reviews_chromium.org, feature-media-reviews_chromium.org, kalyank, nona+watch_chromium.org, yusukes+watch_chromium.org, penghuang+watch_chromium.org, yukishiino+watch_chromium.org, sievers+watch_chromium.org, jbauman+watch_chromium.org, danakj+watch_chromium.org, joi+watch-content_chromium.org, darin-cc_chromium.org, piman+watch_chromium.org, chromium-apps-reviews_chromium.org, cc-bugs_chromium.org, James Su, android-webview-reviews_chromium.org, miu+watch_chromium.org
Visibility:
Public.

Description

don't create SkDevice directly, use SkBitmap or (better) SkCanvas::NewRaster factory BUG=skia:2239 TBR=scherkus@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=254567 reopened (after revewer) to address media_unittest valgrind issue Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255137

Patch Set 1 #

Total comments: 2

Patch Set 2 : fix indentation #

Total comments: 2

Patch Set 3 : #

Patch Set 4 : was reverted (valgrind errors in media unittest) so rebasing to fix those #

Patch Set 5 : erase bitmap if not opaque in media unittest #

Patch Set 6 : revert changes to media unittests til we understand valgrind errors #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -41 lines) Patch
M android_webview/browser/browser_view_renderer.cc View 1 2 3 4 5 2 chunks +4 lines, -3 lines 0 comments Download
M android_webview/native/java_browser_view_renderer_helper.cc View 1 2 2 chunks +5 lines, -9 lines 0 comments Download
M cc/output/gl_renderer.cc View 1 2 3 4 5 1 chunk +1 line, -2 lines 0 comments Download
M cc/resources/resource_provider.cc View 1 2 3 1 chunk +1 line, -3 lines 0 comments Download
M chrome/browser/extensions/extension_action.cc View 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/compositor/software_output_device_ozone.cc View 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/renderer_host/render_widget_host_view_browsertest.cc View 1 2 chunks +5 lines, -8 lines 0 comments Download
M content/renderer/skia_benchmarking_extension.cc View 2 chunks +3 lines, -5 lines 0 comments Download
M skia/ext/benchmarking_canvas.cc View 2 chunks +1 line, -4 lines 0 comments Download
M skia/tools/filter_fuzz_stub/filter_fuzz_stub.cc View 2 chunks +2 lines, -5 lines 0 comments Download

Messages

Total messages: 27 (0 generated)
reed1
should be search/replace of the following pattern: SkBitmapDevice device(...) SkCanvas canvas(&device) becoming SkCanvas canvas(...) as ...
6 years, 9 months ago (2014-02-28 17:18:05 UTC) #1
reed1
compile failure in #1 is just mismatch between LKGR and tip-of-tree (which has this new ...
6 years, 9 months ago (2014-02-28 17:34:31 UTC) #2
jam
content & chrome lgtm https://codereview.chromium.org/184743002/diff/1/content/browser/renderer_host/render_widget_host_view_browsertest.cc File content/browser/renderer_host/render_widget_host_view_browsertest.cc (right): https://codereview.chromium.org/184743002/diff/1/content/browser/renderer_host/render_widget_host_view_browsertest.cc#newcode584 content/browser/renderer_host/render_widget_host_view_browsertest.cc:584: bitmap.allocPixels(SkImageInfo::Make(video_frame->visible_rect().width(), nit: indentation
6 years, 9 months ago (2014-02-28 18:09:50 UTC) #3
reed1
https://codereview.chromium.org/184743002/diff/1/content/browser/renderer_host/render_widget_host_view_browsertest.cc File content/browser/renderer_host/render_widget_host_view_browsertest.cc (right): https://codereview.chromium.org/184743002/diff/1/content/browser/renderer_host/render_widget_host_view_browsertest.cc#newcode584 content/browser/renderer_host/render_widget_host_view_browsertest.cc:584: bitmap.allocPixels(SkImageInfo::Make(video_frame->visible_rect().width(), On 2014/02/28 18:09:50, jam wrote: > nit: indentation ...
6 years, 9 months ago (2014-02-28 18:44:23 UTC) #4
enne (OOO)
cc lgtm Is there a bug (Chromium or Skia) that you can link to in ...
6 years, 9 months ago (2014-02-28 19:10:32 UTC) #5
boliu
android_webview lgtm % compile error https://codereview.chromium.org/184743002/diff/10001/android_webview/native/java_browser_view_renderer_helper.cc File android_webview/native/java_browser_view_renderer_helper.cc (right): https://codereview.chromium.org/184743002/diff/10001/android_webview/native/java_browser_view_renderer_helper.cc#newcode158 android_webview/native/java_browser_view_renderer_helper.cc:158: SkImageInfo info = MakeN32Premul(bitmap_info.width, ...
6 years, 9 months ago (2014-02-28 19:36:30 UTC) #6
reed1
6 years, 9 months ago (2014-02-28 19:51:46 UTC) #7
reed1
https://codereview.chromium.org/184743002/diff/10001/android_webview/native/java_browser_view_renderer_helper.cc File android_webview/native/java_browser_view_renderer_helper.cc (right): https://codereview.chromium.org/184743002/diff/10001/android_webview/native/java_browser_view_renderer_helper.cc#newcode158 android_webview/native/java_browser_view_renderer_helper.cc:158: SkImageInfo info = MakeN32Premul(bitmap_info.width, bitmap_info.height); On 2014/02/28 19:36:31, boliu ...
6 years, 9 months ago (2014-02-28 19:52:08 UTC) #8
reed1
On 2014/02/28 19:10:32, enne wrote: > cc lgtm > > Is there a bug (Chromium ...
6 years, 9 months ago (2014-02-28 20:14:08 UTC) #9
reed2
The CQ bit was checked by reed@chromium.org
6 years, 9 months ago (2014-03-01 13:32:26 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/reed@google.com/184743002/30001
6 years, 9 months ago (2014-03-01 13:32:40 UTC) #11
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-01 14:10:42 UTC) #12
commit-bot: I haz the power
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=52916
6 years, 9 months ago (2014-03-01 14:10:42 UTC) #13
reed2
still need OWNER for media/
6 years, 9 months ago (2014-03-01 14:48:44 UTC) #14
reed1
looking for media/ owner
6 years, 9 months ago (2014-03-03 14:03:29 UTC) #15
jam
On 2014/03/03 14:03:29, reed1 wrote: > looking for media/ owner note it's fine to tbr ...
6 years, 9 months ago (2014-03-03 16:13:25 UTC) #16
reed1
On 2014/03/03 16:13:25, jam wrote: > On 2014/03/03 14:03:29, reed1 wrote: > > looking for ...
6 years, 9 months ago (2014-03-03 16:26:55 UTC) #17
reed1
The CQ bit was checked by reed@google.com
6 years, 9 months ago (2014-03-03 17:36:14 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/reed@google.com/184743002/30001
6 years, 9 months ago (2014-03-03 17:39:20 UTC) #19
Ami GONE FROM CHROMIUM
drive-by On Mon, Mar 3, 2014 at 8:26 AM, <reed@google.com> wrote: > Ah. Are there ...
6 years, 9 months ago (2014-03-03 18:14:25 UTC) #20
jam
On 2014/03/03 16:26:55, reed1 wrote: > On 2014/03/03 16:13:25, jam wrote: > > On 2014/03/03 ...
6 years, 9 months ago (2014-03-03 18:16:20 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/reed@google.com/184743002/30001
6 years, 9 months ago (2014-03-03 18:28:47 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/reed@google.com/184743002/30001
6 years, 9 months ago (2014-03-03 19:55:03 UTC) #23
commit-bot: I haz the power
Change committed as 254567
6 years, 9 months ago (2014-03-03 21:30:20 UTC) #24
reed1
The CQ bit was checked by reed@google.com
6 years, 9 months ago (2014-03-05 16:54:01 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/reed@google.com/184743002/90001
6 years, 9 months ago (2014-03-05 16:54:28 UTC) #26
commit-bot: I haz the power
6 years, 9 months ago (2014-03-05 20:21:54 UTC) #27
Message was sent while issue was closed. 
Change committed as 255137

Powered by Google App Engine
This is Rietveld 408576698