[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
Mon May 13 19:41:39 PDT 2013
https://bugs.webkit.org/show_bug.cgi?id=116049
--- Comment #2 from Ryosuke Niwa <rniwa at webkit.org> 2013-05-13 19:40:04 PST ---
(From update of attachment 201661)
View in context: https://bugs.webkit.org/attachment.cgi?id=201661&action=review
> Source/WebCore/platform/DisplayAndPowerController.h:39
> +class DisplayAndPowerController {
> + WTF_MAKE_NONCOPYABLE(DisplayAndPowerController);
> +private:
> + DisplayAndPowerController();
> + ~DisplayAndPowerController();
I’d call this PowerSavingController instead.
> Source/WebCore/platform/DisplayAndPowerController.h:47
> + bool s_screenDimmed;
> + void platformInit();
> +#if PLATFORM(MAC)
> + OwnPtr<struct DisplayAndPowerControllerInternals> m_internals;
> + static void didReceiveSystemPowerNotification(void* context, io_service_t, uint32_t messageType, void* messageArgument);
> + static void didReceiveDisplayStateNotification(void* context, io_service_t, uint32_t messageType, void* messageArgument);
> + void didReceiveSystemPowerNotification(uint32_t messageType, void* messageArgument);
> + void didReceiveDisplayStateNotification(uint32_t messageType, void* messageArgument);
Why do these member functions and variables need to be in this class if they were private?
Can’t they just be on DisplayAndPowerControllerInternals instead?
> Source/WebCore/platform/DisplayAndPowerController.h:49
> +#endif
> +public:
Nit: You need a blank line between #endif and public.
> Source/WebCore/platform/DisplayAndPowerController.h:51
> + bool isScreenDimmed() { return s_screenDimmed; }
> + static DisplayAndPowerController* sharedController();
We normally put public member functions first. isScreenDimmed should have const modifier.
> Source/WebCore/platform/mac/SharedTimerMac.mm:85
> + //Makes sure that a DisplayAndPowerController has been created
> + DisplayAndPowerController::sharedController();
I’m not sure it’s such a good idea for a function named sharedController to have a side effect…
Instead of adding a comment above, we should rename it to be more descriptive.
How about something like init() or createSharedController()?
--
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