Bug 139744 – Remove custom token mechanism from ProcessThrottler

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Gavin Barraclough]

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.

[Gavin Barraclough]

Created attachment 243456 [details]
Fix

[WebKit Commit Bot]

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.

[Gavin Barraclough]

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

[Geoffrey Garen]

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.

[Gavin Barraclough]

Landed WTF changes separately as https://bugs.webkit.org/show_bug.cgi?id=139776.

[Gavin Barraclough]

Transmitting file data ............
Committed revision 177548.