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

Issue 2268563003: Replaced CursorLoader in PlatformDisplay with a ImageCursor (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 months ago by Alex Z.
Modified:
7 months, 1 week ago
Reviewers:
sadrul, sky
CC:
chromium-reviews, rjkroege
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Replaced CursorLoader in PlatformDisplay with a ImageCursor ImageCursor->SetDisplay() is called in Init() so that all cursors are loaded at launch. This fixes the bug where the cursor image is positioned off to the left in Chrome mash. BUG=615861 Committed: https://crrev.com/41725e3dd88eadb07f8a1297c0f8f6ecb9b30c79 Cr-Commit-Position: refs/heads/master@{#439154}

Patch Set 1 #

Total comments: 6

Patch Set 2 : Add display::Display PlatformDisplayDelegate::GetDisplay() #

Total comments: 4

Patch Set 3 : Merged namespace blocks and replaced static_cast with GetDisplay() #

Patch Set 4 : Moved ImageCursors::SetDisplay() to the end of DefaultPlatformDisplay::Init() #

Patch Set 5 : rebase #

Patch Set 6 : Rebase #

Messages

Total messages: 60 (32 generated)
Alex Z.
Please review my changes.
11 months ago (2016-08-22 13:45:16 UTC) #2
sadrul
https://codereview.chromium.org/2268563003/diff/1/services/ui/ws/platform_display.cc File services/ui/ws/platform_display.cc (right): https://codereview.chromium.org/2268563003/diff/1/services/ui/ws/platform_display.cc#newcode85 services/ui/ws/platform_display.cc:85: GetDeviceScaleFactor()); Instead of static casting, introduce display::Display PlatformDisplayDelegate::GetDisplay() https://codereview.chromium.org/2268563003/diff/1/services/ui/ws/platform_display.cc#newcode143 ...
11 months ago (2016-08-22 14:39:40 UTC) #4
Alex Z.
https://codereview.chromium.org/2268563003/diff/1/services/ui/ws/platform_display.cc File services/ui/ws/platform_display.cc (right): https://codereview.chromium.org/2268563003/diff/1/services/ui/ws/platform_display.cc#newcode85 services/ui/ws/platform_display.cc:85: GetDeviceScaleFactor()); On 2016/08/22 14:39:40, sadrul wrote: > Instead of ...
11 months ago (2016-08-22 15:11:51 UTC) #5
sadrul
https://codereview.chromium.org/2268563003/diff/20001/services/ui/ws/platform_display.cc File services/ui/ws/platform_display.cc (right): https://codereview.chromium.org/2268563003/diff/20001/services/ui/ws/platform_display.cc#newcode84 services/ui/ws/platform_display.cc:84: static_cast<ui::ws::Display*>(delegate_)->ToDisplay(), Use the newly added delegate function. https://codereview.chromium.org/2268563003/diff/20001/services/ui/ws/platform_display.h File ...
11 months ago (2016-08-22 15:18:14 UTC) #6
Alex Z.
https://codereview.chromium.org/2268563003/diff/20001/services/ui/ws/platform_display.cc File services/ui/ws/platform_display.cc (right): https://codereview.chromium.org/2268563003/diff/20001/services/ui/ws/platform_display.cc#newcode84 services/ui/ws/platform_display.cc:84: static_cast<ui::ws::Display*>(delegate_)->ToDisplay(), On 2016/08/22 15:18:14, sadrul wrote: > Use the ...
11 months ago (2016-08-22 15:26:02 UTC) #7
sadrul
lgtm
11 months ago (2016-08-22 15:35:21 UTC) #8
sky
LGTM
11 months ago (2016-08-22 15:50:22 UTC) #9
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/2268563003/40001
11 months ago (2016-08-22 15:57:23 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/265148)
11 months ago (2016-08-22 18:38:08 UTC) #13
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/2268563003/40001
11 months ago (2016-08-22 20:14:49 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/284380)
11 months ago (2016-08-22 23:21:49 UTC) #17
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/2268563003/40001
11 months ago (2016-08-23 13:34:12 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/284804)
11 months ago (2016-08-23 16:14:49 UTC) #21
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/2268563003/60001
11 months ago (2016-08-23 22:22:36 UTC) #24
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/285257)
11 months ago (2016-08-24 01:20:56 UTC) #26
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/2268563003/60001
11 months ago (2016-08-24 12:44:09 UTC) #28
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/285645)
11 months ago (2016-08-24 15:24:18 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/2268563003/60001
11 months ago (2016-08-26 01:47:01 UTC) #32
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/287028)
11 months ago (2016-08-26 03:11:01 UTC) #34
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/2268563003/60001
10 months, 4 weeks ago (2016-08-26 05:11:12 UTC) #36
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/287087)
10 months, 4 weeks ago (2016-08-26 07:36:18 UTC) #38
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/2268563003/60001
10 months, 3 weeks ago (2016-08-29 17:15:50 UTC) #40
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/269319) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, ...
10 months, 3 weeks ago (2016-08-30 03:30:03 UTC) #42
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/2268563003/100001
7 months, 1 week ago (2016-12-16 18:16:23 UTC) #53
commit-bot: I haz the power
Committed patchset #6 (id:100001)
7 months, 1 week ago (2016-12-16 18:27:00 UTC) #56
commit-bot: I haz the power
Patchset 6 (id:??) landed as https://crrev.com/41725e3dd88eadb07f8a1297c0f8f6ecb9b30c79 Cr-Commit-Position: refs/heads/master@{#439154}
7 months, 1 week ago (2016-12-16 18:29:14 UTC) #58
sadrul
On 2016/12/16 18:29:14, commit-bot: I haz the power wrote: > Patchset 6 (id:??) landed as ...
7 months, 1 week ago (2016-12-16 18:31:12 UTC) #59
Alex Z.
7 months, 1 week ago (2016-12-16 18:33:10 UTC) #60
Message was sent while issue was closed.
On 2016/12/16 18:31:12, sadrul wrote:
> On 2016/12/16 18:29:14, commit-bot: I haz the power wrote:
> > Patchset 6 (id:??) landed as
> > https://crrev.com/41725e3dd88eadb07f8a1297c0f8f6ecb9b30c79
> > Cr-Commit-Position: refs/heads/master@{#439154}
> 
> Oh awesome! Thanks for getting back to this! Do you know what/when the problem
> with skia was resolved?

I am not 100% sure but I think it was when this was fixed:
https://bugs.chromium.org/p/chromium/issues/detail?id=642879
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 25c286973