The class UserActivity currently implements two things – a hysteresis mechanism, and an abstraction of NSActivity controlled by that mechanism. PageThrottler implements its own hysteresis mechanism, which directly controls DOM timer throttling and also controls a couple of UserActivities, giving a total of 3 separate hysteresis mechanisms, layered two deep. Split UserActivity into three, with HysteresisActivity implementing an abstract hysteresis mechanism, UserActivity::Impl controlling the NSActivity, and then UserActivity combining these two back together. The interface to UserActivity is unchanged. Remove PageThrottler's bespoke hysteresis, replacing it with a use of HysteresisActivity. Replace the two UserActivities with a single UserActivity::Impl, so there are no longer layered hysteresis mechanisms.
Created attachment 225575 [details] Fix
Comment on attachment 225575 [details] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=225575&action=review > Source/WebCore/platform/HysteresisActivity.h:42 > + UserActivity(const char* description); Should mark this explicit. > Source/WebCore/platform/HysteresisActivity.h:56 > +private: > +#if HAVE(NS_ACTIVITY) > + void hysteresisTimerFired(); > + bool isValid(); > + > + bool m_active; > + RetainPtr<NSString> m_description; > + RunLoop::Timer<UserActivity> m_timer; > + RetainPtr<id> m_activity; > +#endif I think the private has to go inside the #if or we won’t compile with it off. Maybe that’s one reason for the red I see below in EWS. > Source/WebCore/platform/HysteresisActivity.h:36 > + HysteresisActivity(Delegate* delegate, double hysteresisSeconds = DefaultHysteresisSeconds) Can delegate be a reference instead of a pointer? I think that would be much better. Also, I think this constructor should be marked explicit. > Source/WebCore/platform/HysteresisActivity.h:74 > + constexpr static const double DefaultHysteresisSeconds = 5.0; I don’t think we have working constexpr on the Windows compiler. We maybe were working around it with a macro? I’d ask Anders about this. > Source/WebCore/platform/UserActivity.h:45 > + Impl(const char* description); explicit > Source/WebCore/platform/UserActivity.h:57 > + UserActivity(const char* description); explicit
Created attachment 225590 [details] EWS fixes
Created attachment 225603 [details] GTK fix
Transmitting file data ............ Committed revision 164948.