Bug 82027

Summary:

Move Notifications APIs from DOMWindow.idl to DOMWindowNotifications.idl (Part 3)

Product:

WebKit

Reporter:

Adam Barth <abarth>

Component:

New Bugs

Assignee:

Adam Barth <abarth>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

79636

Attachments:

Description	Flags
Patch	none

Description Adam Barth 2012-03-23 00:03:04 PDT

Move Notifications APIs from DOMWindow.idl to DOMWindowNotifications.idl (Part 3)

Comment 1 Adam Barth 2012-03-23 00:07:18 PDT

Created attachment 133437 [details]
Patch

Comment 2 Kentaro Hara 2012-03-23 00:15:21 PDT

Comment on attachment 133437 [details]
Patch

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

> Source/WebCore/notifications/DOMWindowNotifications.cpp:75
> +    DOMWindowProperty::reconnectFrame(frame);
> +    m_notificationCenter = m_suspendedNotificationCenter.release();

Nit: Shall we exchange the two statements, for consistency? DOMWindowNotification operations, and then DOMWindowProperty:: operations.

Comment 3 Adam Barth 2012-03-23 00:20:23 PDT

(In reply to comment #2)
> (From update of attachment 133437 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133437&action=review
> 
> > Source/WebCore/notifications/DOMWindowNotifications.cpp:75
> > +    DOMWindowProperty::reconnectFrame(frame);
> > +    m_notificationCenter = m_suspendedNotificationCenter.release();
> 
> Nit: Shall we exchange the two statements, for consistency? DOMWindowNotification operations, and then DOMWindowProperty:: operations.

I'm not sure.  I got the idea for this ordering from beidson's patch to http://trac.webkit.org/browser/trunk/Source/WebCore/Modules/indexeddb/DOMWindowIndexedDatabase.cpp#L61 .  The idea is that you want to suspect the notification center before you actually disconnect the frame, but you want to reconnect the frame before you "unsuspend" the notification center.

I don't think it makes any difference in practice.  If you'd prefer the other ordering, I'm happy to change it.

Comment 4 Adam Barth 2012-03-23 14:52:00 PDT

Comment on attachment 133437 [details]
Patch

I think I'm going to leave the calls in this order.  It's a little idiosyncratic, but I think that's ok.

Comment 5 WebKit Review Bot 2012-03-23 15:36:12 PDT

Comment on attachment 133437 [details]
Patch

Clearing flags on attachment: 133437

Committed r111920: <http://trac.webkit.org/changeset/111920>

Comment 6 WebKit Review Bot 2012-03-23 15:36:16 PDT

All reviewed patches have been landed.  Closing bug.