Code review comment for lp:~dandrader/qtmir/appRestart-lp1527737

=== modified file 'src/modules/Unity/Application/application_manager.cpp'
+ // We interpret this as a focus request for a suspended app.
+ // Shell will have this app resumed if it complies with the focus request
     QObject::connect(controller, &TaskController::resumeRequested,
- manager, &ApplicationManager::onResumeRequested);
+ manager, &ApplicationManager::onFocusRequested);

The original purpose of the resumeRequested signal from UAL was to tell AppMan to un-suspend a suspended app, since AppMan used to manage that. But since UAL does suspend/resume now itself, this signal is worthless. Suspect we should remove completely.

Does not the focusRequested signal always get fired if url-dispatcher wants to focus an app? In which case, is this interpretation even necessary? If we can, I think we should remove it.

+ << "ApplicationManager::onProcessStarting(appId="<<appId<<") - User wants to start"
+ " a new instance of an application that is still closing. Queuing its start.";
we should investigate how to launch new instance immediately, instead of waiting for the closing one to quit. I'd be worried if the closing app crashes during close, as it won't be considered closed until apport has completed collecting the crash dump, causing the new instance a long delay. Not for this MP, but perhaps a TODO?

Also nitpick: could you line up the qCDebug strings, in both spots in this method?

+ // TaskController doesn't give us any PID so we have no way of telling apart two instances
+ // of an application.
we should get that fixed. UAL must have way of dealing with multiple instances of the same App, and distinguishing them. If not, we'll need to add it.

+1 on the header for the test. We should do that from now on.

+ // DeferredDelete is special: likes to be called out specifically or it won't come out
weird?? Event loop blocked somehow?

Nice work