Summary: | [ContentChangeObserver] Introduce ContentChangeObserver::adjustObservedState | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | zalan <zalan> | ||||
Component: | Layout and Rendering | Assignee: | zalan <zalan> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bfulgham, simon.fraser, webkit-bug-importer, zalan | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
zalan
2019-03-02 08:38:12 PST
Created attachment 363422 [details]
Patch
Comment on attachment 363422 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363422&action=review > LayoutTests/fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html:3 > +<title>This test that we trigger hover when the content change starts as async but it turns into a sync style recalc.</title> This tests that > LayoutTests/fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html:50 > + document.body.offsetHeight; Is this necessary? > Source/WebCore/page/ios/ContentChangeObserver.cpp:143 > void ContentChangeObserver::didContentVisibilityChange() contentVisibilityDidChange? didContentVisibilityChange sounds like a question. > Source/WebCore/page/ios/ContentChangeObserver.cpp:197 > + if (event == Event::DidContentObservationStart) { Use a switch so that you get warnings from the complier? > Source/WebCore/page/ios/ContentChangeObserver.cpp:219 > +void ContentChangeObserver::signalContentChangeIfNeeded() I think "notify" would be a better term to use. > Source/WebCore/page/ios/ContentChangeObserver.h:93 > + void addObservedDOMTimer(const DOMTimer&); > void removeObservedDOMTimer(const DOMTimer&); Normally we'd say "startObservingFoo/stopObservingFoo". > Source/WebCore/page/ios/ContentChangeObserver.h:115 > + enum class Event { Event is a bit of an overloaded term. Maybe OK here. > Source/WebCore/page/ios/ContentChangeObserver.h:119 > + DidContentObservationStart, > + DidInstallDOMTimer, > + DidRemoveDOMTimer, > + DidContentVisibilityChange I think you can remove the "Did" from all these. Maybe use the past tense: Event::InstalledDOMTimer. Committed r242321: <https://trac.webkit.org/changeset/242321> |