Chromium Code Reviews Chromium Code Reviews
Issue 2780883002:
Correct overlay scrollbar check FrameView::scrollbarExistenceDidChange (Closed)
| OLD | NEW |
|---|---|
| 1 // Copyright 2016 The Chromium Authors. All rights reserved. | 1 // Copyright 2016 The Chromium Authors. All rights reserved. |
| 2 // Use of this source code is governed by a BSD-style license that can be | 2 // Use of this source code is governed by a BSD-style license that can be |
| 3 // found in the LICENSE file. | 3 // found in the LICENSE file. |
| 4 | 4 |
| 5 #include "core/frame/FrameView.h" | |
| 5 #include "core/layout/LayoutView.h" | 6 #include "core/layout/LayoutView.h" |
| 6 #include "core/paint/PaintLayerScrollableArea.h" | 7 #include "core/paint/PaintLayerScrollableArea.h" |
| 7 #include "platform/testing/RuntimeEnabledFeaturesTestHelpers.h" | 8 #include "platform/testing/RuntimeEnabledFeaturesTestHelpers.h" |
| 8 #include "platform/testing/UnitTestHelpers.h" | 9 #include "platform/testing/UnitTestHelpers.h" |
| 9 #include "public/web/WebScriptSource.h" | 10 #include "public/web/WebScriptSource.h" |
| 10 #include "testing/gtest/include/gtest/gtest.h" | 11 #include "testing/gtest/include/gtest/gtest.h" |
| 11 #include "web/WebLocalFrameImpl.h" | 12 #include "web/WebLocalFrameImpl.h" |
| 12 #include "web/tests/sim/SimDisplayItemList.h" | 13 #include "web/tests/sim/SimDisplayItemList.h" |
| 13 #include "web/tests/sim/SimRequest.h" | 14 #include "web/tests/sim/SimRequest.h" |
| 14 #include "web/tests/sim/SimTest.h" | 15 #include "web/tests/sim/SimTest.h" |
| (...skipping 20 matching lines...) Expand all Loading... | |
| 35 | 36 |
| 36 // Forces recalc of LayoutView's computed style in Document::updateStyle, | 37 // Forces recalc of LayoutView's computed style in Document::updateStyle, |
| 37 // without invalidating layout. | 38 // without invalidating layout. |
| 38 mainFrame().executeScriptAndReturnValue(WebScriptSource( | 39 mainFrame().executeScriptAndReturnValue(WebScriptSource( |
| 39 "document.querySelector('style').sheet.insertRule('body {}', 1);")); | 40 "document.querySelector('style').sheet.insertRule('body {}', 1);")); |
| 40 | 41 |
| 41 compositor().beginFrame(); | 42 compositor().beginFrame(); |
| 42 ASSERT_TRUE(plsa->verticalScrollbar() && plsa->horizontalScrollbar()); | 43 ASSERT_TRUE(plsa->verticalScrollbar() && plsa->horizontalScrollbar()); |
| 43 } | 44 } |
| 44 | 45 |
| 46 // Ensure that causing a change in scrollbar existence causes a nested layout | |
| 47 // to recalculate the existence of the opposite scrollbar. The bug here was | |
| 48 // caused by trying to avoid the layout when overlays are enabled but not | |
| 49 // checking whether the scrollbars should be custom - which do take up layout | |
| 50 // space. https://crbug.com/668387. | |
| 51 TEST_F(ScrollbarsTest, CustomScrollbarsCauseLayoutOnExistenceChange) { | |
| 52 // The bug reproduces only with RLS off. When RLS ships we can keep the test | |
| 53 // but remove this setting. | |
| 54 ScopedRootLayerScrollingForTest turnOffRootLayerScrolling(false); | |
| 55 | |
| 56 // This test is specifically checking the behavior when overlay scrollbars | |
| 57 // are enabled. | |
| 58 DCHECK(ScrollbarTheme::theme().usesOverlayScrollbars()); | |
|
skobes
2017/03/28 21:22:34
Doesn't this DCHECK make the test crash on platfor
bokan
2017/03/28 21:26:32
No, unit tests run with mock overlay scrollbars en
| |
| 59 | |
| 60 webView().resize(WebSize(800, 600)); | |
| 61 SimRequest request("https://example.com/test.html", "text/html"); | |
| 62 loadURL("https://example.com/test.html"); | |
| 63 request.complete( | |
| 64 "<!DOCTYPE html>" | |
| 65 "<style>" | |
| 66 " ::-webkit-scrollbar {" | |
| 67 " height: 16px;" | |
| 68 " width: 16px" | |
| 69 " }" | |
| 70 " ::-webkit-scrollbar-thumb {" | |
| 71 " background-color: rgba(0,0,0,.2);" | |
| 72 " }" | |
| 73 " html, body{" | |
| 74 " margin: 0;" | |
| 75 " height: 100%;" | |
| 76 " }" | |
| 77 " .box {" | |
| 78 " width: 100%;" | |
| 79 " height: 100%;" | |
| 80 " }" | |
| 81 " .transformed {" | |
| 82 " transform: translateY(100px);" | |
| 83 " }" | |
| 84 "</style>" | |
| 85 "<div id='box' class='box'></div>"); | |
| 86 | |
| 87 ScrollableArea* layoutViewport = | |
| 88 document().view()->layoutViewportScrollableArea(); | |
| 89 | |
| 90 compositor().beginFrame(); | |
| 91 ASSERT_FALSE(layoutViewport->verticalScrollbar()); | |
| 92 ASSERT_FALSE(layoutViewport->horizontalScrollbar()); | |
| 93 | |
| 94 // Adding translation will cause a vertical scrollbar to appear but not dirty | |
| 95 // layout otherwise. Ensure the change of scrollbar causes a layout to | |
| 96 // recalculate the page width with the vertical scrollbar added. | |
| 97 mainFrame().executeScript(WebScriptSource( | |
| 98 "document.getElementById('box').className = 'box transformed';")); | |
| 99 compositor().beginFrame(); | |
| 100 | |
| 101 ASSERT_TRUE(layoutViewport->verticalScrollbar()); | |
| 102 ASSERT_FALSE(layoutViewport->horizontalScrollbar()); | |
| 103 } | |
| 104 | |
| 45 } // namespace | 105 } // namespace |
| 46 | 106 |
| 47 } // namespace blink | 107 } // namespace blink |
| OLD | NEW |
