Closed
Bug 707589
Opened 13 years ago
Closed 12 years ago
[Gonk] When screen is enabled/disabled, send visibility change events to windows
Categories
(Core :: General, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: justin.lebar+bug, Assigned: kanru)
References
Details
Attachments
(1 file, 2 obsolete files)
6.37 KB,
patch
|
mwu
:
review+
|
Details | Diff | Splinter Review |
When you set screen.mozEnabled, we need to inform the windows that their visibility has changed.
Updated•13 years ago
|
OS: Gonk → All
Hardware: x86 → All
Version: unspecified → Trunk
Updated•13 years ago
|
OS: All → Gonk
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → kchen
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #610845 -
Flags: review?(mwu)
Comment 2•12 years ago
|
||
Comment on attachment 610845 [details] [diff] [review] Send "sizemodechange" event when the screen is turned on/off. Review of attachment 610845 [details] [diff] [review]: ----------------------------------------------------------------- Looks good overall, but I would like to see the changes in nsAppShell.cpp move to nsWindow.cpp. It should simplify things. ::: widget/gonk/nsAppShell.cpp @@ +142,5 @@ > + int fd; > + > + // Cannot use epoll here because kSleepFile and kWakeFile are > + // always ready to read and blocking. > + for (;;) { Generally, I would prefer having code dispatch itself to the thread at the end instead of an infinite loop like this. @@ +143,5 @@ > + > + // Cannot use epoll here because kSleepFile and kWakeFile are > + // always ready to read and blocking. > + for (;;) { > + fd = open(kSleepFile, O_RDONLY, 0); I recommend using ScopedClose @@ +158,5 @@ > + close(fd); > + NS_DispatchToMainThread(new ScreenOnOffEvent(true)); > + } > + > + return NULL; NS_IMETHOD methods should return NS_* (nsresult) return codes. In this case, NS_OK.
Attachment #610845 -
Flags: review?(mwu)
Assignee | ||
Comment 3•12 years ago
|
||
Move monitor thread to nsWindow.cpp and replace infinite loop with self dispatching.
Attachment #610845 -
Attachment is obsolete: true
Attachment #611409 -
Flags: review?(mwu)
Comment 4•12 years ago
|
||
Comment on attachment 611409 [details] [diff] [review] Send "sizemodechange" event when the screen is turned on/off. v2 Review of attachment 611409 [details] [diff] [review]: ----------------------------------------------------------------- Looks good but I think we can simplify the code further. ::: widget/gonk/nsWindow.cpp @@ +108,5 @@ > + char buf; > + > + // Cannot use epoll here because kSleepFile and kWakeFile are > + // always ready to read and blocking. > + ScopedClose fd(open(kSleepFile, O_RDONLY, 0)); I recommend putting each of these read sections into their own block so they can close the fd immediately after finishing. @@ +123,5 @@ > + NS_WARN_IF_FALSE(len >= 0, "WAIT_FOR_FB_WAKE failed"); > + NS_DispatchToMainThread(new ScreenOnOffEvent(true)); > + > + // Dispatch to ourself. > + NS_DispatchToCurrentThread(new FramebufferWatcher()); Can you use |this| instead of creating a new object? @@ +225,5 @@ > return gFocusedWindow->mEventCallback(&aEvent); > } > > +/* static */ void > +nsWindow::TurnOnScreen() Let's inline TurnOnScreen/TurnOffScreen into ScreenOnOffEvent::Run. You can use DispatchEvent to fire events on the windows since mEventCallback is protected. @@ +229,5 @@ > +nsWindow::TurnOnScreen() > +{ > + for (unsigned int i = 0; i < sTopWindows.Length(); i++) { > + nsWindow *win = sTopWindows[i]; > + nsSizeModeEvent event(true, NS_SIZEMODE, win); Set event.time. Some of the events fired in this file don't set it unfortunately, but it's a bug.
Attachment #611409 -
Flags: review?(mwu)
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #611409 -
Attachment is obsolete: true
Attachment #612455 -
Flags: review?(mwu)
Comment 6•12 years ago
|
||
Comment on attachment 612455 [details] [diff] [review] Send "sizemodechange" event when the screen is turned on/off. v3 Excellent.
Attachment #612455 -
Flags: review?(mwu) → review+
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 7•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/348e9d3a6bec
Keywords: checkin-needed
Reporter | ||
Comment 8•12 years ago
|
||
Comment on attachment 612455 [details] [diff] [review] Send "sizemodechange" event when the screen is turned on/off. v3 Wait a second...how does FramebufferWatcher shut down?
Assignee | ||
Comment 9•12 years ago
|
||
(In reply to Justin Lebar [:jlebar] from comment #8) > Comment on attachment 612455 [details] [diff] [review] > Send "sizemodechange" event when the screen is turned on/off. v3 > > Wait a second...how does FramebufferWatcher shut down? We could call mFramebufferWatcher->Shutdown() or set a exit flag, but it still requires a screen toggle to break the read(). Could we kill the thread?
Reporter | ||
Comment 10•12 years ago
|
||
(In reply to Kan-Ru Chen [:kanru] from comment #9) > We could call mFramebufferWatcher->Shutdown() or set a exit flag, but it > still requires a screen toggle to break the read(). Could we kill the thread? I don't know. Benjamin?
Assignee | ||
Comment 11•12 years ago
|
||
Maybe I could just close the fd from outside. Will try it later.
Comment 12•12 years ago
|
||
The NSPR functions do not provide any way to kill a thread, because in general it is not safe to do so because that thread may be in a mutex or have some other inconsistent state. You need to find a way to unblock the thread and have it terminate itself.
Comment 13•12 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/348e9d3a6bec
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in
before you can comment on or make changes to this bug.
Description
•