[Webkit-unassigned] [Bug 116049] Add a DisplayNotificationObserver class to handle Display notifications

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=116049





--- Comment #38 from Tim Horton <timothy_horton at apple.com>  2013-05-15 18:45:49 PST ---
(From update of attachment 201907)
View in context: https://bugs.webkit.org/attachment.cgi?id=201907&action=review

> Source/WebCore/platform/DisplayNotificationObserver.h:49
> +    OwnPtr<struct DisplayNotificationObserverInternals> m_internals;

Shouldn't you typedef the struct so that you can just use it as a naked type instead of with the struct prefix everywhere?

> Source/WebCore/platform/mac/DisplayNotificationObserver.mm:61
> +        IOObjectRelease(m_internals->m_displayWrangler);
> +        IOObjectRelease(m_internals->m_displayReference);
> +        IONotificationPortDestroy(m_internals->m_displayPort);

I think you have to test each of these for null before releasing since you seem to think any of them could be null in platformInit().

> Source/WebCore/platform/mac/DisplayNotificationObserver.mm:69
> +    m_internals->m_displayWrangler = IOServiceGetMatchingService(NULL, IOServiceNameMatching("IODisplayWrangler"));

0 instead of null blah blah blah stylebot (see style guidelines even if stylebot is being dumb)

> Source/WebCore/platform/mac/DisplayNotificationObserver.mm:73
> +    m_internals->m_displayPort = IONotificationPortCreate(NULL);

here too

> Source/WebCore/platform/mac/DisplayNotificationObserver.mm:80
> +    m_internals->m_dispatchQueue = dispatch_queue_create("com.apple.WebKit.DisplayNotificationObserver", 0);
> +    IONotificationPortSetDispatchQueue(m_internals->m_displayPort, m_internals->m_dispatchQueue);

Do you have to make your own dispatch queue? Seems like you could just use one of the shared queues, maybe? Anders would know.

> Source/WebCore/platform/mac/DisplayNotificationObserver.mm:87
> +DisplayNotificationObserver* DisplayNotificationObserver::sharedDisplayObserver()
> +{
> +    DEFINE_STATIC_LOCAL(DisplayNotificationObserver, observer, ());
> +    return &observer;
> +}

Why does this need to be in the platform specific file?

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