Bug 139744

Summary: Remove custom token mechanism from ProcessThrottler
Product: WebKit Reporter: Gavin Barraclough <barraclough>
Component: WebKit2Assignee: Gavin Barraclough <barraclough>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cmarcelo, commit-queue
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 139776    
Bug Blocks:    
Attachments:
Description Flags
Fix ggaren: review+

Description Gavin Barraclough 2014-12-17 13:21:13 PST
ProcessThrottler implements its own token-based counter mechanism, but should just use RefCounter.

There is one new feature here. ProcessThrottler used distinct token types for background & foreground activity. It would be a compile error to try to store a background token in the handle for a foreground token, and vice versa. RefCounter doesn't currently provide the facility to distinguish token types in this manner (all tokens are just Ref<RefCounter::Count>). Add a convenient way to easily create distinct token types.
Comment 1 Gavin Barraclough 2014-12-17 13:29:10 PST
Created attachment 243456 [details]
Fix
Comment 2 WebKit Commit Bot 2014-12-17 13:32:03 PST
Attachment 243456 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/GenericCallback.h:194:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/RefCounter.h:73:  The parameter name "ref" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/RefCounter.h:77:  The parameter name "token" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/RefCounter.h:78:  The parameter name "token" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/RefCounter.h:81:  The parameter name "o" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/RefCounter.h:82:  The parameter name "o" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit2/UIProcess/ProcessThrottler.cpp:39:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 7 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Gavin Barraclough 2014-12-17 13:34:56 PST
Comment on attachment 243456 [details]
Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=243456&action=review

> Source/WTF/ChangeLog:24
> +            - tokens can be copied, assigned to, and suport operator!.

suport -> support
Comment 4 Geoffrey Garen 2014-12-17 13:54:40 PST
Comment on attachment 243456 [details]
Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=243456&action=review

r=me

> Source/WTF/wtf/RefCounter.h:73
> +        inline explicit Token(Ref<Count>&& ref);

I think implicit is cool here. There's nothing lossy about accepting an rvalue reference to a count in order to make a token.

> Source/WebKit2/UIProcess/GenericCallback.h:72
> -    explicit CallbackBase(Type type, std::unique_ptr<ProcessThrottler::BackgroundActivityToken> activityToken)
> +    explicit CallbackBase(Type type, ProcessThrottler::BackgroundActivityToken activityToken)

I think you want some reference action here to avoid churning the refcounts.

> Source/WebKit2/UIProcess/ProcessThrottler.h:42
> +    class ForegroundActivityTokenClass;
> +    class BackgroundActivityTokenClass;

Maybe this would be clearer if you used an enum instead of opaque class types. enum is OK as template argument.
Comment 5 Gavin Barraclough 2014-12-18 16:54:47 PST
Landed WTF changes separately as https://bugs.webkit.org/show_bug.cgi?id=139776.
Comment 6 Gavin Barraclough 2014-12-18 17:41:12 PST
Transmitting file data ............
Committed revision 177548.