Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(52)

Issue 673623002: Fix regions generation on windows. (Closed)

Created:
5 years, 5 months ago by alex-ac
Modified:
5 years, 4 months ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Fix regions generation on windows. On windows HRGN for window is generated from the SkPath using dumb function that doesn`t respects anything except simple polygons. It's impossible to create window with round corners on windows. Replacing implementation of that function with a new one will fix this. R=asvitkine@chromium.org BUG=322360 Committed: https://crrev.com/2cb37ad893097cd5c19184c7f2b937df59b24996 Cr-Commit-Position: refs/heads/master@{#304045}

Patch Set 1 #

Total comments: 1

Patch Set 2 : Add more tests. Fix windows without mask. #

Total comments: 22

Patch Set 3 : Style fixes. #

Patch Set 4 : Yet another style fix. #

Patch Set 5 : CreateHRGNFromSkPath can return empty region now. #

Total comments: 12

Patch Set 6 : Fix review issues. #

Total comments: 10

Patch Set 7 : Fix build. #

Patch Set 8 : Fix review issues. #

Total comments: 4

Patch Set 9 : Fix update region conditions. #

Total comments: 4

Patch Set 10 : Improve readability. Fix missed release. #

Patch Set 11 : Add static casts. #

Patch Set 12 : Fix build. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+138 lines, -26 lines) Patch
M chrome/browser/ui/views/panels/panel_frame_view.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +8 lines, -4 lines 0 comments Download
M ui/gfx/gfx_tests.gyp View 1 chunk +1 line, -0 lines 0 comments Download
M ui/gfx/path_win.h View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M ui/gfx/path_win.cc View 2 3 4 1 chunk +5 lines, -10 lines 0 comments Download
A ui/gfx/path_win_unittest.cc View 1 2 3 4 5 6 7 1 chunk +111 lines, -0 lines 0 comments Download
M ui/views/win/hwnd_message_handler.cc View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +12 lines, -11 lines 0 comments Download

Messages

Total messages: 41 (11 generated)
alex-ac
5 years, 5 months ago (2014-10-22 13:05:35 UTC) #1
Alexei Svitkine (slow)
Looks like a similar thing was attempted in https://codereview.chromium.org/83293008 but later reverted. Added the people ...
5 years, 5 months ago (2014-10-22 15:14:47 UTC) #3
Wez
On 2014/10/22 15:14:47, Alexei Svitkine wrote: > Looks like a similar thing was attempted in ...
5 years, 5 months ago (2014-10-22 18:37:23 UTC) #4
Wez
nit: You have some typos in your CL description. :)
5 years, 5 months ago (2014-10-22 18:38:13 UTC) #5
alex-ac
On 2014/10/22 15:14:47, Alexei Svitkine wrote: > https://codereview.chromium.org/673623002/diff/1/ui/gfx/path_win_unittest.cc#... > ui/gfx/path_win_unittest.cc:63: }; > Nit: De-indent this. ...
5 years, 5 months ago (2014-10-23 09:32:17 UTC) #6
Wez
https://codereview.chromium.org/673623002/diff/20001/ui/gfx/path_win_unittest.cc File ui/gfx/path_win_unittest.cc (right): https://codereview.chromium.org/673623002/diff/20001/ui/gfx/path_win_unittest.cc#newcode5 ui/gfx/path_win_unittest.cc:5: // Author: Aleksandr Derbenev <alex-ac@yandex-team.ru> You'll need to remove ...
5 years, 5 months ago (2014-10-23 18:50:06 UTC) #7
alex-ac
https://codereview.chromium.org/673623002/diff/20001/ui/gfx/path_win_unittest.cc File ui/gfx/path_win_unittest.cc (right): https://codereview.chromium.org/673623002/diff/20001/ui/gfx/path_win_unittest.cc#newcode5 ui/gfx/path_win_unittest.cc:5: // Author: Aleksandr Derbenev <alex-ac@yandex-team.ru> On 2014/10/23 18:50:05, Wez ...
5 years, 5 months ago (2014-10-24 07:05:11 UTC) #8
Wez
https://codereview.chromium.org/673623002/diff/20001/ui/gfx/path_win_unittest.cc File ui/gfx/path_win_unittest.cc (right): https://codereview.chromium.org/673623002/diff/20001/ui/gfx/path_win_unittest.cc#newcode28 ui/gfx/path_win_unittest.cc:28: std::transform(rects, rects + region_data->rdh.nCount, On 2014/10/24 07:05:10, alex-ac wrote: ...
5 years, 5 months ago (2014-10-24 23:07:21 UTC) #9
alex-ac
On 2014/10/24 23:07:21, Wez wrote: > There are only actually four calls to the API, ...
5 years, 5 months ago (2014-10-27 10:35:25 UTC) #11
Wez
LGTM w/ the nits below addressed. Thanks for fixing this bug! https://codereview.chromium.org/673623002/diff/80001/chrome/browser/ui/views/panels/panel_frame_view.cc File chrome/browser/ui/views/panels/panel_frame_view.cc (right): ...
5 years, 5 months ago (2014-10-27 20:28:39 UTC) #12
alex-ac
https://codereview.chromium.org/673623002/diff/80001/chrome/browser/ui/views/panels/panel_frame_view.cc File chrome/browser/ui/views/panels/panel_frame_view.cc (right): https://codereview.chromium.org/673623002/diff/80001/chrome/browser/ui/views/panels/panel_frame_view.cc#newcode361 chrome/browser/ui/views/panels/panel_frame_view.cc:361: NULL); On 2014/10/27 20:28:39, Wez wrote: > base::win::ScopedRegion new_region; ...
5 years, 5 months ago (2014-10-28 10:38:36 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/673623002/100001
5 years, 5 months ago (2014-10-28 10:40:25 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/20573)
5 years, 5 months ago (2014-10-28 10:49:33 UTC) #17
Alexei Svitkine (slow)
https://codereview.chromium.org/673623002/diff/100001/ui/gfx/path_win_unittest.cc File ui/gfx/path_win_unittest.cc (right): https://codereview.chromium.org/673623002/diff/100001/ui/gfx/path_win_unittest.cc#newcode10 ui/gfx/path_win_unittest.cc:10: #include "ui/gfx/path_win.h" Nit: This should be first include followed ...
5 years, 5 months ago (2014-10-28 13:21:03 UTC) #18
alex-ac
https://codereview.chromium.org/673623002/diff/100001/ui/gfx/path_win_unittest.cc File ui/gfx/path_win_unittest.cc (right): https://codereview.chromium.org/673623002/diff/100001/ui/gfx/path_win_unittest.cc#newcode10 ui/gfx/path_win_unittest.cc:10: #include "ui/gfx/path_win.h" On 2014/10/28 13:21:02, Alexei Svitkine wrote: > ...
5 years, 5 months ago (2014-10-28 13:32:58 UTC) #19
Alexei Svitkine (slow)
lgtm
5 years, 5 months ago (2014-10-28 14:22:50 UTC) #20
Wez
https://codereview.chromium.org/673623002/diff/140001/chrome/browser/ui/views/panels/panel_frame_view.cc File chrome/browser/ui/views/panels/panel_frame_view.cc (right): https://codereview.chromium.org/673623002/diff/140001/chrome/browser/ui/views/panels/panel_frame_view.cc#newcode363 chrome/browser/ui/views/panels/panel_frame_view.cc:363: new_region == NULL || We need to treat a ...
5 years, 5 months ago (2014-10-28 18:21:58 UTC) #21
alex-ac
https://codereview.chromium.org/673623002/diff/140001/chrome/browser/ui/views/panels/panel_frame_view.cc File chrome/browser/ui/views/panels/panel_frame_view.cc (right): https://codereview.chromium.org/673623002/diff/140001/chrome/browser/ui/views/panels/panel_frame_view.cc#newcode363 chrome/browser/ui/views/panels/panel_frame_view.cc:363: new_region == NULL || On 2014/10/28 18:21:57, Wez wrote: ...
5 years, 5 months ago (2014-10-29 14:19:48 UTC) #22
Wez
lgtm
5 years, 5 months ago (2014-10-29 22:15:48 UTC) #23
alex-ac
On 2014/11/06 11:14:32, alex-ac wrote: > mailto:alex-ac@yandex-team.ru changed reviewers: > + mailto:dimich@chromium.org > - mailto:sky@chromium.org ...
5 years, 5 months ago (2014-11-06 11:15:47 UTC) #25
alex-ac
On 2014/11/06 11:15:47, alex-ac wrote: > On 2014/11/06 11:14:32, alex-ac wrote: > > mailto:alex-ac@yandex-team.ru changed ...
5 years, 4 months ago (2014-11-12 13:07:37 UTC) #27
sky
https://codereview.chromium.org/673623002/diff/160001/ui/views/win/hwnd_message_handler.cc File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/673623002/diff/160001/ui/views/win/hwnd_message_handler.cc#newcode1158 ui/views/win/hwnd_message_handler.cc:1158: if (!!current_rgn != !!new_region || Wow, could you make ...
5 years, 4 months ago (2014-11-12 15:28:36 UTC) #28
alex-ac
https://codereview.chromium.org/673623002/diff/160001/ui/views/win/hwnd_message_handler.cc File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/673623002/diff/160001/ui/views/win/hwnd_message_handler.cc#newcode1158 ui/views/win/hwnd_message_handler.cc:1158: if (!!current_rgn != !!new_region || On 2014/11/12 15:28:36, sky ...
5 years, 4 months ago (2014-11-13 08:24:40 UTC) #29
sky
LGTM
5 years, 4 months ago (2014-11-13 15:35:21 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/673623002/180001
5 years, 4 months ago (2014-11-13 15:37:15 UTC) #32
commit-bot: I haz the power
Try jobs failed on following builders: win_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/86017)
5 years, 4 months ago (2014-11-13 16:24:15 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/673623002/200001
5 years, 4 months ago (2014-11-13 16:42:02 UTC) #36
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/673623002/220001
5 years, 4 months ago (2014-11-13 17:14:26 UTC) #39
commit-bot: I haz the power
Committed patchset #12 (id:220001)
5 years, 4 months ago (2014-11-13 18:03:36 UTC) #40
commit-bot: I haz the power
5 years, 4 months ago (2014-11-13 18:04:16 UTC) #41
Message was sent while issue was closed.
Patchset 12 (id:??) landed as
https://crrev.com/2cb37ad893097cd5c19184c7f2b937df59b24996
Cr-Commit-Position: refs/heads/master@{#304045}

Powered by Google App Engine
This is Rietveld 408576698