Bug 80487 - Move NotificationContents into Notification
Summary: Move NotificationContents into Notification
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.7
: P2 Normal
Assignee: Jon Lee
URL:
Keywords: InRadar
Depends on:
Blocks: 80472
  Show dependency treegraph
 
Reported: 2012-03-06 21:50 PST by Jon Lee
Modified: 2012-03-15 16:07 PDT (History)
5 users (show)

See Also:


Attachments
Patch (20.56 KB, patch)
2012-03-07 17:46 PST, Jon Lee
no flags Details | Formatted Diff | Diff
Patch (20.67 KB, patch)
2012-03-07 21:22 PST, Jon Lee
no flags Details | Formatted Diff | Diff
Patch (27.64 KB, patch)
2012-03-07 22:44 PST, Jon Lee
no flags Details | Formatted Diff | Diff
Patch (28.64 KB, patch)
2012-03-10 11:03 PST, Jon Lee
no flags Details | Formatted Diff | Diff
Patch (28.49 KB, patch)
2012-03-10 13:18 PST, Jon Lee
jianli: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jon Lee 2012-03-06 21:50:46 PST
Splitting out the data members for text notifications is unnecessary. Move those data members (title, body, URL) back into Notification, and remove NotificationContents from WebCore.

<rdar://problem/10965519>
Comment 1 Jon Lee 2012-03-07 17:46:21 PST
Created attachment 130737 [details]
Patch
Comment 2 Build Bot 2012-03-07 17:57:30 PST
Comment on attachment 130737 [details]
Patch

Attachment 130737 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/11862037
Comment 3 Early Warning System Bot 2012-03-07 18:52:57 PST
Comment on attachment 130737 [details]
Patch

Attachment 130737 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11863083
Comment 4 WebKit Review Bot 2012-03-07 19:58:25 PST
Comment on attachment 130737 [details]
Patch

Attachment 130737 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11865076
Comment 5 Jon Lee 2012-03-07 21:22:14 PST
Created attachment 130761 [details]
Patch
Comment 6 Early Warning System Bot 2012-03-07 21:40:42 PST
Comment on attachment 130761 [details]
Patch

Attachment 130761 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11866121
Comment 7 WebKit Review Bot 2012-03-07 21:43:26 PST
Comment on attachment 130761 [details]
Patch

Attachment 130761 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11861150
Comment 8 Jon Lee 2012-03-07 22:44:05 PST
Created attachment 130772 [details]
Patch
Comment 9 Jon Lee 2012-03-10 11:03:19 PST
Created attachment 131177 [details]
Patch
Comment 10 Jon Lee 2012-03-10 13:18:50 PST
Created attachment 131186 [details]
Patch
Comment 11 Jian Li 2012-03-15 11:31:47 PDT
Comment on attachment 131186 [details]
Patch

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

> Source/WebCore/notifications/Notification.h:77
> +    KURL iconURL() { return m_icon; }

Please add const modifier.

> Source/WebCore/notifications/Notification.h:79
> +    void setTitle(const String& title) { m_title = title; }

It seems that none is accessing setTitle and setBody now. Will they be used in the future patch?
Comment 12 Jon Lee 2012-03-15 11:42:07 PDT
(In reply to comment #11)
> (From update of attachment 131186 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=131186&action=review
> 
> > Source/WebCore/notifications/Notification.h:77
> > +    KURL iconURL() { return m_icon; }
> 
> Please add const modifier.
Sure.

> 
> > Source/WebCore/notifications/Notification.h:79
> > +    void setTitle(const String& title) { m_title = title; }
> 
> It seems that none is accessing setTitle and setBody now. Will they be used in the future patch?
Not at this point. I can remove.
Comment 13 Jon Lee 2012-03-15 16:07:45 PDT
Committed r110903: <http://trac.webkit.org/changeset/110903>