[Webkit-unassigned] [Bug 116049] Add a DisplayAndPowerController class to handle IOKit Display and Power notifications
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed May 15 10:49:11 PDT 2013
https://bugs.webkit.org/show_bug.cgi?id=116049
Simon Fraser (smfr) <simon.fraser at apple.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #201808|review? |review-
Flag| |
--- Comment #33 from Simon Fraser (smfr) <simon.fraser at apple.com> 2013-05-15 10:47:37 PST ---
(From update of attachment 201808)
View in context: https://bugs.webkit.org/attachment.cgi?id=201808&action=review
> Source/WebCore/platform/DisplayNotificationObserver.cpp:35
> +DisplayNotificationObserver::DisplayNotificationObserver() { }
> +
> +DisplayNotificationObserver::~DisplayNotificationObserver() { }
> +
> +void DisplayNotificationObserver::platformInit() { }
Please put the braces on their own lines; we only do this style in headers.
> Source/WebCore/platform/DisplayNotificationObserver.h:31
> +#include <wtf/OwnPtr.h>
> +#include <wtf/PassOwnPtr.h>
You're not using these two headers.
> Source/WebCore/platform/DisplayNotificationObserver.h:43
> + bool s_screenDimmed;
Why s_screenDimmed? This is not static, so it should be m_screenDimmed.
> Source/WebCore/platform/mac/DisplayNotificationObserver.mm:71
> + m_internals = adoptPtr(new DisplayNotificationObserverInternals());
> +
> + m_internals->m_displayWrangler = IOServiceGetMatchingService(NULL, IOServiceNameMatching("IODisplayWrangler"));
> + if (!m_internals->m_displayWrangler)
> + return;
> +
> + m_internals->m_displayPort = IONotificationPortCreate(NULL);
> + if (!m_internals->m_displayPort)
> + return;
> +
> + IOServiceAddInterestNotification(m_internals->m_displayPort, m_internals->m_displayWrangler, kIOGeneralInterest, didReceiveDisplayStateNotification, NULL, &m_internals->m_displayReference);
> +
> + m_internals->m_dispatchQueue = dispatch_queue_create("com.apple.WebKit.DisplayAndPowerObserver", 0);
> + IONotificationPortSetDispatchQueue(m_internals->m_displayPort, m_internals->m_dispatchQueue);
> +}
Why doesn't this set up the power-related stuff?
> Source/WebCore/platform/mac/DisplayNotificationObserver.mm:77
> +DisplayNotificationObserver* DisplayNotificationObserver::createSharedDisplayObserver()
> +{
> + DEFINE_STATIC_LOCAL(DisplayNotificationObserver, observer, ());
> + return &observer;
> +}
This only does the create the first time, so I think it should just be sharedDisplayObserver()
> Source/WebCore/platform/mac/DisplayNotificationObserver.mm:90
> + if (!m_internals->m_powerConnection)
> + return;
You should never early return from a destructor.
> Source/WebCore/platform/mac/DisplayNotificationObserver.mm:106
> + createSharedDisplayObserver()->didReceiveDisplayStateNotification(messageType, messageArgument);
Is the first parameter a 'context' param? Normally we'd use that to get to the target object, which would be cleaner than going through createSharedDisplayObserver().
--
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