WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
129551
Split UserActivity, simplify PageThrottler
https://bugs.webkit.org/show_bug.cgi?id=129551
Summary
Split UserActivity, simplify PageThrottler
Gavin Barraclough
Reported
2014-03-01 12:25:22 PST
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.
Attachments
Fix
(25.83 KB, patch)
2014-03-01 17:04 PST
,
Gavin Barraclough
darin
: review+
Details
Formatted Diff
Diff
EWS fixes
(27.76 KB, patch)
2014-03-01 23:38 PST
,
Gavin Barraclough
no flags
Details
Formatted Diff
Diff
GTK fix
(27.75 KB, patch)
2014-03-02 11:29 PST
,
Gavin Barraclough
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Gavin Barraclough
Comment 1
2014-03-01 17:04:03 PST
Created
attachment 225575
[details]
Fix
Darin Adler
Comment 2
2014-03-01 18:13:00 PST
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
Gavin Barraclough
Comment 3
2014-03-01 23:38:43 PST
Created
attachment 225590
[details]
EWS fixes
Gavin Barraclough
Comment 4
2014-03-02 11:29:33 PST
Created
attachment 225603
[details]
GTK fix
Gavin Barraclough
Comment 5
2014-03-02 12:38:59 PST
Transmitting file data ............ Committed revision 164948.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug