Last Comment Bug 714694 - Add IdleService to gonk backend
: Add IdleService to gonk backend
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget (show other bugs)
: unspecified
: Other Gonk (Firefox OS)
: -- normal (vote)
: mozilla12
Assigned To: Kan-Ru Chen [:kanru] (UTC+8)
:
Mentors:
Depends on:
Blocks: 697132
  Show dependency treegraph
 
Reported: 2012-01-02 17:43 PST by Kan-Ru Chen [:kanru] (UTC+8)
Modified: 2012-02-01 13:58 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part 1, Add IdleService to gonk backend (2.60 KB, patch)
2012-01-02 17:58 PST, Kan-Ru Chen [:kanru] (UTC+8)
no flags Details | Diff | Review
Part 2, Track user activity in gonk backend (3.01 KB, patch)
2012-01-02 17:58 PST, Kan-Ru Chen [:kanru] (UTC+8)
no flags Details | Diff | Review
Part 1, Add IdleService to gonk backend, v2 (7.34 KB, patch)
2012-01-03 23:20 PST, Kan-Ru Chen [:kanru] (UTC+8)
mwu.code: review+
Details | Diff | Review
Part 2, Track user activity in gonk backend, v2 (2.53 KB, patch)
2012-01-05 01:24 PST, Kan-Ru Chen [:kanru] (UTC+8)
mwu.code: review+
Details | Diff | Review
Part 2, Track user activity in gonk backend (2.54 KB, patch)
2012-01-05 18:14 PST, Kan-Ru Chen [:kanru] (UTC+8)
mwu.code: review+
Details | Diff | Review
Part 1, Add IdleService to gonk backend, v2 (7.27 KB, patch)
2012-01-10 17:07 PST, Kan-Ru Chen [:kanru] (UTC+8)
no flags Details | Diff | Review
Part 2, Track user activity in gonk backend (2.51 KB, patch)
2012-01-10 17:07 PST, Kan-Ru Chen [:kanru] (UTC+8)
no flags Details | Diff | Review

Description Kan-Ru Chen [:kanru] (UTC+8) 2012-01-02 17:43:16 PST
Add IdleService to gonk backend and track user activity.
Comment 1 Kan-Ru Chen [:kanru] (UTC+8) 2012-01-02 17:58:21 PST
Created attachment 585343 [details] [diff] [review]
Part 1, Add IdleService to gonk backend
Comment 2 Kan-Ru Chen [:kanru] (UTC+8) 2012-01-02 17:58:35 PST
Created attachment 585344 [details] [diff] [review]
Part 2, Track user activity in gonk backend
Comment 3 Kan-Ru Chen [:kanru] (UTC+8) 2012-01-03 23:20:13 PST
Created attachment 585660 [details] [diff] [review]
Part 1, Add IdleService to gonk backend, v2

--
Attachment #585343 [details] [diff] - Attachment is obsolete: true
Comment 4 Michael Wu [:mwu] 2012-01-04 15:45:39 PST
Comment on attachment 585344 [details] [diff] [review]
Part 2, Track user activity in gonk backend

Review of attachment 585344 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/src/gonk/nsWindow.cpp
@@ +68,5 @@
>  static nsWindow *gWindowToRedraw = nsnull;
>  static nsWindow *gFocusedWindow = nsnull;
>  static android::FramebufferNativeWindow *gNativeWindow = nsnull;
>  static bool sFramebufferOpen;
> +static nsCOMPtr<nsIdleService> gIdleService;

nsCOMPtrs generally shouldn't be used in a global since they'll hold objects past xpcom shutdown, and those objects end up being counted as leaked. There's a way to fix that, but instead I suggest just putting this variable in nsWindow and calling UserActivity() on the focused window.
Comment 5 Kan-Ru Chen [:kanru] (UTC+8) 2012-01-05 01:24:08 PST
Created attachment 586004 [details] [diff] [review]
Part 2, Track user activity in gonk backend, v2

--
Attachment #585344 [details] [diff] - Attachment is obsolete: true
Comment 6 Michael Wu [:mwu] 2012-01-05 10:54:23 PST
Comment on attachment 586004 [details] [diff] [review]
Part 2, Track user activity in gonk backend, v2

Review of attachment 586004 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. r=me with indenting nit fixed.

::: widget/src/gonk/nsWindow.cpp
@@ +433,5 @@
>      (*mEventCallback)(&event);
>  }
>  
> +void
> +nsWindow::UserActivity()

This function should use four space indent to be consistent with the rest of the file.
Comment 7 Kan-Ru Chen [:kanru] (UTC+8) 2012-01-05 18:14:38 PST
Created attachment 586302 [details] [diff] [review]
Part 2, Track user activity in gonk backend

--
Attachment #586004 [details] [diff] - Attachment is obsolete: true
Comment 8 Ed Morley [:emorley] 2012-01-10 15:48:44 PST
applying thg-import-yu21aq.patch
unable to find 'widget/src/gonk/Makefile.in' for patching
1 out of 1 hunks FAILED -- saving rejects to file widget/src/gonk/Makefile.in.rej
unable to find 'widget/src/gonk/nsWidgetFactory.cpp' for patching
1 out of 1 hunks FAILED -- saving rejects to file widget/src/gonk/nsWidgetFactory.cpp.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
Comment 9 Kan-Ru Chen [:kanru] (UTC+8) 2012-01-10 17:07:08 PST
Created attachment 587546 [details] [diff] [review]
Part 1, Add IdleService to gonk backend, v2

--
Attachment #585660 [details] [diff] - Attachment is obsolete: true
Comment 10 Kan-Ru Chen [:kanru] (UTC+8) 2012-01-10 17:07:22 PST
Created attachment 587547 [details] [diff] [review]
Part 2, Track user activity in gonk backend

--
Attachment #586302 [details] [diff] - Attachment is obsolete: true
Comment 11 Michael Wu [:mwu] 2012-01-10 17:30:18 PST
You don't need to request review again if you've only done minor things to mske your patch apply again, or if you are addressing minor nits in review comments. Some people carry review over from older patches to reflect this, though I consider this optional.
Comment 12 Kan-Ru Chen [:kanru] (UTC+8) 2012-01-10 17:35:21 PST
(In reply to Michael Wu [:mwu] from comment #11)
> You don't need to request review again if you've only done minor things to
> mske your patch apply again, or if you are addressing minor nits in review
> comments. Some people carry review over from older patches to reflect this,
> though I consider this optional.

I see :)
Comment 13 Ed Morley [:emorley] 2012-01-10 18:13:15 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c5ecb76b94c
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e41b25f9a59

(^ Second landing, first I backed out since I didn't spot the patch author was missing, as I was mistakenly convinced I'd checked everything before the patch was updated for bitrot)

To save time for future landings, could you set your hgrc to include the author automatically & also add a commit message that includes the bug number (and if reuploading after review for bitrot, the r=foo too), along the lines of:
https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Thanks :-)

Note You need to log in before you can comment on or make changes to this bug.