Bug 91313 - Increase the readability of Functional related to handling wrapping/unwrapping RefPtr and PassRefPtr.
Summary: Increase the readability of Functional related to handling wrapping/unwrappin...
Status: UNCONFIRMED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 91310
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-13 23:48 PDT by Dongseong Hwang
Modified: 2012-08-13 02:04 PDT (History)
11 users (show)

See Also:


Attachments
Patch (2.83 KB, patch)
2012-07-13 23:55 PDT, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (5.17 KB, patch)
2012-07-14 00:26 PDT, Dongseong Hwang
eric: review-
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dongseong Hwang 2012-07-13 23:48:00 PDT
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.
Comment 1 Dongseong Hwang 2012-07-13 23:55:30 PDT
Created attachment 152411 [details]
Patch
Comment 2 Dongseong Hwang 2012-07-13 23:57:28 PDT
This patch amended the code that had been implemented by only Bug 75266.
Comment 3 WebKit Review Bot 2012-07-14 00:01:19 PDT
Comment on attachment 152411 [details]
Patch

Attachment 152411 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13230468
Comment 4 Build Bot 2012-07-14 00:03:52 PDT
Comment on attachment 152411 [details]
Patch

Attachment 152411 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13241361
Comment 5 Dongseong Hwang 2012-07-14 00:11:25 PDT
(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.
Comment 6 Gustavo Noronha (kov) 2012-07-14 00:15:53 PDT
Comment on attachment 152411 [details]
Patch

Attachment 152411 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/13237412
Comment 7 Build Bot 2012-07-14 00:19:17 PDT
Comment on attachment 152411 [details]
Patch

Attachment 152411 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13236402
Comment 8 Dongseong Hwang 2012-07-14 00:26:26 PDT
Created attachment 152412 [details]
Patch
Comment 9 Dongseong Hwang 2012-07-14 00:30:50 PDT
(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.
Comment 10 WebKit Review Bot 2012-07-14 00:32:57 PDT
Comment on attachment 152412 [details]
Patch

Attachment 152412 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13237415
Comment 11 Build Bot 2012-07-14 00:41:23 PDT
Comment on attachment 152412 [details]
Patch

Attachment 152412 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13221957
Comment 12 Build Bot 2012-07-14 00:51:55 PDT
Comment on attachment 152412 [details]
Patch

Attachment 152412 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13236411
Comment 13 Peter Beverloo (cr-android ews) 2012-08-09 23:53:55 PDT
Comment on attachment 152412 [details]
Patch

Attachment 152412 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/13471439
Comment 14 Eric Seidel (no email) 2012-08-12 03:28:03 PDT
This doesn't compile and thus can't be landed.
Comment 15 Dongseong Hwang 2012-08-13 02:04:10 PDT
(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