Chromium Code Reviews Chromium Code Reviews
Issue 1840113005:
Move viewport actions into an ApplyScroll callback. (Closed)
Base URL: https://chromium.googlesource.com/chromium/src.git@master| Index: third_party/WebKit/Source/core/dom/Document.cpp |
| diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp |
| index dae347c7eed3cb2beeaa3708f6564629b8a72db9..1358091f99915e54f50daa99f64f5e7e294ba8ae 100644 |
| --- a/third_party/WebKit/Source/core/dom/Document.cpp |
| +++ b/third_party/WebKit/Source/core/dom/Document.cpp |
| @@ -194,6 +194,7 @@ |
| #include "core/page/Page.h" |
| #include "core/page/PointerLockController.h" |
| #include "core/page/scrolling/ScrollingCoordinator.h" |
| +#include "core/page/scrolling/ViewportScrollCallback.h" |
| #include "core/svg/SVGDocumentExtensions.h" |
| #include "core/svg/SVGTitleElement.h" |
| #include "core/svg/SVGUseElement.h" |
| @@ -701,6 +702,44 @@ void Document::childrenChanged(const ChildrenChange& change) |
| m_documentElement = ElementTraversal::firstWithin(*this); |
| } |
| +void Document::updateViewportApplyScroll() |
| +{ |
| + if (!m_documentElement |
| + || !m_documentElement->isHTMLElement() |
| + || ownerElement()) |
| + return; |
| + |
| + Element* newScrollingElement = scrollingElement(); |
| + |
| + // If there is no scrolling element (in QuirksMode and body is scrollable), |
| + // install the viewport scroll callback on the <HTML> element. |
| + if (!newScrollingElement) |
| + newScrollingElement = m_documentElement; |
| + |
| + if (newScrollingElement == m_oldScrollingElement) |
| + return; |
| + |
| + ScrollStateCallback* applyScroll = nullptr; |
| + |
| + // If the scrolling element changed, remove the apply scroll from the |
| + // old one and keep it to put on the new scrolling element. |
| + if (m_oldScrollingElement) { |
|
tdresser
2016/04/05 19:31:12
Ah, is this why we need m_oldScrollingElement to b
bokan
2016/04/06 15:21:01
Ah, TBH I did that out of habit and hadn't thought
bokan
2016/04/07 01:27:15
Ok, as far as I can tell there's no way to detach
|
| + applyScroll = m_oldScrollingElement->getApplyScroll(); |
| + m_oldScrollingElement->removeApplyScroll(); |
| + } |
| + |
| + if (newScrollingElement) { |
| + if (!applyScroll) |
| + applyScroll = new ViewportScrollCallback(*this, *frameHost()); |
|
tdresser
2016/04/05 19:31:12
This is going to end up looking pretty different o
bokan
2016/04/06 15:21:01
Acknowledged.
|
| + |
| + newScrollingElement->setApplyScroll( |
| + applyScroll, |
| + "disable-native-scroll"); |
|
tdresser
2016/04/05 19:31:12
Add a comment indicating why we disable-native-scr
bokan
2016/04/06 15:21:01
Done.
|
| + } |
| + |
| + m_oldScrollingElement = newScrollingElement; |
| +} |
| + |
| AtomicString Document::convertLocalName(const AtomicString& name) |
| { |
| return isHTMLDocument() ? name.lower() : name; |
| @@ -1980,6 +2019,8 @@ void Document::updateLayout() |
| void Document::layoutUpdated() |
| { |
| + updateViewportApplyScroll(); |
|
bokan
2016/04/04 21:44:38
This is the wrong place for this. Because scrollin
|
| + |
| // Plugins can run script inside layout which can detach the page. |
| // TODO(esprehn): Can this still happen now that all plugins are out of |
| // process? |
| @@ -5992,6 +6033,7 @@ DEFINE_TRACE(Document) |
| visitor->trace(m_contextFeatures); |
| visitor->trace(m_styleSheetList); |
| visitor->trace(m_documentTiming); |
| + visitor->trace(m_oldScrollingElement); |
| visitor->trace(m_mediaQueryMatcher); |
| visitor->trace(m_scriptedAnimationController); |
| visitor->trace(m_scriptedIdleTaskController); |
