|
|
Chromium Code Reviews
DescriptionDon't do DefWindowProc on WM_NCACTIVATE if there's no frame.
WM_NCACTIVATE might draw the border around the edge of the window
which causes artifacts, but if there's no system frame then it shouldn't
need to.
BUG=725421
Review-Url: https://codereview.chromium.org/2899053007
Cr-Commit-Position: refs/heads/master@{#474837}
Committed: https://chromium.googlesource.com/chromium/src/+/a283fb63e50aacdb88401e8d1f4b05f15256b705
Patch Set 1 #
Total comments: 1
Patch Set 2 : remove parens #Patch Set 3 : add comment #Messages
Total messages: 28 (18 generated)
The CQ bit was checked by jbauman@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
jbauman@chromium.org changed reviewers: + bsep@chromium.org
lgtm I just noticed that the Hung Renderer Dialog (chrome://hang) also has artifacts, does this fix that as well? https://codereview.chromium.org/2899053007/diff/1/ui/views/win/hwnd_message_h... File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/2899053007/diff/1/ui/views/win/hwnd_message_h... ui/views/win/hwnd_message_handler.cc:1823: (delegate_->GetFrameMode() == FrameMode::CUSTOM_DRAWN)) { nit: you don't need parens around this clause
The CQ bit was checked by jbauman@chromium.org to run a CQ dry run
On 2017/05/24 22:22:05, Bret Sepulveda wrote: > lgtm > > I just noticed that the Hung Renderer Dialog (chrome://hang) also has artifacts, > does this fix that as well? Yeah, seems to fix that. > > https://codereview.chromium.org/2899053007/diff/1/ui/views/win/hwnd_message_h... > File ui/views/win/hwnd_message_handler.cc (right): > > https://codereview.chromium.org/2899053007/diff/1/ui/views/win/hwnd_message_h... > ui/views/win/hwnd_message_handler.cc:1823: (delegate_->GetFrameMode() == > FrameMode::CUSTOM_DRAWN)) { > nit: you don't need parens around this clause Done.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
jbauman@chromium.org changed reviewers: + sky@chromium.org
sky@: ui/views OWNERS review.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
This code is very subtle. Did you look back at the history to make sure you aren't going to introduce other painting artifacts?
At a minimum, please add a comment.
The CQ bit was checked by jbauman@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/05/25 02:40:39, sky wrote: > This code is very subtle. Did you look back at the history to make sure you > aren't going to introduce other painting artifacts? It's hard to prove that it won't cause painting artifacts, but there doesn't seem to be any evidence from the history that it would. The original code to not call DefWindowProc was added in https://codereview.chromium.org/8133009 , which was before remove_standard_frame_ (now !HasFrame()) was added, so I think not doing it in that case is just an oversight. Not calling DefWindowProc was causing some issues on XP and Vista, but that doesn't matter anymore. I've also added a comment.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM
The CQ bit was checked by jbauman@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bsep@chromium.org Link to the patchset: https://codereview.chromium.org/2899053007/#ps40001 (title: "add comment")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495753090957040,
"parent_rev": "bbb23b76fd2d8e7f864deff6e70c10bf4fddeb87", "commit_rev":
"a283fb63e50aacdb88401e8d1f4b05f15256b705"}
Message was sent while issue was closed.
Description was changed from ========== Don't do DefWindowProc on WM_NCACTIVATE if there's no frame. WM_NCACTIVATE might draw the border around the edge of the window which causes artifacts, but if there's no system frame then it shouldn't need to. BUG=725421 ========== to ========== Don't do DefWindowProc on WM_NCACTIVATE if there's no frame. WM_NCACTIVATE might draw the border around the edge of the window which causes artifacts, but if there's no system frame then it shouldn't need to. BUG=725421 Review-Url: https://codereview.chromium.org/2899053007 Cr-Commit-Position: refs/heads/master@{#474837} Committed: https://chromium.googlesource.com/chromium/src/+/a283fb63e50aacdb88401e8d1f4b... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/a283fb63e50aacdb88401e8d1f4b... |
