Bug 139776 – Add strong typing to RefCounter interface, return value as a bool.

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

[Gavin Barraclough]

Currently all token vended by a RefCounter have the same type – Ref<RefCounter::Count>. This means there is no compile time type checking to prevent mistakes. Update the count() method to token<>(), templated on type used to identify the token being returned. Calls to token<T>() will return a result of type RefCounter::Token<T>.

There are a few problems with the fact the counter will return you an exact count of the number of outstanding tokens:
  - It is desirable to only fire the callback on zero-edge changes; it is more consistent to do so if the value is only readable as a boolean.
  - It is desirable to provide the value as an argument to the callback, however to make this useful for integer values it is also necessary to indicate the direction of change (0->1 is often interesting where 2->1 is not).
  - There is a mismatch between the precision of returning a count, and the inherent imprecision of a token based mechanism, where it may be difficult to guarantee absolutely no unnecessary refcount churn, and thus unintentional counter values.

[Gavin Barraclough]

Created attachment 243505 [details]
Fix

[Geoffrey Garen]

Comment on attachment 243505 [details]
Fix

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

r=me

> Source/WTF/ChangeLog:8
> +        Currently all token vended by a RefCounter have the same type â Ref<RefCounter::Count>.

No squiggles, please.

> Source/WebKit2/UIProcess/Plugins/mac/PluginProcessManagerMac.mm:37
> -void PluginProcessManager::updateProcessSuppressionState()
> +void PluginProcessManager::updateProcessSuppressionDisabled(bool disabled)
>  {
> -    bool processSuppressionEnabled = !m_processSuppressionDisabledForPageCounter.value();
> -    if (m_processSuppressionEnabled == processSuppressionEnabled)
> -        return;
> -    
> -    m_processSuppressionEnabled = processSuppressionEnabled;
> +    bool enabled = !disabled;

This is not my least favorite way not to do things.

[Gavin Barraclough]

Transmitting file data ...................
Committed revision 177541.

[Brent Fulgham]

This breaks the Windows build and perhaps others.

[Brent Fulgham]

Build errors:

5>  RefCounter.cpp
5>c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(105): warning C4812: obsolete declaration style: please use 'WTF::RefCounter::Token<T>::Token' instead
5>c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(105): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
5>c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(105): error C2751: 'WTF::RefCounter::Token<T>::{ctor}' : the name of a function parameter cannot be qualified
5>c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(105): warning C4346: 'WTF::RefCounter::Token<T>::{ctor}' : dependent name is not a type
5>          prefix with 'typename' to indicate a type
5>c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(105): error C2143: syntax error : missing ',' before '&'
5>c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(108): error C2244: 'WTF::RefCounter::Token<T>::{ctor}' : unable to match function definition to an existing declaration
5>          definition
5>          'WTF::RefCounter::Token<T>::Token(const int)'
5>          existing declarations
5>          'WTF::RefCounter::Token<T>::Token(WTF::RefCounter::Count *)'
5>          'WTF::RefCounter::Token<T>::Token(WTF::RefCounter::Token<T> &&)'
5>          'WTF::RefCounter::Token<T>::Token(const WTF::RefCounter::Token<T> &)'
5>          'WTF::RefCounter::Token<T>::Token(std::nullptr_t)'
5>          'WTF::RefCounter::Token<T>::Token(void)'
5>c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(111): warning C4812: obsolete declaration style: please use 'WTF::RefCounter::Token<T>::Token' instead
5>c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(111): warning C4551: function call missing argument list
5>c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(111): error C2277: 'WTF::RefCounter::Token<T>::{ctor}' : cannot take address of this member function
5>c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(111): error C2568: '&&' : unable to resolve function overload
5>          c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(74): could be 'WTF::RefCounter::Token<T>::Token(WTF::RefCounter::Count *)'
5>          c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(64): or       'WTF::RefCounter::Token<T>::Token(WTF::RefCounter::Token<T> &&)'
5>          c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(63): or       'WTF::RefCounter::Token<T>::Token(const WTF::RefCounter::Token<T> &)'
5>          c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(62): or       'WTF::RefCounter::Token<T>::Token(std::nullptr_t)'
5>          c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(61): or       'WTF::RefCounter::Token<T>::Token(void)'
5>c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(114): error C2433: 'WTF::RefCounter::Token<T>::{ctor}' : 'inline' not permitted on data declarations
5>c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(114): warning C4346: 'WTF::RefCounter::Token<T>::{ctor}' : dependent name is not a type
5>          prefix with 'typename' to indicate a type
5>c:\projects\webkit\opensource\source\wtf\wtf\RefCounter.h(114): error C2350: 'WTF::RefCounter::Token<T>::{ctor}' is not a static member

[Brent Fulgham]

This compiles on Windows, is this a comparable implementation?

Index: Source/WTF/wtf/RefCounter.h
===================================================================
--- Source/WTF/wtf/RefCounter.h (revision 177541)
+++ Source/WTF/wtf/RefCounter.h (working copy)
@@ -102,13 +102,13 @@
 }

 template<class T>
-inline RefCounter::Token<T>::Token(const RefCounter::Token<T>::Token<T>& token)
+inline RefCounter::Token<T>::Token(const RefCounter::Token<T>& token)
     : m_ptr(token.m_ptr)
 {
 }

 template<class T>
-inline RefCounter::Token<T>::Token(RefCounter::Token<T>::Token<T>&& token)
+inline RefCounter::Token<T>::Token(RefCounter::Token<T>&& token)
     : m_ptr(token.m_ptr)
 {
 }

[Brent Fulgham]

Reviewed my proposed change with Gavin.

Committed r177544.

[Gavin Barraclough]

Committed revision 177549.

[Beth Dakin]

Build fix: http://trac.webkit.org/changeset/177553