Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1560403002:
Scale scrollbar in use-zoom-for-dsf mode (Closed)
|
Created:
4 years, 11 months ago by oshima Modified:
4 years, 11 months ago CC:
chromium-reviews, blink-reviews, cc-bugs_chromium.org, dshwang, slimming-paint-reviews_chromium.org, blink-reviews-paint_chromium.org, danakj Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionScale scrollbar in use-zoom-for-dsf mode
* Scale the size of scrollbar
* Compute the trackRect in ScrollbarThemeNonMacCommon using width/height of the reference scrollbar to compute thickness, instead of using scrollbarThickness
BUG=485650
TEST=manual. Layout test will be covered in a separate CL that adds virtual test suite for this mode.
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
Committed: https://crrev.com/0a1920a7d4dd833fa8cffb4b7acd38b040236326
Cr-Commit-Position: refs/heads/master@{#369477}
Patch Set 1 : #Patch Set 2 : #
Total comments: 12
Patch Set 3 : #Patch Set 4 : #
Total comments: 4
Patch Set 5 : #Patch Set 6 : #Patch Set 7 : #
Total comments: 4
Patch Set 8 : #
Total comments: 1
Patch Set 9 : remove unintentional change #Messages
Total messages: 86 (41 generated)
Description was changed from ========== Scrollbar BUG= ========== to ========== Scrollbar BUG= CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
The CQ bit was checked by oshima@chromium.org to run a CQ dry run
Patchset #4 (id:60001) has been deleted
Patchset #3 (id:40001) has been deleted
Patchset #2 (id:20001) has been deleted
Patchset #1 (id:1) has been deleted
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1560403002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1560403002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by oshima@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1560403002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1560403002/100001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by oshima@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1560403002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1560403002/120001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by oshima@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1560403002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1560403002/140001
Patchset #1 (id:80001) has been deleted
Patchset #1 (id:100001) has been deleted
Patchset #1 (id:120001) has been deleted
The CQ bit was checked by oshima@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1560403002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1560403002/160001
Description was changed from ========== Scrollbar BUG= CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== Scale scrollbar in use-zoom-for-dsf mode BUG=485650 TEST=manual. Layout test will be covered in a separate CL that adds virtual test suite for this mode. ==========
oshima@chromium.org changed reviewers: + skobes@chromium.org
Description was changed from ========== Scale scrollbar in use-zoom-for-dsf mode BUG=485650 TEST=manual. Layout test will be covered in a separate CL that adds virtual test suite for this mode. ========== to ========== Scale scrollbar in use-zoom-for-dsf mode * Scale the size of scrollbar * Compute the trackRect in ScrollbarThemeNonMacCommon using width/height of the reference scrollbar to compute thickness, instead of using scrollbarThickness * Generate the image for scrollbar at the right scale. BUG=485650 TEST=manual. Layout test will be covered in a separate CL that adds virtual test suite for this mode. ==========
Description was changed from ========== Scale scrollbar in use-zoom-for-dsf mode * Scale the size of scrollbar * Compute the trackRect in ScrollbarThemeNonMacCommon using width/height of the reference scrollbar to compute thickness, instead of using scrollbarThickness * Generate the image for scrollbar at the right scale. BUG=485650 TEST=manual. Layout test will be covered in a separate CL that adds virtual test suite for this mode. ========== to ========== Scale scrollbar in use-zoom-for-dsf mode * Scale the size of scrollbar * Compute the trackRect in ScrollbarThemeNonMacCommon using width/height of the reference scrollbar to compute thickness, instead of using scrollbarThickness * Generate the image for scrollbar at the right scale. BUG=485650 TEST=manual. Layout test will be covered in a separate CL that adds virtual test suite for this mode. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
Patchset #1 (id:140001) has been deleted
The CQ bit was checked by oshima@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1560403002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1560403002/180001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1560403002/diff/180001/cc/layers/painted_scro... File cc/layers/painted_scrollbar_layer.cc (right): https://codereview.chromium.org/1560403002/diff/180001/cc/layers/painted_scro... cc/layers/painted_scrollbar_layer.cc:286: gfx::Rect adjusted_layer_rect( "scaled_layer_rect" is clearer https://codereview.chromium.org/1560403002/diff/180001/cc/layers/painted_scro... cc/layers/painted_scrollbar_layer.cc:287: layer_rect.origin(), Don't we need to scale the origin as well? I assume origin and size are in the same coordinate system... https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/scroll/Scrollbar.cpp (right): https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/scroll/Scrollbar.cpp:83: IntRect tmp(0, 0, 100, 0); Where does this "100" come from? https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/scroll/ScrollbarThemeNonMacCommon.cpp (left): https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/scroll/ScrollbarThemeNonMacCommon.cpp:76: int thickness = scrollbarThickness(scrollbar.controlSize()); The other caller of ScrollbarThemeClient::controlSize is ScrollbarTheme::minimumThumbLength. Does that need updating as well? https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/scroll/ScrollbarThemeNonMacCommon.cpp (right): https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/scroll/ScrollbarThemeNonMacCommon.cpp:81: } else { Remove the "else" since we return in the if block.
https://codereview.chromium.org/1560403002/diff/180001/cc/layers/painted_scro... File cc/layers/painted_scrollbar_layer.cc (right): https://codereview.chromium.org/1560403002/diff/180001/cc/layers/painted_scro... cc/layers/painted_scrollbar_layer.cc:286: gfx::Rect adjusted_layer_rect( On 2016/01/07 21:49:10, skobes wrote: > "scaled_layer_rect" is clearer I used adjusted because its origin isn't scaled. I changed it to scaled anyway, but let me know if you're convinced. https://codereview.chromium.org/1560403002/diff/180001/cc/layers/painted_scro... cc/layers/painted_scrollbar_layer.cc:287: layer_rect.origin(), On 2016/01/07 21:49:10, skobes wrote: > Don't we need to scale the origin as well? I assume origin and size are in the > same coordinate system... This is correct. We want to scale the size only and the drawing code adjust the canvas's coordinate so that the image will be drawn at 0.0 in the SkBitmap. I updated a bit to make it clearer. https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/scroll/Scrollbar.cpp (right): https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/scroll/Scrollbar.cpp:83: IntRect tmp(0, 0, 100, 0); On 2016/01/07 21:49:10, skobes wrote: > Where does this "100" come from? It's picked as large enough number so that you can compute the viewport to window ratio. I didn't add new method because this serves enough and got lgtm from owner in the past. https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... I can add new method if you want, but want to do that in separate CL as it involves other unrelated changes, and need owners approval. https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/scroll/ScrollbarThemeNonMacCommon.cpp (left): https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/scroll/ScrollbarThemeNonMacCommon.cpp:76: int thickness = scrollbarThickness(scrollbar.controlSize()); On 2016/01/07 21:49:10, skobes wrote: > The other caller of ScrollbarThemeClient::controlSize is > ScrollbarTheme::minimumThumbLength. Does that need updating as well? Other uses seems to be for custom scrollbars, which gets scaled properly when zoomed. https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/scroll/ScrollbarThemeNonMacCommon.cpp (right): https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/scroll/ScrollbarThemeNonMacCommon.cpp:81: } else { On 2016/01/07 21:49:10, skobes wrote: > Remove the "else" since we return in the if block. Done.
The CQ bit was checked by oshima@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1560403002/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1560403002/200001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/scroll/Scrollbar.cpp (right): https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/scroll/Scrollbar.cpp:83: IntRect tmp(0, 0, 100, 0); On 2016/01/09 00:58:39, oshima wrote: > On 2016/01/07 21:49:10, skobes wrote: > > Where does this "100" come from? > > It's picked as large enough number so that you can compute the viewport to > window ratio. > I didn't add new method because this serves enough and got lgtm from owner in > the past. > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... > > > I can add new method if you want, but want to do that in separate CL as it > involves other unrelated changes, and need owners approval. It would be much clearer to have an overload of viewportToScreen that takes a WebSize, and pass the thickness to it directly. Separate CL is fine but at least add a comment here explaining what it's doing. Related: are the horizontal and vertical ratios always the same? If not you will need to use height or width here depending on scrollbar orientation.
https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/scroll/Scrollbar.cpp (right): https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/scroll/Scrollbar.cpp:83: IntRect tmp(0, 0, 100, 0); On 2016/01/11 03:28:29, skobes wrote: > It would be much clearer to have an overload of viewportToScreen that takes a > WebSize Clarification: I meant something like HostWindow::viewportToScreen(IntSize&) delegating to WebWidgetClient::convertViewportToWindow(WebSize&).
On 2016/01/11 03:28:30, skobes wrote: > https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... > File third_party/WebKit/Source/platform/scroll/Scrollbar.cpp (right): > > https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... > third_party/WebKit/Source/platform/scroll/Scrollbar.cpp:83: IntRect tmp(0, 0, > 100, 0); > On 2016/01/09 00:58:39, oshima wrote: > > On 2016/01/07 21:49:10, skobes wrote: > > > Where does this "100" come from? > > > > It's picked as large enough number so that you can compute the viewport to > > window ratio. > > I didn't add new method because this serves enough and got lgtm from owner in > > the past. > > > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... > > > > > > I can add new method if you want, but want to do that in separate CL as it > > involves other unrelated changes, and need owners approval. > > It would be much clearer to have an overload of viewportToScreen that takes a > WebSize, and pass the thickness to it directly. Separate CL is fine but at > least add a comment here explaining what it's doing. > > Related: are the horizontal and vertical ratios always the same? If not you > will need to use height or width here depending on scrollbar orientation.
Hmm, my message seems to be removed for some reason. Here is another try. On 2016/01/11 03:28:30, skobes wrote: > https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... > File third_party/WebKit/Source/platform/scroll/Scrollbar.cpp (right): > > https://codereview.chromium.org/1560403002/diff/180001/third_party/WebKit/Sou... > third_party/WebKit/Source/platform/scroll/Scrollbar.cpp:83: IntRect tmp(0, 0, > 100, 0); > On 2016/01/09 00:58:39, oshima wrote: > > On 2016/01/07 21:49:10, skobes wrote: > > > Where does this "100" come from? > > > > It's picked as large enough number so that you can compute the viewport to > > window ratio. > > I didn't add new method because this serves enough and got lgtm from owner in > > the past. > > > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... > > > > > > I can add new method if you want, but want to do that in separate CL as it > > involves other unrelated changes, and need owners approval. > > It would be much clearer to have an overload of viewportToScreen that takes a > WebSize, and pass the thickness to it directly. Separate CL is fine but at > least add a comment here explaining what it's doing. My plan was to add screenToViewport(float) so that it can return the scaled thickness directly. I added comment. > > Related: are the horizontal and vertical ratios always the same? If not you > will need to use height or width here depending on scrollbar orientation. Yes, there is only one scale, deviceScaleFactor, by which the width/height are scaled.
ping?
On 2016/01/12 18:16:18, oshima wrote: > ping? Can you add the comment in Scrollbar.cpp explaining the use of viewportToScreen?
On 2016/01/12 19:06:28, skobes wrote: > On 2016/01/12 18:16:18, oshima wrote: > > ping? > > Can you add the comment in Scrollbar.cpp explaining the use of viewportToScreen? Done
lgtm
oshima@chromium.org changed reviewers: + danakj@chromium.org, eae@chromium.org
eae@ -> WebKit/Source/platform owner danakj@ -> cc/ owner
https://codereview.chromium.org/1560403002/diff/220001/cc/layers/painted_scro... File cc/layers/painted_scrollbar_layer.cc (right): https://codereview.chromium.org/1560403002/diff/220001/cc/layers/painted_scro... cc/layers/painted_scrollbar_layer.cc:282: // If zoom is used to scale the content for device scale factor, "zoom" isn't a concept here. Instead "If device scale is applied during painting instead of by the compositor..." https://codereview.chromium.org/1560403002/diff/220001/cc/layers/painted_scro... cc/layers/painted_scrollbar_layer.cc:287: gfx::ScaleToFlooredSize(layer_rect.size(), scale, scale); But why isn't blink just making the scrollbar layer the right size?
https://codereview.chromium.org/1560403002/diff/220001/cc/layers/painted_scro... File cc/layers/painted_scrollbar_layer.cc (right): https://codereview.chromium.org/1560403002/diff/220001/cc/layers/painted_scro... cc/layers/painted_scrollbar_layer.cc:282: // If zoom is used to scale the content for device scale factor, On 2016/01/12 20:07:03, danakj wrote: > "zoom" isn't a concept here. Instead "If device scale is applied during painting > instead of by the compositor..." Done. https://codereview.chromium.org/1560403002/diff/220001/cc/layers/painted_scro... cc/layers/painted_scrollbar_layer.cc:287: gfx::ScaleToFlooredSize(layer_rect.size(), scale, scale); On 2016/01/12 20:07:03, danakj wrote: > But why isn't blink just making the scrollbar layer the right size? It does (in viewport size) and that's why I have to convert it back to DIP so that painter draw scaled image, not bigger one. That's being said, I found the issue in current approach where it can't handle the scale factor change properly. Please hold, and will send request when it is resolved.
Description was changed from ========== Scale scrollbar in use-zoom-for-dsf mode * Scale the size of scrollbar * Compute the trackRect in ScrollbarThemeNonMacCommon using width/height of the reference scrollbar to compute thickness, instead of using scrollbarThickness * Generate the image for scrollbar at the right scale. BUG=485650 TEST=manual. Layout test will be covered in a separate CL that adds virtual test suite for this mode. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== Scale scrollbar in use-zoom-for-dsf mode * Scale the size of scrollbar * Compute the trackRect in ScrollbarThemeNonMacCommon using width/height of the reference scrollbar to compute thickness, instead of using scrollbarThickness BUG=485650 TEST=manual. Layout test will be covered in a separate CL that adds virtual test suite for this mode. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
oshima@chromium.org changed reviewers: - danakj@chromium.org
The CQ bit was checked by oshima@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1560403002/260001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1560403002/260001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
skobes@, can you take another look? I added Scrollbar::scrollbarThickness to deal with dynamic dsf change. I also noticed that just scaling thumb image wasn't enough. It may need to change the way scrollbar is drawn, so I removed cc/ for now. I'll address it in a separate CL.
The CQ bit was checked by oshima@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1560403002/280001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1560403002/280001
https://codereview.chromium.org/1560403002/diff/280001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp (right): https://codereview.chromium.org/1560403002/diff/280001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:251: horizontalThickness = horizontalScrollbar->scrollbarThickness(); This line and the next are backwards? https://codereview.chromium.org/1560403002/diff/280001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/HostWindow.h (right): https://codereview.chromium.org/1560403002/diff/280001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/HostWindow.h:53: // Converts the length in screen coordinates to the viewport coordinates. "in" -> "from"
Patchset #8 (id:300001) has been deleted
ptal https://codereview.chromium.org/1560403002/diff/280001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp (right): https://codereview.chromium.org/1560403002/diff/280001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:251: horizontalThickness = horizontalScrollbar->scrollbarThickness(); On 2016/01/13 23:46:00, skobes wrote: > This line and the next are backwards? Good catch, thanks! https://codereview.chromium.org/1560403002/diff/280001/third_party/WebKit/Sou... File third_party/WebKit/Source/platform/HostWindow.h (right): https://codereview.chromium.org/1560403002/diff/280001/third_party/WebKit/Sou... third_party/WebKit/Source/platform/HostWindow.h:53: // Converts the length in screen coordinates to the viewport coordinates. On 2016/01/13 23:46:00, skobes wrote: > "in" -> "from" Done.
The CQ bit was checked by oshima@chromium.org to run a CQ dry run
lgtm except... https://codereview.chromium.org/1560403002/diff/320001/cc/layers/painted_scro... File cc/layers/painted_scrollbar_layer.cc (right): https://codereview.chromium.org/1560403002/diff/320001/cc/layers/painted_scro... cc/layers/painted_scrollbar_layer.cc:290: LOG(ERROR) << "scale:" << scale_x << "x" << scale_y remove this :)
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1560403002/320001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1560403002/320001
The CQ bit was unchecked by oshima@chromium.org
On 2016/01/14 00:00:36, skobes wrote: > lgtm except... > > https://codereview.chromium.org/1560403002/diff/320001/cc/layers/painted_scro... > File cc/layers/painted_scrollbar_layer.cc (right): > > https://codereview.chromium.org/1560403002/diff/320001/cc/layers/painted_scro... > cc/layers/painted_scrollbar_layer.cc:290: LOG(ERROR) << "scale:" << scale_x << > "x" << scale_y > remove this :) That was meant to be in different working branch, sorry. removed.
eae@ -> owners
The CQ bit was checked by oshima@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1560403002/340001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1560403002/340001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_blink_rel on tryserver.blink (JOB_TIMED_OUT, no build URL)
LGTM
The CQ bit was checked by oshima@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from skobes@chromium.org Link to the patchset: https://codereview.chromium.org/1560403002/#ps340001 (title: "remove unintentional change")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1560403002/340001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1560403002/340001
Message was sent while issue was closed.
Description was changed from ========== Scale scrollbar in use-zoom-for-dsf mode * Scale the size of scrollbar * Compute the trackRect in ScrollbarThemeNonMacCommon using width/height of the reference scrollbar to compute thickness, instead of using scrollbarThickness BUG=485650 TEST=manual. Layout test will be covered in a separate CL that adds virtual test suite for this mode. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== Scale scrollbar in use-zoom-for-dsf mode * Scale the size of scrollbar * Compute the trackRect in ScrollbarThemeNonMacCommon using width/height of the reference scrollbar to compute thickness, instead of using scrollbarThickness BUG=485650 TEST=manual. Layout test will be covered in a separate CL that adds virtual test suite for this mode. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
Message was sent while issue was closed.
Committed patchset #9 (id:340001)
Message was sent while issue was closed.
Description was changed from ========== Scale scrollbar in use-zoom-for-dsf mode * Scale the size of scrollbar * Compute the trackRect in ScrollbarThemeNonMacCommon using width/height of the reference scrollbar to compute thickness, instead of using scrollbarThickness BUG=485650 TEST=manual. Layout test will be covered in a separate CL that adds virtual test suite for this mode. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== Scale scrollbar in use-zoom-for-dsf mode * Scale the size of scrollbar * Compute the trackRect in ScrollbarThemeNonMacCommon using width/height of the reference scrollbar to compute thickness, instead of using scrollbarThickness BUG=485650 TEST=manual. Layout test will be covered in a separate CL that adds virtual test suite for this mode. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Committed: https://crrev.com/0a1920a7d4dd833fa8cffb4b7acd38b040236326 Cr-Commit-Position: refs/heads/master@{#369477} ==========
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/0a1920a7d4dd833fa8cffb4b7acd38b040236326 Cr-Commit-Position: refs/heads/master@{#369477}
Message was sent while issue was closed.
A revert of this CL (patchset #9 id:340001) has been created in https://codereview.chromium.org/1586983004/ by huangs@chromium.org. The reason for reverting is: This is causing oilpan compile errors under Clang: ../../third_party/WebKit/Source/platform/scroll/Scrollbar.h:196:5: note: [blink-gc] Raw pointer field 'm_hostWindow' to a GC managed class declared here: HostWindow* m_hostWindow;.
Message was sent while issue was closed.
On 2016/01/14 19:09:27, huangs wrote: > A revert of this CL (patchset #9 id:340001) has been created in > https://codereview.chromium.org/1586983004/ by mailto:huangs@chromium.org. > > The reason for reverting is: This is causing oilpan compile errors under Clang: > > ../../third_party/WebKit/Source/platform/scroll/Scrollbar.h:196:5: note: > [blink-gc] Raw pointer field 'm_hostWindow' to a GC managed class declared here: > HostWindow* m_hostWindow;. Thank you for reverting. skobes@, I'm not familiar with oilpan. What's the right way to fix this? Should I use RawPtrWillBeMember?
Message was sent while issue was closed.
On 2016/01/14 19:16:16, oshima wrote: > On 2016/01/14 19:09:27, huangs wrote: > > A revert of this CL (patchset #9 id:340001) has been created in > > https://codereview.chromium.org/1586983004/ by mailto:huangs@chromium.org. > > > > The reason for reverting is: This is causing oilpan compile errors under > Clang: > > > > ../../third_party/WebKit/Source/platform/scroll/Scrollbar.h:196:5: note: > > [blink-gc] Raw pointer field 'm_hostWindow' to a GC managed class declared > here: > > HostWindow* m_hostWindow;. > > Thank you for reverting. > > skobes@, I'm not familiar with oilpan. What's the right way to fix this? Should > I use RawPtrWillBeMember? I've got help from sof@. Thanks!
Message was sent while issue was closed.
Description was changed from ========== Scale scrollbar in use-zoom-for-dsf mode * Scale the size of scrollbar * Compute the trackRect in ScrollbarThemeNonMacCommon using width/height of the reference scrollbar to compute thickness, instead of using scrollbarThickness BUG=485650 TEST=manual. Layout test will be covered in a separate CL that adds virtual test suite for this mode. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Committed: https://crrev.com/0a1920a7d4dd833fa8cffb4b7acd38b040236326 Cr-Commit-Position: refs/heads/master@{#369477} ========== to ========== Scale scrollbar in use-zoom-for-dsf mode * Scale the size of scrollbar * Compute the trackRect in ScrollbarThemeNonMacCommon using width/height of the reference scrollbar to compute thickness, instead of using scrollbarThickness BUG=485650 TEST=manual. Layout test will be covered in a separate CL that adds virtual test suite for this mode. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Committed: https://crrev.com/0a1920a7d4dd833fa8cffb4b7acd38b040236326 Cr-Commit-Position: refs/heads/master@{#369477} ========== |
