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

Issue 2799213003: Remove deprecated functions from Canvas in favor of the RectF version. (Closed)

Created:
3 years, 8 months ago by Uzair
Modified:
3 years, 8 months ago
CC:
chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove deprecated functions from Canvas in favor of the RectF version. BUG=553726 Review-Url: https://codereview.chromium.org/2799213003 Cr-Commit-Position: refs/heads/master@{#463518} Committed: https://chromium.googlesource.com/chromium/src/+/81137314dd619ccb9b3ca3475616a47447a99c58

Patch Set 1 #

Total comments: 4

Patch Set 2 : Fixed compile error in content_analysis_unittest #

Total comments: 4

Patch Set 3 : Addressed review comments #

Total comments: 2

Patch Set 4 : Remove deprecated functions from Canvas in favor of the RectF version #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -42 lines) Patch
M chrome/browser/thumbnails/content_analysis_unittest.cc View 1 2 3 7 chunks +9 lines, -9 lines 0 comments Download
M ui/gfx/canvas.h View 1 2 chunks +0 lines, -21 lines 0 comments Download
M ui/gfx/canvas.cc View 1 2 chunks +0 lines, -12 lines 0 comments Download

Messages

Total messages: 36 (16 generated)
Uzair
On 2017/04/06 13:17:52, Uzair wrote: > mailto:uzair.jaleel@samsung.com changed reviewers: > + mailto:mgiuca@chromium.org Dear Matt, PTAL. ...
3 years, 8 months ago (2017-04-06 13:19:30 UTC) #3
Matt Giuca
Hi, this lgtm. Please update the CL description: - Capitalize 'C' in Canvas so it's ...
3 years, 8 months ago (2017-04-07 03:40:45 UTC) #8
Matt Giuca
I checked all the other usages of deprecated methods in this class, and they all ...
3 years, 8 months ago (2017-04-07 03:47:54 UTC) #9
Uzair
Done https://codereview.chromium.org/2799213003/diff/1/ui/gfx/canvas.h File ui/gfx/canvas.h (left): https://codereview.chromium.org/2799213003/diff/1/ui/gfx/canvas.h#oldcode244 ui/gfx/canvas.h:244: void DrawRect(const Rect& rect, SkColor color); On 2017/04/07 ...
3 years, 8 months ago (2017-04-07 05:30:59 UTC) #12
Matt Giuca
https://codereview.chromium.org/2799213003/diff/20001/chrome/browser/thumbnails/content_analysis_unittest.cc File chrome/browser/thumbnails/content_analysis_unittest.cc (right): https://codereview.chromium.org/2799213003/diff/20001/chrome/browser/thumbnails/content_analysis_unittest.cc#newcode132 chrome/browser/thumbnails/content_analysis_unittest.cc:132: canvas.DrawRect(gfx::RectF(draw_rect), SkColorSetRGB(255, 255, 255)); Don't cast the Rect to ...
3 years, 8 months ago (2017-04-07 06:38:08 UTC) #13
Uzair
Done https://codereview.chromium.org/2799213003/diff/20001/chrome/browser/thumbnails/content_analysis_unittest.cc File chrome/browser/thumbnails/content_analysis_unittest.cc (right): https://codereview.chromium.org/2799213003/diff/20001/chrome/browser/thumbnails/content_analysis_unittest.cc#newcode132 chrome/browser/thumbnails/content_analysis_unittest.cc:132: canvas.DrawRect(gfx::RectF(draw_rect), SkColorSetRGB(255, 255, 255)); On 2017/04/07 06:38:08, Matt ...
3 years, 8 months ago (2017-04-07 08:50:15 UTC) #14
msw
Nice, lgtm with a cautionary comment/q (probably fine as-is). https://codereview.chromium.org/2799213003/diff/40001/chrome/browser/thumbnails/content_analysis_unittest.cc File chrome/browser/thumbnails/content_analysis_unittest.cc (right): https://codereview.chromium.org/2799213003/diff/40001/chrome/browser/thumbnails/content_analysis_unittest.cc#newcode663 chrome/browser/thumbnails/content_analysis_unittest.cc:663: ...
3 years, 8 months ago (2017-04-07 10:12:23 UTC) #15
Uzair
PTAL https://codereview.chromium.org/2799213003/diff/40001/chrome/browser/thumbnails/content_analysis_unittest.cc File chrome/browser/thumbnails/content_analysis_unittest.cc (right): https://codereview.chromium.org/2799213003/diff/40001/chrome/browser/thumbnails/content_analysis_unittest.cc#newcode663 chrome/browser/thumbnails/content_analysis_unittest.cc:663: gfx::RectF pict_rect(x, y, body_rect.width() / 2 - 2 ...
3 years, 8 months ago (2017-04-07 10:53:55 UTC) #16
Uzair
On 2017/04/07 10:53:55, Uzair wrote: > PTAL > > https://codereview.chromium.org/2799213003/diff/40001/chrome/browser/thumbnails/content_analysis_unittest.cc > File chrome/browser/thumbnails/content_analysis_unittest.cc (right): > ...
3 years, 8 months ago (2017-04-07 11:03:01 UTC) #17
Uzair
On 2017/04/07 11:03:01, Uzair wrote: > On 2017/04/07 10:53:55, Uzair wrote: > > PTAL > ...
3 years, 8 months ago (2017-04-07 16:10:10 UTC) #18
Uzair
On 2017/04/07 16:10:10, Uzair wrote: > On 2017/04/07 11:03:01, Uzair wrote: > > On 2017/04/07 ...
3 years, 8 months ago (2017-04-07 16:11:08 UTC) #19
msw
lgtm
3 years, 8 months ago (2017-04-07 17:00:32 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2799213003/50001
3 years, 8 months ago (2017-04-09 12:53:01 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/406363)
3 years, 8 months ago (2017-04-09 12:59:10 UTC) #25
Uzair
On 2017/04/09 12:59:10, commit-bot: I haz the power wrote: > Try jobs failed on following ...
3 years, 8 months ago (2017-04-10 04:11:35 UTC) #27
Lei Zhang
On 2017/04/10 04:11:35, Uzair wrote: > Dear Reviewers, > > Need review for chrome/browser/thumbnails/content_analysis_unittest.cc > ...
3 years, 8 months ago (2017-04-10 17:56:22 UTC) #29
Uzair
On 2017/04/10 17:56:22, Lei Zhang wrote: > On 2017/04/10 04:11:35, Uzair wrote: > > Dear ...
3 years, 8 months ago (2017-04-11 01:01:08 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2799213003/50001
3 years, 8 months ago (2017-04-11 01:02:12 UTC) #32
commit-bot: I haz the power
Committed patchset #4 (id:50001) as https://chromium.googlesource.com/chromium/src/+/81137314dd619ccb9b3ca3475616a47447a99c58
3 years, 8 months ago (2017-04-11 03:28:18 UTC) #35
danakj
3 years, 8 months ago (2017-04-12 16:00:34 UTC) #36
Message was sent while issue was closed.
LGTM

Powered by Google App Engine
This is Rietveld 408576698