|
|
Chromium Code Reviews
DescriptionDevTools shows overlay scrollbar for enabled aura overlay scrollbar and mac
DevTools mobile emulator currently does not show android overlay
scrollbar when we enabled aura overlay scrollbar or we use it on Mac.
This issue is caused by we have not set any overlay scrollbar setting
on Mac and have not set solid_color_scrollbar_color when enabled aura
overlay scrollbar.
In this patch, we set default overlay scrollbar same as Android Overlay
Scrollbar for Android and DevTools mobile emulator, Aura Overlay
Scrollbar will override the default settings.
BUG=673258
Review-Url: https://codereview.chromium.org/2870733002
Cr-Commit-Position: refs/heads/master@{#470570}
Committed: https://chromium.googlesource.com/chromium/src/+/b50f6446eb79c4aaaac0a5eefdf90d2b4a99ce54
Patch Set 1 #Patch Set 2 : fix for mac #Patch Set 3 : rebase #Patch Set 4 : add comment #
Total comments: 6
Patch Set 5 : aelias comments addressed #Messages
Total messages: 30 (21 generated)
The CQ bit was checked by chaopeng@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...
Patchset #2 (id:20001) has been deleted
The CQ bit was checked by chaopeng@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: 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_...) win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...) win_clang on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
The CQ bit was checked by chaopeng@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.
Description was changed from ========== [Not For Review] Dev Tools Overlay Scrollbar BUG=667455 ========== to ========== DevTools shows overlay scrollbar for enabled aura overlay scrollbar and mac DevTools mobile emulator currently does not show android overlay scrollbar when we enabled aura overlay scrollbar or we use it on Mac. This issue is caused by we have not set any overlay scrollbar setting on Mac and have not set solid_color_scrollbar_color when enabled aura overlay scrollbar. In this patch, we set default overlay scrollbar as android overlay scrollbar for DevTools mobile emulator, then Android and Aura Overlay Scrollbar will override the default settings. BUG=667455 ==========
chaopeng@chromium.org changed reviewers: + aelias@chromium.org
PTAL. Thank you.
Description was changed from ========== DevTools shows overlay scrollbar for enabled aura overlay scrollbar and mac DevTools mobile emulator currently does not show android overlay scrollbar when we enabled aura overlay scrollbar or we use it on Mac. This issue is caused by we have not set any overlay scrollbar setting on Mac and have not set solid_color_scrollbar_color when enabled aura overlay scrollbar. In this patch, we set default overlay scrollbar as android overlay scrollbar for DevTools mobile emulator, then Android and Aura Overlay Scrollbar will override the default settings. BUG=667455 ========== to ========== DevTools shows overlay scrollbar for enabled aura overlay scrollbar and mac DevTools mobile emulator currently does not show android overlay scrollbar when we enabled aura overlay scrollbar or we use it on Mac. This issue is caused by we have not set any overlay scrollbar setting on Mac and have not set solid_color_scrollbar_color when enabled aura overlay scrollbar. In this patch, we set default overlay scrollbar as android overlay scrollbar for DevTools mobile emulator, then Android and Aura Overlay Scrollbar will override the default settings. BUG=673258 ==========
https://codereview.chromium.org/2870733002/diff/80001/content/renderer/gpu/re... File content/renderer/gpu/render_widget_compositor.cc (right): https://codereview.chromium.org/2870733002/diff/80001/content/renderer/gpu/re... content/renderer/gpu/render_widget_compositor.cc:437: settings.scrollbar_fade_delay = base::TimeDelta::FromMilliseconds(500); Please set this to 300 for consistency with Android. https://codereview.chromium.org/2870733002/diff/80001/content/renderer/gpu/re... content/renderer/gpu/render_widget_compositor.cc:453: } else { Please delete the whole content of this else block since it's now no-op, and update your comment to mention it applies to both Android and devtools. https://codereview.chromium.org/2870733002/diff/80001/content/renderer/gpu/re... content/renderer/gpu/render_widget_compositor.cc:496: #if !defined(OS_MACOSX) Please delete this #if !defined(OS_MACOSX). Looks like ui::IsOverlayScrollbarEnabled() always returns false on Mac anyway.
Description was changed from ========== DevTools shows overlay scrollbar for enabled aura overlay scrollbar and mac DevTools mobile emulator currently does not show android overlay scrollbar when we enabled aura overlay scrollbar or we use it on Mac. This issue is caused by we have not set any overlay scrollbar setting on Mac and have not set solid_color_scrollbar_color when enabled aura overlay scrollbar. In this patch, we set default overlay scrollbar as android overlay scrollbar for DevTools mobile emulator, then Android and Aura Overlay Scrollbar will override the default settings. BUG=673258 ========== to ========== DevTools shows overlay scrollbar for enabled aura overlay scrollbar and mac DevTools mobile emulator currently does not show android overlay scrollbar when we enabled aura overlay scrollbar or we use it on Mac. This issue is caused by we have not set any overlay scrollbar setting on Mac and have not set solid_color_scrollbar_color when enabled aura overlay scrollbar. In this patch, we set default overlay scrollbar same as Android Overlay Scrollbar for Android and DevTools mobile emulator, Aura Overlay Scrollbar will override the default settings. BUG=673258 ==========
chaopeng@chromium.org changed reviewers: + bokan@chromium.org
PTAL. Thank you. https://codereview.chromium.org/2870733002/diff/80001/content/renderer/gpu/re... File content/renderer/gpu/render_widget_compositor.cc (right): https://codereview.chromium.org/2870733002/diff/80001/content/renderer/gpu/re... content/renderer/gpu/render_widget_compositor.cc:437: settings.scrollbar_fade_delay = base::TimeDelta::FromMilliseconds(500); On 2017/05/09 19:20:23, aelias wrote: > Please set this to 300 for consistency with Android. Done. https://codereview.chromium.org/2870733002/diff/80001/content/renderer/gpu/re... content/renderer/gpu/render_widget_compositor.cc:453: } else { On 2017/05/09 19:20:23, aelias wrote: > Please delete the whole content of this else block since it's now no-op, and > update your comment to mention it applies to both Android and devtools. Done. https://codereview.chromium.org/2870733002/diff/80001/content/renderer/gpu/re... content/renderer/gpu/render_widget_compositor.cc:496: #if !defined(OS_MACOSX) On 2017/05/09 19:20:23, aelias wrote: > Please delete this #if !defined(OS_MACOSX). Looks like > ui::IsOverlayScrollbarEnabled() always returns false on Mac anyway. Done.
lgtm
lgtm
The CQ bit was checked by chaopeng@chromium.org
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
Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?))
The CQ bit was checked by chaopeng@chromium.org
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": 100001, "attempt_start_ts": 1494423787085600,
"parent_rev": "6771b159ea1b5c67fb573d6b88c90b7b29550f52", "commit_rev":
"b50f6446eb79c4aaaac0a5eefdf90d2b4a99ce54"}
Message was sent while issue was closed.
Description was changed from ========== DevTools shows overlay scrollbar for enabled aura overlay scrollbar and mac DevTools mobile emulator currently does not show android overlay scrollbar when we enabled aura overlay scrollbar or we use it on Mac. This issue is caused by we have not set any overlay scrollbar setting on Mac and have not set solid_color_scrollbar_color when enabled aura overlay scrollbar. In this patch, we set default overlay scrollbar same as Android Overlay Scrollbar for Android and DevTools mobile emulator, Aura Overlay Scrollbar will override the default settings. BUG=673258 ========== to ========== DevTools shows overlay scrollbar for enabled aura overlay scrollbar and mac DevTools mobile emulator currently does not show android overlay scrollbar when we enabled aura overlay scrollbar or we use it on Mac. This issue is caused by we have not set any overlay scrollbar setting on Mac and have not set solid_color_scrollbar_color when enabled aura overlay scrollbar. In this patch, we set default overlay scrollbar same as Android Overlay Scrollbar for Android and DevTools mobile emulator, Aura Overlay Scrollbar will override the default settings. BUG=673258 Review-Url: https://codereview.chromium.org/2870733002 Cr-Commit-Position: refs/heads/master@{#470570} Committed: https://chromium.googlesource.com/chromium/src/+/b50f6446eb79c4aaaac0a5eefdf9... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:100001) as https://chromium.googlesource.com/chromium/src/+/b50f6446eb79c4aaaac0a5eefdf9... |
