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

Issue 1143323009: Report new screen position to renderer when ancestor window moves. (Closed)

Created:
5 years, 7 months ago by mharanczyk
Modified:
5 years, 6 months ago
Reviewers:
sadrul, sky
CC:
chromium-reviews, yusukes+watch_chromium.org, shuchen+watch_chromium.org, jam, penghuang+watch_chromium.org, sievers+watch_chromium.org, jbauman+watch_chromium.org, nona+watch_chromium.org, darin-cc_chromium.org, kalyank, piman+watch_chromium.org, danakj+watch_chromium.org, James Su
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Report new screen position to renderer when ancestor window moves. This change is related to https://codereview.chromium.org/1062273002/. Without it renderer can draw controls (dropdowns, etc.) out of place, because it will have outdated screen coordinates. BUG=469857, 468669 Committed: https://crrev.com/647e2a749681a5f9e5bba3510524d898f9b4b910 Cr-Commit-Position: refs/heads/master@{#333749}

Patch Set 1 #

Patch Set 2 : Added unittest. #

Total comments: 9

Patch Set 3 : Address review nits. #

Patch Set 4 : Rebase #

Patch Set 5 : Do not try to send screen rect during shutdown. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -0 lines) Patch
M content/browser/renderer_host/render_widget_host_view_aura.cc View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_aura_unittest.cc View 1 2 3 1 chunk +61 lines, -0 lines 0 comments Download

Messages

Total messages: 32 (11 generated)
mharanczyk
@sky, please take a look, this CL is a follow up of our mail discussion.
5 years, 7 months ago (2015-05-27 13:05:59 UTC) #2
sadrul
Can this have a test?
5 years, 7 months ago (2015-05-27 14:18:17 UTC) #3
mharanczyk
I haven't find a nice way to test it, I will keep looking. If you ...
5 years, 7 months ago (2015-05-27 15:17:16 UTC) #4
sadrul
On 2015/05/27 15:17:16, mharanczyk wrote: > I haven't find a nice way to test it, ...
5 years, 7 months ago (2015-05-27 15:33:37 UTC) #5
mharanczyk
On 2015/05/27 15:33:37, sadrul wrote: > The test should be in render_widget_host_view_aura_unittest.cc: the test could ...
5 years, 7 months ago (2015-05-27 15:38:10 UTC) #6
mharanczyk
Added unit test, could you please take another look.
5 years, 6 months ago (2015-05-28 16:12:07 UTC) #7
mharanczyk
Ping! Can anyone please take a look?
5 years, 6 months ago (2015-06-03 09:36:49 UTC) #8
sadrul
A couple of nits. Otherwise, lgtm https://codereview.chromium.org/1143323009/diff/20001/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc (right): https://codereview.chromium.org/1143323009/diff/20001/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc#newcode841 content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:841: EXPECT_EQ(1U, sink_->message_count()); Do ...
5 years, 6 months ago (2015-06-03 18:51:41 UTC) #9
mharanczyk
https://codereview.chromium.org/1143323009/diff/20001/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc (right): https://codereview.chromium.org/1143323009/diff/20001/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc#newcode841 content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:841: EXPECT_EQ(1U, sink_->message_count()); On 2015/06/03 18:51:41, sadrul wrote: > Do ...
5 years, 6 months ago (2015-06-08 11:56:31 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1143323009/40001
5 years, 6 months ago (2015-06-08 11:58:11 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_gn_dbg on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_gn_dbg/builds/79597)
5 years, 6 months ago (2015-06-08 12:13:23 UTC) #15
mharanczyk
Rebase moved tuple get<> to base namespace. Fixed that.
5 years, 6 months ago (2015-06-08 13:20:18 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1143323009/60001
5 years, 6 months ago (2015-06-08 13:20:35 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/25629)
5 years, 6 months ago (2015-06-08 13:55:01 UTC) #21
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1143323009/80001
5 years, 6 months ago (2015-06-08 14:58:55 UTC) #24
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 6 months ago (2015-06-08 15:42:49 UTC) #26
mharanczyk
I had to add conditional to not send rects when shutting down to avoid crashes ...
5 years, 6 months ago (2015-06-08 15:45:29 UTC) #27
sadrul
On 2015/06/08 15:45:29, mharanczyk wrote: > I had to add conditional to not send rects ...
5 years, 6 months ago (2015-06-10 16:12:07 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1143323009/80001
5 years, 6 months ago (2015-06-10 16:13:42 UTC) #30
commit-bot: I haz the power
Committed patchset #5 (id:80001)
5 years, 6 months ago (2015-06-10 17:04:51 UTC) #31
commit-bot: I haz the power
5 years, 6 months ago (2015-06-10 17:05:46 UTC) #32
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/647e2a749681a5f9e5bba3510524d898f9b4b910
Cr-Commit-Position: refs/heads/master@{#333749}

Powered by Google App Engine
This is Rietveld 408576698