Merge lp:~mikemc/ubuntuone-control-panel/launch-separate-darwin-menu into lp:ubuntuone-control-panel

Proposed by Mike McCracken
Status: Merged
Approved by: Mike McCracken
Approved revision: 383
Merged at revision: 385
Proposed branch: lp:~mikemc/ubuntuone-control-panel/launch-separate-darwin-menu
Merge into: lp:ubuntuone-control-panel
Diff against target: 420 lines (+226/-26)
6 files modified
ubuntuone/controlpanel/backend.py (+5/-0)
ubuntuone/controlpanel/gui/qt/main/__init__.py (+22/-6)
ubuntuone/controlpanel/gui/qt/main/darwin.py (+52/-8)
ubuntuone/controlpanel/gui/qt/main/tests/test_darwin.py (+104/-5)
ubuntuone/controlpanel/gui/qt/main/tests/test_main.py (+17/-7)
ubuntuone/controlpanel/tests/test_backend.py (+26/-0)
To merge this branch: bzr merge lp:~mikemc/ubuntuone-control-panel/launch-separate-darwin-menu
Reviewer Review Type Date Requested Status
Diego Sarmentero (community) Approve
dobey (community) Approve
Review via email: mp+135268@code.launchpad.net

Commit message

- Add support for menu in separate process on darwin. (LP: #1045939)

Description of the change

- Add support for menu in separate process on darwin. (LP: #1045939)

- cmd-q handler in control panel no longer kills syncdaemon
- control panel waits until it gets first syncdaemon status update then launches menu to avoid IPC race in syncdaemontool.

Tests pass on darwin and linux P+Q.

To test IRL on darwin, you need lp:ubuntuone-cocoa-menu, and then you *MUST* package the entire app.
The ubuntuone-cocoa-menu code is not set up to run from source.
PyObjC works from source in general, but the Cocoa UI loading code doesn't handle that case.

To post a comment you must log in.
Revision history for this message
dobey (dobey) :
review: Approve
383. By Mike McCracken on 2012-11-30

add docstring for dummy class

Revision history for this message
Diego Sarmentero (diegosarmentero) wrote :

+1
"Let's put a smile on that face!" [The Dark Knight]

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'ubuntuone/controlpanel/backend.py'
--- ubuntuone/controlpanel/backend.py 2012-11-29 20:15:08 +0000
+++ ubuntuone/controlpanel/backend.py 2012-11-30 18:08:22 +0000
@@ -226,6 +226,11 @@
226 self.sd_client.set_status_changed_handler(cb)226 self.sd_client.set_status_changed_handler(cb)
227 self.is_sd_client_handler_set = True227 self.is_sd_client_handler_set = True
228228
229 def remove_status_changed_handler(self, handler):
230 """Remove 'handler' from callbacks list."""
231 if handler in self._status_changed_handlers:
232 self._status_changed_handlers.remove(handler)
233
229 @inlineCallbacks234 @inlineCallbacks
230 def _process_device_web_info(self, devices,235 def _process_device_web_info(self, devices,
231 enabled=None, limit_bw=None,236 enabled=None, limit_bw=None,
232237
=== modified file 'ubuntuone/controlpanel/gui/qt/main/__init__.py'
--- ubuntuone/controlpanel/gui/qt/main/__init__.py 2012-10-23 19:26:30 +0000
+++ ubuntuone/controlpanel/gui/qt/main/__init__.py 2012-11-30 18:08:22 +0000
@@ -40,10 +40,14 @@
4040
41if sys.platform == 'darwin':41if sys.platform == 'darwin':
42 from ubuntuone.controlpanel.gui.qt.main.darwin import (42 from ubuntuone.controlpanel.gui.qt.main.darwin import (
43 install_platform_event_handlers)43 install_platform_event_handlers, MenubarIconLauncher)
44 assert(install_platform_event_handlers)44 assert(install_platform_event_handlers)
45 assert(MenubarIconLauncher)
45else:46else:
46 install_platform_event_handlers = lambda app: None47 class MenubarIconLauncher:
48 """Dummy. Separate menu launcher not used on win/linux."""
49 install_platform_event_handlers = lambda app, **kwargs: None
50
47# pylint: enable=C010351# pylint: enable=C0103
4852
49from ubuntuone.controlpanel.utils import install_config_and_daemons53from ubuntuone.controlpanel.utils import install_config_and_daemons
@@ -118,9 +122,7 @@
118 args = parser.parse_args(args=arg_list[bin_position:])122 args = parser.parse_args(args=arg_list[bin_position:])
119 switch_to = args.switch_to123 switch_to = args.switch_to
120 minimized = args.minimized124 minimized = args.minimized
121 # On Darwin, because of how apps are started, we need125 with_icon = args.with_icon
122 # this by default.
123 with_icon = args.with_icon or sys.platform == 'darwin'
124 installer = args.installer126 installer = args.installer
125 source.main(app)127 source.main(app)
126128
@@ -135,9 +137,17 @@
135137
136 install_config_and_daemons()138 install_config_and_daemons()
137139
140 if sys.platform == 'darwin':
141 with_icon = False
142 # cmd-q in CP shouldn't kill SD or the menu:
143 quit_kills_sd = False
144 else:
145 quit_kills_sd = not with_icon
146
138 # need to keep a reference to the menu or our handler will be147 # need to keep a reference to the menu or our handler will be
139 # removed148 # removed
140 menubar = install_platform_event_handlers(app)149 menubar = install_platform_event_handlers(app,
150 quit_kills_sd=quit_kills_sd)
141 menubar151 menubar
142152
143 # Unused variable 'window', 'icon', pylint: disable=W0612153 # Unused variable 'window', 'icon', pylint: disable=W0612
@@ -151,4 +161,10 @@
151 if icon:161 if icon:
152 app.new_instance.connect(icon.restore_window)162 app.new_instance.connect(icon.restore_window)
153163
164 if sys.platform == 'darwin':
165 # start separate menu app instead of Qt systray menu, waiting
166 # until it is safe to do so without IPC races:
167 menu_launcher = MenubarIconLauncher()
168 assert(menu_launcher)
169
154 source.main_start(app)170 source.main_start(app)
155171
=== modified file 'ubuntuone/controlpanel/gui/qt/main/darwin.py'
--- ubuntuone/controlpanel/gui/qt/main/darwin.py 2012-10-04 20:34:22 +0000
+++ ubuntuone/controlpanel/gui/qt/main/darwin.py 2012-11-30 18:08:22 +0000
@@ -16,28 +16,40 @@
1616
17"""Darwin-specific GUI event handling code."""17"""Darwin-specific GUI event handling code."""
1818
19import sys
20import subprocess
21
19from PyQt4 import QtGui22from PyQt4 import QtGui
20from twisted.internet.defer import inlineCallbacks23from twisted.internet.defer import inlineCallbacks
2124
25from dirspec.utils import get_program_path
26
22from ubuntuone.platform.tools import SyncDaemonTool27from ubuntuone.platform.tools import SyncDaemonTool
2328
29from ubuntuone.controlpanel import cache
30from ubuntuone.controlpanel.backend import (STATUS_KEY, FILE_SYNC_ERROR)
24from ubuntuone.controlpanel.gui.qt.main import twisted_main31from ubuntuone.controlpanel.gui.qt.main import twisted_main
32from ubuntuone.controlpanel.logger import setup_logging
33
34
35logger = setup_logging('qt.main.darwin')
2536
2637
27@inlineCallbacks38@inlineCallbacks
28def handle_cmd_q(app, event):39def handle_cmd_q(app, event, should_quit_sd=False):
29 """Handle the quit event on darwin."""40 """Handle the quit event on darwin."""
30 # pylint: disable=W070241 # pylint: disable=W0702
31 try:42 if should_quit_sd:
32 st = SyncDaemonTool()43 try:
33 yield st.quit()44 st = SyncDaemonTool()
34 except:45 yield st.quit()
35 pass46 except:
47 pass
36 twisted_main.main_quit(app)48 twisted_main.main_quit(app)
37 app.aboutToQuit.emit()49 app.aboutToQuit.emit()
3850
3951
40def install_platform_event_handlers(app):52def install_platform_event_handlers(app, quit_kills_sd=False):
41 """Add code to catch cmd-Q."""53 """Add code to catch cmd-Q."""
42 # on darwin, a menu item with 'quit' in its title is moved to54 # on darwin, a menu item with 'quit' in its title is moved to
43 # the application menu and given the cmd-Q shortcut by Qt. If55 # the application menu and given the cmd-Q shortcut by Qt. If
@@ -52,9 +64,41 @@
5264
53 quit_action = QtGui.QAction("quit", menubar,65 quit_action = QtGui.QAction("quit", menubar,
54 triggered=lambda x:66 triggered=lambda x:
55 handle_cmd_q(app, x))67 handle_cmd_q(app,
68 x,
69 quit_kills_sd))
56 quit_action.setObjectName("darwin-cmd-q")70 quit_action.setObjectName("darwin-cmd-q")
57 menu = menubar.addMenu("This string is not used.")71 menu = menubar.addMenu("This string is not used.")
58 menu.addAction(quit_action)72 menu.addAction(quit_action)
5973
60 return menubar74 return menubar
75
76
77class MenubarIconLauncher(cache.Cache):
78 """Listens to status, launches the menubar app once it is safe to do so."""
79
80 def __init__(self):
81 """Register as listener."""
82 super(MenubarIconLauncher, self).__init__()
83 self.backend.add_status_changed_handler(self.handle_status_update)
84
85 def handle_status_update(self, result):
86 """Process updates, launch menu on non-error"""
87
88 if result[STATUS_KEY] == FILE_SYNC_ERROR:
89 logger.warning("not starting menu, file sync in error state")
90 return
91
92 self.start_menu_process()
93 self.backend.remove_status_changed_handler(self.handle_status_update)
94
95 def start_menu_process(self):
96 """Find and launch separate menu process."""
97 if getattr(sys, 'frozen', None) is None:
98 logger.warning("Can not launch pyobjc menu from source, ignoring.")
99 return
100 menu_app_name = 'Ubuntu One Menu'
101 path = get_program_path(menu_app_name,
102 app_names={menu_app_name:
103 'Ubuntu One Menu.app'})
104 subprocess.Popen(path)
61105
=== modified file 'ubuntuone/controlpanel/gui/qt/main/tests/test_darwin.py'
--- ubuntuone/controlpanel/gui/qt/main/tests/test_darwin.py 2012-10-04 20:34:22 +0000
+++ ubuntuone/controlpanel/gui/qt/main/tests/test_darwin.py 2012-11-30 18:08:22 +0000
@@ -18,8 +18,12 @@
1818
19from PyQt4 import QtGui19from PyQt4 import QtGui
2020
21import subprocess
22import sys
23
21from twisted.internet.defer import (inlineCallbacks, succeed)24from twisted.internet.defer import (inlineCallbacks, succeed)
2225
26from ubuntuone.controlpanel import backend
23from ubuntuone.controlpanel.gui.qt import main27from ubuntuone.controlpanel.gui.qt import main
24from ubuntuone.controlpanel.tests import TestCase28from ubuntuone.controlpanel.tests import TestCase
25from ubuntuone.controlpanel.gui.tests import FakeSignal29from ubuntuone.controlpanel.gui.tests import FakeSignal
@@ -60,6 +64,28 @@
60 raise Exception("exception")64 raise Exception("exception")
6165
6266
67class FakeBackend(object):
68 """Mock backend"""
69
70 def __init__(self):
71 """init"""
72
73 super(FakeBackend, self).__init__()
74 self.handlers = []
75
76 def add_status_changed_handler(self, h):
77 """add handler"""
78 self.handlers.append(h)
79
80 def remove_status_changed_handler(self, h):
81 """remove handler """
82 self.handlers.remove(h)
83
84 def clear(self):
85 """reset handlers"""
86 self.handlers = []
87
88
63class QuitTestCase(TestCase):89class QuitTestCase(TestCase):
64 """Test quit event handling."""90 """Test quit event handling."""
6591
@@ -73,13 +99,22 @@
7399
74 @inlineCallbacks100 @inlineCallbacks
75 def test_cmd_q_func_quits_sd(self):101 def test_cmd_q_func_quits_sd(self):
76 """Test that we call syncdaemontool.quit """102 """Test that we call syncdaemontool.quit when asked"""
77 self.patch(main.darwin, 'SyncDaemonTool', FakeSDTool)103 self.patch(main.darwin, 'SyncDaemonTool', FakeSDTool)
78104
79 yield main.darwin.handle_cmd_q(self.fake_app, None)105 yield main.darwin.handle_cmd_q(self.fake_app, None,
106 should_quit_sd=True)
80 self.assertEqual(FakeSDTool.called, True)107 self.assertEqual(FakeSDTool.called, True)
81108
82 @inlineCallbacks109 @inlineCallbacks
110 def test_cmd_q_func_doesnt_quit_sd(self):
111 """Test that we don't call syncdaemontool.quit by default"""
112 self.patch(main.darwin, 'SyncDaemonTool', FakeSDTool)
113
114 yield main.darwin.handle_cmd_q(self.fake_app, None)
115 self.assertEqual(FakeSDTool.called, False)
116
117 @inlineCallbacks
83 def test_cmd_q_func_ignores_exception(self):118 def test_cmd_q_func_ignores_exception(self):
84 """Test that we keep going when SDT raises."""119 """Test that we keep going when SDT raises."""
85 self.patch(main.darwin, 'SyncDaemonTool', FakeSDToolRaising)120 self.patch(main.darwin, 'SyncDaemonTool', FakeSDToolRaising)
@@ -93,12 +128,76 @@
93class InstallEventHandlersTestCase(TestCase):128class InstallEventHandlersTestCase(TestCase):
94 """Test creating Qt menu items."""129 """Test creating Qt menu items."""
95130
96 def test_install_handlers_creates_action(self):131 def test_install_handlers_creates_kill_action(self):
97 """Test menu items created"""132 """Test creating menu item that will kill sd"""
133 self.patch(main.darwin, 'handle_cmd_q',
134 self._set_called)
135 app = QtGui.QApplication.instance()
136 k = dict(quit_kills_sd=True)
137 menubar = main.darwin.install_platform_event_handlers(app, **k)
138 quit_action = menubar.findChild(QtGui.QAction, "darwin-cmd-q")
139 quit_action.trigger()
140 self.assertEqual(self._called, ((app, False, True), {}))
141
142 def test_install_handlers_creates_nonkill_action(self):
143 """Test creating menu item that will not kill sd"""
98 self.patch(main.darwin, 'handle_cmd_q',144 self.patch(main.darwin, 'handle_cmd_q',
99 self._set_called)145 self._set_called)
100 app = QtGui.QApplication.instance()146 app = QtGui.QApplication.instance()
101 menubar = main.darwin.install_platform_event_handlers(app)147 menubar = main.darwin.install_platform_event_handlers(app)
102 quit_action = menubar.findChild(QtGui.QAction, "darwin-cmd-q")148 quit_action = menubar.findChild(QtGui.QAction, "darwin-cmd-q")
103 quit_action.trigger()149 quit_action.trigger()
104 self.assertEqual(self._called, ((app, False), {}))150 self.assertEqual(self._called, ((app, False, False), {}))
151
152
153class LauncherTestCase(TestCase):
154 """Test pyobjc menu launcher."""
155
156 @inlineCallbacks
157 def setUp(self):
158 """Set up launcher and patches"""
159 yield super(LauncherTestCase, self).setUp()
160
161 self.patch(backend, 'ControlBackend', FakeBackend)
162 self.patch(main.darwin, 'get_program_path',
163 lambda a, **kw: 'testpath')
164 self.addCleanup(lambda: self.launcher.backend.clear())
165
166 self.launcher = main.darwin.MenubarIconLauncher()
167
168 def test_add_on_init(self):
169 """Test adding to handler."""
170 self.assertEqual(self.launcher.backend.handlers,
171 [self.launcher.handle_status_update])
172
173 def test_handle_status_update_error(self):
174 """Test doing nothing on error in handle_status_update"""
175 self.patch(self.launcher, 'start_menu_process', self._set_called)
176 self.launcher.handle_status_update({backend.STATUS_KEY:
177 backend.FILE_SYNC_ERROR})
178 self.assertEqual(self._called, False)
179
180 def test_handle_status_update(self):
181 """Test calling start_menu_process and removing handler"""
182 self.patch(self.launcher, 'start_menu_process', self._set_called)
183 self.launcher.handle_status_update({backend.STATUS_KEY: 'copacetic'})
184 self.assertEqual(self._called, ((), {}))
185 self.assertEqual(self.launcher.backend.handlers, [])
186
187 def test_start_menu_process_nonfrozen(self):
188 """Start does nothing when not frozen."""
189 sys.frozen = None
190 self.addCleanup(delattr, sys, 'frozen')
191
192 self.patch(subprocess, 'Popen', self._set_called)
193 self.launcher.start_menu_process()
194 self.assertEqual(self._called, False)
195
196 def test_start_menu_process_frozen(self):
197 """Start launches menu when frozen."""
198 sys.frozen = 'macosx'
199 self.addCleanup(delattr, sys, 'frozen')
200
201 self.patch(subprocess, 'Popen', self._set_called)
202 self.launcher.start_menu_process()
203 self.assertEqual(self._called, (('testpath',), {}))
105204
=== modified file 'ubuntuone/controlpanel/gui/qt/main/tests/test_main.py'
--- ubuntuone/controlpanel/gui/qt/main/tests/test_main.py 2012-10-31 19:32:18 +0000
+++ ubuntuone/controlpanel/gui/qt/main/tests/test_main.py 2012-11-30 18:08:22 +0000
@@ -137,7 +137,7 @@
137 self.patch(main.source, "main_start", lambda app: None)137 self.patch(main.source, "main_start", lambda app: None)
138 self.patch(QtCore, "QTranslator", lambda: self.translator)138 self.patch(QtCore, "QTranslator", lambda: self.translator)
139 self.patch(main, 'install_platform_event_handlers',139 self.patch(main, 'install_platform_event_handlers',
140 lambda app: None)140 lambda app, **kwargs: None)
141141
142 self.qt4reactor_installed = False142 self.qt4reactor_installed = False
143143
@@ -194,11 +194,11 @@
194 {'minimized': True, 'with_icon': False, 'installer': False})194 {'minimized': True, 'with_icon': False, 'installer': False})
195195
196 def test_darwin_defult_is_with_icon(self):196 def test_darwin_defult_is_with_icon(self):
197 """On Darwin, the default is to show the icon."""197 """On Darwin, the default is not to show the icon."""
198 self.patch(sys, 'platform', 'darwin')198 self.patch(sys, 'platform', 'darwin')
199 main.main([sys.argv[0]])199 main.main([sys.argv[0]])
200 self.assertEqual(self.start.args[1],200 self.assertEqual(self.start.args[1],
201 {'minimized': False, 'with_icon': True, 'installer': False})201 {'minimized': False, 'with_icon': False, 'installer': False})
202202
203 def test_not_darwin_defult_is_with_icon(self):203 def test_not_darwin_defult_is_with_icon(self):
204 """On Not-darwin, the default is to not show the icon."""204 """On Not-darwin, the default is to not show the icon."""
@@ -215,7 +215,7 @@
215 {'minimized': True, 'with_icon': False, 'installer': False})215 {'minimized': True, 'with_icon': False, 'installer': False})
216216
217 def test_with_icon_option(self):217 def test_with_icon_option(self):
218 """Ensure the --minimized option is parsed and passed correctly."""218 """Ensure the --with-icon option is parsed and passed correctly."""
219 self.patch(sys, 'platform', 'not-darwin')219 self.patch(sys, 'platform', 'not-darwin')
220 main.main([sys.argv[0], "--with-icon"])220 main.main([sys.argv[0], "--with-icon"])
221 self.assertEqual(self.start.args[1],221 self.assertEqual(self.start.args[1],
@@ -308,10 +308,20 @@
308 main.main([sys.argv[0]], install_reactor_darwin=False)308 main.main([sys.argv[0]], install_reactor_darwin=False)
309 self.assertEqual(self._called, ((), {}))309 self.assertEqual(self._called, ((), {}))
310310
311 def test_install_event_handlers(self):311 def test_install_event_handlers_darwin(self):
312 """Test that install_platform_event_handlers is called."""312 """Test that install_platform_event_handlers is called on darwin."""
313 self.patch(sys, 'platform', 'darwin')
313 self.patch(main, 'install_platform_event_handlers',314 self.patch(main, 'install_platform_event_handlers',
314 self._set_called)315 self._set_called)
315316
316 main.main([sys.argv[0]], install_reactor_darwin=False)317 main.main([sys.argv[0]], install_reactor_darwin=False)
317 self.assertEqual(self._called, ((self.app,), {}))318 self.assertEqual(self._called, ((self.app,), {'quit_kills_sd': False}))
319
320 def test_install_event_handlers_non_darwin(self):
321 """Test install_platform_event_handlers is called on not-darwin."""
322 self.patch(sys, 'platform', 'not-darwin')
323 self.patch(main, 'install_platform_event_handlers',
324 self._set_called)
325
326 main.main([sys.argv[0], '--with-icon'], install_reactor_darwin=False)
327 self.assertEqual(self._called, ((self.app,), {'quit_kills_sd': False}))
318328
=== modified file 'ubuntuone/controlpanel/tests/test_backend.py'
--- ubuntuone/controlpanel/tests/test_backend.py 2012-11-29 20:15:08 +0000
+++ ubuntuone/controlpanel/tests/test_backend.py 2012-11-30 18:08:22 +0000
@@ -1485,6 +1485,32 @@
1485 self.assertEqual(self.call_count_b, 1)1485 self.assertEqual(self.call_count_b, 1)
1486 self.assertEqual(self.call_count_a, 1)1486 self.assertEqual(self.call_count_a, 1)
14871487
1488 def test_remove_status_handler(self):
1489 """Test removing a handler."""
1490 self.call_count_a = 0
1491 self.call_count_b = 0
1492
1493 def inc_a(status):
1494 """Fake status handler #1"""
1495 self.call_count_a += 1
1496
1497 def inc_b(status):
1498 """Fake status handler #2"""
1499 self.call_count_b += 1
1500
1501 self.addCleanup(delattr, self, "call_count_a")
1502 self.addCleanup(delattr, self, "call_count_b")
1503
1504 self.be.add_status_changed_handler(inc_a)
1505 self.be.add_status_changed_handler(inc_b)
1506
1507 self.be.sd_client.status_changed_handler({})
1508 self.be.remove_status_changed_handler(inc_b)
1509 self.be.sd_client.status_changed_handler({})
1510
1511 self.assertEqual(self.call_count_b, 1)
1512 self.assertEqual(self.call_count_a, 2)
1513
14881514
1489class BackendSyncStatusIfDisabledTestCase(BackendSyncStatusTestCase):1515class BackendSyncStatusIfDisabledTestCase(BackendSyncStatusTestCase):
1490 """Syncdaemon state for the backend when file sync is disabled."""1516 """Syncdaemon state for the backend when file sync is disabled."""

Subscribers

People subscribed via source and target branches