Bug 91313 – Increase the readability of Functional related to handling wrapping/unwrapping RefPtr and PassRefPtr.

• Status: UNCONFIRMED
• Component: JavaScriptCore

[Dongseong Hwang]

There are two changes.

1. Partial template specialization of RefAndDeref on RefPtr and PassRefPtr.
Currently, RefAndDeref<RefPtr<T>, true>::ref(m_p1) bounds RefAndDeref<T*, true>.
"static void ref(T* t) { t->ref(); }" does work correctly because m_p1->ref() is
legal, but it is hard to read. It is why this patch specialized on RefPtr and
PassRefPtr also.

2. ParamStorageTraits<(RefPtr|PassRefPtr)<T> >::unref returns PassRefPtr instead of raw pointer.
The raw pointer is eventually casted to PassRefPtr, so it is better to return
PassRefPtr for both performance and readability.

[Dongseong Hwang]

Created attachment 152411 [details]
Patch

[Dongseong Hwang]

This patch amended the code that had been implemented by only Bug 75266.

[WebKit Review Bot]

Comment on attachment 152411 [details]
Patch

Attachment 152411 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13230468

[Build Bot]

Comment on attachment 152411 [details]
Patch

Attachment 152411 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13241361

[Dongseong Hwang]

(In reply to comment #3)
> (From update of attachment 152411 [details])
> Attachment 152411 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/13230468

Chromium build failed because of the following code in Functional.cpp.

static int multiplyNumberByTwo(Number* number)
{
    return number->value() * 2;
}

TEST(FunctionalTest, RefCountedStorage)
{
    RefPtr<Number> five = Number::create(5);
    Function<int ()> multiplyFiveByTwoFunction = bind(multiplyNumberByTwo, five);
    ....
}

I think above code is not correct because we can not call "multiplyNumberByTwo(five)" also.

I fixed above code in Bug 91310, so this bug depends on Bug 91310.

[Gustavo Noronha (kov)]

Comment on attachment 152411 [details]
Patch

Attachment 152411 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/13237412

[Build Bot]

Comment on attachment 152411 [details]
Patch

Attachment 152411 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13236402

[Dongseong Hwang]

Created attachment 152412 [details]
Patch

[Dongseong Hwang]

(In reply to comment #4)
> (From update of attachment 152411 [details])
> Attachment 152411 [details] did not pass mac-ews (mac):
> Output: http://queues.webkit.org/results/13241361

WebProcessProxyMac used Functional like Functional.cpp that caused chromium build failed. I fixed next patch.
Win and Gtk also failed in the same reason of chromium.

[WebKit Review Bot]

Comment on attachment 152412 [details]
Patch

Attachment 152412 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13237415

[Build Bot]

Comment on attachment 152412 [details]
Patch

Attachment 152412 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13221957

[Build Bot]

Comment on attachment 152412 [details]
Patch

Attachment 152412 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13236411

[Peter Beverloo (cr-android ews)]

Comment on attachment 152412 [details]
Patch

Attachment 152412 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/13471439

[Eric Seidel (no email)]

This doesn't compile and thus can't be landed.

[Dongseong Hwang]

(In reply to comment #14)
> This doesn't compile and thus can't be landed.

It is because this bug depends on Bug 91222 and 91310. I'll re-upload a patch after landing Bug 91222 and 91310