Merge lp:~charlesk/indicator-power/lp-1382861-fix-menu-creation-test into lp:indicator-power/15.10

Proposed by Charles Kerr on 2015-05-14
Status: Merged
Approved by: dobey on 2015-05-15
Approved revision: 285
Merged at revision: 285
Proposed branch: lp:~charlesk/indicator-power/lp-1382861-fix-menu-creation-test
Merge into: lp:indicator-power/15.10
Diff against target: 28 lines (+3/-1)
1 file modified
src/service.c (+3/-1)
To merge this branch: bzr merge lp:~charlesk/indicator-power/lp-1382861-fix-menu-creation-test
Reviewer Review Type Date Requested Status
dobey (community) 2015-05-14 Approve on 2015-05-15
PS Jenkins bot (community) continuous-integration Approve on 2015-05-14
Review via email: mp+259180@code.launchpad.net

Commit message

Fix timing issue that caused "Adjust brightness automatically" menuitem to sometimes not be shown.

Description of the change

== Description of Change

Fix timing issue that caused "Adjust brightness automatically" menuitem to sometimes not be shown.

The rebuild_now() method checked to see if menus were built by testing the session bus pointer for NULL because, in earlier versions, menus were created and exported when the session bus was acquired. Menu creation comes earlier now, so this test can give a false negative.

This patch replaces the NULL pointer test with a simple "menus_created" flag.

== Checklist

Are there any related MPs required for this MP to build/function as expected? Please list.

> Is your branch in sync with latest trunk? (e.g. bzr pull lp:trunk -> no changes)

Yes

> Did the code build without warnings?

Yes

> Did the tests run successfully?

Yes

> Did you perform an exploratory manual test run of your code change and any related functionality?

Yes

> If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

N/A

> Did your component test plan pass? If on a device, what image number?

Yes, krillin r277

> Please list which manual tests are germane for the reviewer in this MR.

This was an infrequent timing bug with no manual tests other than confirming that the "Adjust brightness automatically" checkbox is present on the phone.

> Did you provide a link to this page https://wiki.ubuntu.com/Process/Merges/Checklists/indicator-power

Yes

To post a comment you must log in.
dobey (dobey) wrote :

Looks ok to me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/service.c'
2--- src/service.c 2015-04-02 20:30:59 +0000
3+++ src/service.c 2015-05-14 22:19:37 +0000
4@@ -110,6 +110,7 @@
5 guint actions_export_id;
6 GDBusConnection * conn;
7
8+ gboolean menus_built;
9 struct ProfileMenuInfo menus[N_PROFILES];
10
11 GSimpleActionGroup * actions;
12@@ -674,7 +675,7 @@
13 g_simple_action_set_state (p->header_action, create_header_state (self));
14 }
15
16- if (p->conn == NULL) /* we haven't built the menus yet */
17+ if (!p->menus_built)
18 return;
19
20 if (sections & SECTION_DEVICES)
21@@ -1184,6 +1185,7 @@
22
23 for (i=0; i<N_PROFILES; ++i)
24 create_menu(self, i);
25+ p->menus_built = TRUE;
26
27 g_signal_connect_swapped(p->brightness, "notify::auto-brightness-supported",
28 G_CALLBACK(on_auto_brightness_supported_changed), self);

Subscribers

People subscribed via source and target branches