Bug 100208 – [V8] ActiveDOMObjectEpilogueVisitor is unnecessary and can be deleted

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

[V8] ActiveDOMObjectEpilogueVisitor is unnecessary and can be deleted

[Adam Barth]

Created attachment 170332 [details]
Patch

[Kentaro Hara]

Comment on attachment 170332 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        ActiveDOMObjects during every GC, this patch puts all the
> +        ActiveDOMObjects with pending activity into an object group with a live

Great idea.

> Source/WebCore/bindings/v8/V8GCController.cpp:-125
> -        if (V8MessagePort::info.equals(typeInfo)) {
> -            // We marked this port as reachable in ActiveDOMObjectPrologueVisitor.
> -            // Undo this now since the port could be not reachable in the future
> -            // if it gets disentangled (and also ActiveDOMObjectPrologueVisitor
> -            // expects to see all handles marked as weak).
> -            MessagePort* port = reinterpret_cast<MessagePort*>(object);
> -            if ((!wrapper.IsWeak() && !wrapper.IsNearDeath()) || port->hasPendingActivity())
> -                wrapper.MakeWeak(port, m_callback);
> -            return;
> -        }

Don't you need to do something for MessagePort in the new code?

[Peter Beverloo (cr-android ews)]

Comment on attachment 170332 [details]
Patch

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

[WebKit Review Bot]

Comment on attachment 170332 [details]
Patch

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

[Adam Barth]

Comment on attachment 170332 [details]
Patch

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

>> Source/WebCore/bindings/v8/V8GCController.cpp:-125
>> -        }
> 
> Don't you need to do something for MessagePort in the new code?

Yes, the message ports are handled on line 99.

[Adam Barth]

Comment on attachment 170332 [details]
Patch

/me needs to make this compile.

[Adam Barth]

Created attachment 170415 [details]
Patch

[Adam Barth]

Created attachment 170429 [details]
patch---now builds :)

[Eric Seidel (no email)]

Comment on attachment 170429 [details]
patch---now builds :)

Looks reasonable.  Same story, you'll probably want Kentaro to look.

[Adam Barth]

Comment on attachment 170429 [details]
patch---now builds :)

Based on Comment #2, I think Kentaro is happy.  Thanks for the review.

[Adam Barth]

Comment on attachment 170429 [details]
patch---now builds :)

Clearing flags on attachment: 170429

Committed r132397: <http://trac.webkit.org/changeset/132397>

[Adam Barth]

All reviewed patches have been landed.  Closing bug.