[Webkit-unassigned] [Bug 40003] [Qt] Fix the lifecycle of notification objects

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 1 12:03:13 PDT 2010


--- Comment #2 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2010-06-01 12:03:13 PST ---
(From update of attachment 57554)
 +      NotificationPresenterClientQt::notificationPresenter()->addPage();
isn't it more like a addClient? You are adding a page as a client right?

 +      s_notificationPresenter = new NotificationPresenterClientQt();
So we always leak this one? (im not finished reading the patch), maybe we should use STATIC_LOCAL

 +      m_closeTimer.startOneShot(10.0);
Maybe create a static global for the timeout value.

 +          delete this;
Ah you delete it here.

 +  void NotificationPresenterClientQt::removePage()
I don't like these names, remove/addPage, because it is not obvious from the names that they are used for ref counting.

 +      void addPage() { m_pageCount++; }
even harder when one part of the implementation is defined in the header

 +      int m_pageCount;
add a new line between the dumpshowText() and the variables

Apart from these nits, I like the patch!

Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

More information about the webkit-unassigned mailing list