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

• Component: New Bugs
• Status: RESOLVED FIXED

[Adam Barth]

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

[Adam Barth]

Created attachment 133437 [details]
Patch

[Kentaro Hara]

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.

[Adam Barth]

(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.

[Adam Barth]

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.

[WebKit Review Bot]

Comment on attachment 133437 [details]
Patch

Clearing flags on attachment: 133437

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

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.