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

Issue 263453007: Implement "Save image as" for canvas (chromium side). (Closed)

Created:
6 years, 7 months ago by zino
Modified:
6 years, 7 months ago
CC:
chromium-reviews, asanka, creis+watch_chromium.org, benjhayden+dwatch_chromium.org, nasko+codewatch_chromium.org, darin-cc_chromium.org
Base URL:
http://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Implement "Save image as" for canvas (chromium side). Many users want to save the <canvas> as a image file. (like <img>) As well as being useful, The feature is already supported in Firefox and IE. Blink side: https://codereview.chromium.org/255563004/ BUG=19277, 129710 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269171

Patch Set 1 : #

Total comments: 6

Patch Set 2 : #

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+80 lines, -11 lines) Patch
M chrome/browser/renderer_context_menu/context_menu_content_type.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/renderer_context_menu/context_menu_content_type.cc View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/renderer_context_menu/render_view_context_menu.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/renderer_context_menu/render_view_context_menu.cc View 1 2 4 chunks +28 lines, -6 lines 0 comments Download
M chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc View 1 1 chunk +13 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_message_filter.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/renderer_host/render_message_filter.cc View 1 chunk +3 lines, -1 line 0 comments Download
M content/browser/renderer_host/render_view_host_impl.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/renderer_host/render_view_host_impl.cc View 1 1 chunk +4 lines, -0 lines 0 comments Download
M content/common/view_messages.h View 1 2 chunks +9 lines, -2 lines 0 comments Download
M content/public/browser/render_view_host.h View 1 chunk +4 lines, -0 lines 0 comments Download
M content/renderer/render_frame_impl.cc View 1 1 chunk +5 lines, -1 line 0 comments Download
M content/renderer/render_view_impl.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M content/renderer/render_view_impl.cc View 1 2 chunks +5 lines, -0 lines 0 comments Download

Messages

Total messages: 21 (0 generated)
zino
Please take a look. Thank you :)
6 years, 7 months ago (2014-04-30 10:53:41 UTC) #1
Avi (use Gerrit)
+jam for thoughts about the content API. https://codereview.chromium.org/263453007/diff/20001/chrome/browser/renderer_context_menu/render_view_context_menu.cc File chrome/browser/renderer_context_menu/render_view_context_menu.cc (right): https://codereview.chromium.org/263453007/diff/20001/chrome/browser/renderer_context_menu/render_view_context_menu.cc#newcode855 chrome/browser/renderer_context_menu/render_view_context_menu.cc:855: // FIXME: ...
6 years, 7 months ago (2014-04-30 14:52:28 UTC) #2
Justin Novosad
I realize that implementing the feature this way is a lot more work that your ...
6 years, 7 months ago (2014-04-30 15:07:15 UTC) #3
zino
On 2014/04/30 15:07:15, junov wrote: > I realize that implementing the feature this way is ...
6 years, 7 months ago (2014-05-01 15:39:42 UTC) #4
zino
Thank you for review! On 2014/04/30 14:52:28, Avi wrote: > +jam for thoughts about the ...
6 years, 7 months ago (2014-05-01 15:45:47 UTC) #5
Justin Novosad
non-owner lgtm
6 years, 7 months ago (2014-05-01 17:40:45 UTC) #6
Avi (use Gerrit)
This LGTM, though can you follow up to route the existing "save image" code through ...
6 years, 7 months ago (2014-05-01 18:10:58 UTC) #7
jam
On 2014/05/01 18:10:58, Avi wrote: > This LGTM, though can you follow up to route ...
6 years, 7 months ago (2014-05-01 18:13:25 UTC) #8
zino
On 2014/05/01 18:10:58, Avi wrote: > This LGTM, though can you follow up to route ...
6 years, 7 months ago (2014-05-07 08:50:12 UTC) #9
zino
The CQ bit was checked by jinho.bang@samsung.com
6 years, 7 months ago (2014-05-07 10:16:44 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jinho.bang@samsung.com/263453007/80001
6 years, 7 months ago (2014-05-07 10:21:44 UTC) #11
zino
The CQ bit was unchecked by jinho.bang@samsung.com
6 years, 7 months ago (2014-05-07 11:19:21 UTC) #12
zino
The CQ bit was checked by jinho.bang@samsung.com
6 years, 7 months ago (2014-05-07 11:19:22 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jinho.bang@samsung.com/263453007/80001
6 years, 7 months ago (2014-05-07 11:26:49 UTC) #14
zino
The CQ bit was unchecked by jinho.bang@samsung.com
6 years, 7 months ago (2014-05-07 13:34:19 UTC) #15
zino
Dear @jln, This CL seems to need your approval. (content/common/view_messages.h) Could you please review?
6 years, 7 months ago (2014-05-07 13:50:10 UTC) #16
dcheng
rslgtm for view_messages.h changes.
6 years, 7 months ago (2014-05-09 00:25:55 UTC) #17
zino
The CQ bit was checked by jinho.bang@samsung.com
6 years, 7 months ago (2014-05-09 00:38:25 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jinho.bang@samsung.com/263453007/80001
6 years, 7 months ago (2014-05-09 00:43:18 UTC) #19
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are ...
6 years, 7 months ago (2014-05-09 04:32:25 UTC) #20
commit-bot: I haz the power
6 years, 7 months ago (2014-05-09 06:57:22 UTC) #21
Message was sent while issue was closed.
Change committed as 269171

Powered by Google App Engine
This is Rietveld 408576698