UserActivity is a mechanism to express to the operating system (where appropriate) that a user initiated activity is taking place, and as such that resources should be made available to the process accordingly. Currently we hold a single NSActivity, at the WebKit layer. This refactoring allows us to hold different activity tokens for different user actions (which simplifies the handling, and aides debugging since the token can more accurately express the activity taking place), and also will allow us to avoid the layering difficulty of calling back up the stack to WebKit to register that an activity is taking place.
Created attachment 220175 [details] Fix
Comment on attachment 220175 [details] Fix Attachment 220175 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/5535485384458240
Comment on attachment 220175 [details] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=220175&action=review > Source/WTF/ChangeLog:16 > + UserActivity is a mechanism to express to the operating system (where appropriate) > + that a user initiated activity is taking place, and as such that resources should be > + made available to the process accordingly. > + > + Currently we hold a single NSActivity, at the WebKit layer. This refactoring allows us > + to hold different activity tokens for different user actions (which simplifies the > + handling, and aides debugging since the token can more accurately express the activity > + taking place), and also will allow us to avoid the layering difficulty of calling back > + up the stack to WebKit to register that an activity is taking place. Why is WTF the right layer for this? Will JSC want to use this? If not, it might make more sense to put this in WebCore/platform. > Source/WTF/wtf/UserActivity.h:34 > +class UserActivity { I think this is an abstract enough concept that this could use a comment explaining what this about. > Source/WTF/wtf/UserActivity.h:46 > +#if PLATFORM(MAC) && !PLATFORM(IOS) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1090 Instead of this complicated #ifdef here, I would rather there be a single HAVE(NS_ACTIVITY) or something like that. > Source/WTF/wtf/UserActivity.h:50 > + RetainPtr<id> m_description; Can this be a WTF::String or at least a RetainPtr<NSString>. > Source/WTF/wtf/mac/UserActivityMac.mm:66 > + syslog(LOG_ERR, "beginActivity(%s) -> %d", [m_description.get() UTF8String], (int)m_count); Why syslog and not one of the normal logging means? > Source/WTF/wtf/mac/UserActivityMac.mm:93 > +// ASSERT(isValid()); This looks bogus. > Source/WebKit2/Shared/ChildProcess.h:125 > + UserActivity m_processSuppressionDisabled; > + UserActivity m_activeTasks; Since we have cross platform type now, can we move these out of PLATFORM(MAC)?
Created attachment 220179 [details] GTK fix
> > Source/WTF/wtf/mac/UserActivityMac.mm:66 > > + syslog(LOG_ERR, "beginActivity(%s) -> %d", [m_description.get() UTF8String], (int)m_count); > > Why syslog and not one of the normal logging means? Ooops, debug code I was using to test, meant to delete. :-| Will clean up & fix a couple of the other review comments, then put up a new patch. Clearing r? for now.
Created attachment 220282 [details] Fix
Comment on attachment 220282 [details] Fix Attachment 220282 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/6405113719554048
Created attachment 220285 [details] gtk fix
Comment on attachment 220285 [details] gtk fix Attachment 220285 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/5789202524405760
Committed revision 161271.