Bug 115763 – Make caret not blink when entering dimmed display state before going to display sleep

• Status: NEW
• Component: Layout and Rendering

[Roger Fong]

Make the caret not blink on mac when the display goes dim (which happens about 15 seconds before the display goes to sleep).

[Roger Fong]

<rdar://problem/13521485>

[Roger Fong]

Created attachment 200999 [details]
patch

[Simon Fraser (smfr)]

Comment on attachment 200999 [details]
patch

I don't think RenderTheme is the right place for the IOKit code; we might even want it in WebKit/WebKit2. WebCore shouldn't know about screen dimming; it should just be told that the caret should stop blinking.

[Tim Horton]

Comment on attachment 200999 [details]
patch

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

> Source/WebCore/ChangeLog:9
> +        Update RenderTheme to keep track of the receive display sleep notifications and keep track of the display's state.

keep track of the receive?

> Source/WebCore/editing/FrameSelection.cpp:1849
> +    if (m_frame->view()->renderView()->theme()->isScreenDimmed() || (isCaretBlinkingSuspended() && caretPaint))

Can any of these ever be null?

> Source/WebCore/rendering/RenderTheme.h:350
> +    static bool m_screenDimmed;

I think that statics use s instead of m or something but you should look it up. Also maybe this should just be in the RenderThemeMac file and not in this class at all? But I dunno.

> Source/WebCore/rendering/RenderThemeMac.h:262
> +    io_service_t m_displayWrangler;

This is pretty bizarre to be in RenderTheme*.

> Source/WebCore/rendering/RenderThemeMac.mm:2316
> +        case kIOMessageDeviceWillPowerOff:
> +            m_screenDimmed = true;
> +            break;
> +            
> +        case kIOMessageDeviceHasPoweredOn:
> +            m_screenDimmed = false;
> +            break;

This is not indented correctly. Does this really need to be a switch?

[Roger Fong]

I'm going to move PowerObserver code into a class that handles both Display/Power notifications (the both use IOKit) (calling it DisplayAndPowerController until I or someone else comes up with a better name).

This requires me however to also modify the SharedTimerMac.mm (I'm creating a SharedTimerMac.h as well).

One piece that I still haven't figured out is where I'm going to instantiate said DisplayAndPowerController.
Thoughts?

[Roger Fong]

Created attachment 201484 [details]
patch

I may want to split this into two patches.
One that adds the DisplayAndPowerController class (and gets rid of old PowerObserver stuff)
And then another that actually has FrameSelection check the screen state.

[EFL EWS Bot]

Comment on attachment 201484 [details]
patch

Attachment 201484 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/385415

[EFL EWS Bot]

Comment on attachment 201484 [details]
patch

Attachment 201484 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/404368

[Early Warning System Bot]

Comment on attachment 201484 [details]
patch

Attachment 201484 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/404370

[Early Warning System Bot]

Comment on attachment 201484 [details]
patch

Attachment 201484 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/395428

[Build Bot]

Comment on attachment 201484 [details]
patch

Attachment 201484 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/460343

[Build Bot]

Comment on attachment 201484 [details]
patch

Attachment 201484 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/286334

[kov's GTK+ EWS bot]

Comment on attachment 201484 [details]
patch

Attachment 201484 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/417719

[Ryosuke Niwa]

(In reply to comment #6)
> Created an attachment (id=201484) [details]
> patch
> 
> I may want to split this into two patches.
> One that adds the DisplayAndPowerController class (and gets rid of old PowerObserver stuff)
> And then another that actually has FrameSelection check the screen state.

Please do.

[Build Bot]

Comment on attachment 201484 [details]
patch

Attachment 201484 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/417862

[Ryosuke Niwa]

Comment on attachment 201484 [details]
patch

Please split the patch to add DisplayAndPowerController.