Bug 237485 – Modernize OriginLock

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

Modernize OriginLock.

[Chris Dumez]

Created attachment 453870 [details]
Patch

[Chris Dumez]

Created attachment 453871 [details]
Patch

[Darin Adler]

Comment on attachment 453871 [details]
Patch

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

> Source/WebCore/Modules/webdatabase/OriginLock.h:-36
> -    WTF_MAKE_NONCOPYABLE(OriginLock); WTF_MAKE_FAST_ALLOCATED;

Why aren’t these helpful? I don’t understand this change.

> Source/WebCore/Modules/webdatabase/OriginLock.h:40
> +    static Ref<OriginLock> create(const String& originPath)
> +    {
> +        return adoptRef(*new OriginLock(originPath));
> +    }

I don’t think we want to inline this. It would be better to have it in the .cpp file, since we’d rather inline the constructor in the create function rather than inlining the create function in the create call site.

[Chris Dumez]

(In reply to Darin Adler from comment #3)
> Comment on attachment 453871 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=453871&action=review
> 
> > Source/WebCore/Modules/webdatabase/OriginLock.h:-36
> > -    WTF_MAKE_NONCOPYABLE(OriginLock); WTF_MAKE_FAST_ALLOCATED;
> 
> Why aren’t these helpful? I don’t understand this change.

We get them from the base class already.

> 
> > Source/WebCore/Modules/webdatabase/OriginLock.h:40
> > +    static Ref<OriginLock> create(const String& originPath)
> > +    {
> > +        return adoptRef(*new OriginLock(originPath));
> > +    }
> 
> I don’t think we want to inline this. It would be better to have it in the
> .cpp file, since we’d rather inline the constructor in the create function
> rather than inlining the create function in the create call site.

OK.

[Darin Adler]

Comment on attachment 453871 [details]
Patch

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

>>> Source/WebCore/Modules/webdatabase/OriginLock.h:-36
>>> -    WTF_MAKE_NONCOPYABLE(OriginLock); WTF_MAKE_FAST_ALLOCATED;
>> 
>> Why aren’t these helpful? I don’t understand this change.
> 
> We get them from the base class already.

Not being copyable or movable is effectively inherited. Is fast allocation inherited? I didn’t realize it was.

[Chris Dumez]

(In reply to Darin Adler from comment #5)
> Comment on attachment 453871 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=453871&action=review
> 
> >>> Source/WebCore/Modules/webdatabase/OriginLock.h:-36
> >>> -    WTF_MAKE_NONCOPYABLE(OriginLock); WTF_MAKE_FAST_ALLOCATED;
> >> 
> >> Why aren’t these helpful? I don’t understand this change.
> > 
> > We get them from the base class already.
> 
> Not being copyable or movable is effectively inherited. Is fast allocation
> inherited? I didn’t realize it was.

I am pretty sure it is. As a matter of fact, we almost never explicit mark our classes as FACT_ALLOCATED when they subclass RefCounted.

To be certain, I ran the following code:
```
#include <iostream>

class A {
public:
    void* operator new(size_t size)
    {
        std::cout << "Uses custom allocator" << std::endl;
        return malloc(size);
    }
};

class B : public A {
};

int main() {
    auto b = new B;
    return 0;
}
```

It definitely printed my log line so the `operator new` gets inherited just fine and this is essentially what WTF_MAKE_FAST_ALLOCATED defines.

[EWS]

Committed r290854 (248086@main): <https://commits.webkit.org/248086@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 453871 [details].

[Radar WebKit Bug Importer]

<rdar://problem/89841503>