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

> On 07/01/2016 14:39, Gerry Boland wrote:
> > Review: Needs Fixing
> >
> > === 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.
>
> When you tap on a suspended app in the dash you immediately get a
> appStarting followed by a resumeRequest. If you don't respond to it (ie,
> resume&focus the app), ages later you get a focusRequested.
>
> So if I remove resumeRequested I would have to interpret appStarting for
> a already running application as a focus request, which actually
> wouldn't be wrong.
>
> Btw, I find it odd that ubuntu_app_launch work that way, always sending
> an appStarting signal even though the app is already running, but that's
> the way it is...

Fair enough. This will do for now.

>

> >
> >
> > + // 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.
>
> As far as I understand it ubuntu-app-launch (and the upstart service?)
> have the concept that you can have only one running instance of an
> application (like a service, that you start/stop using upstart).
>
> This sounds like a bigger architectural change, out of scope for this MP.

Oh of course. Was just observation.

> > + // DeferredDelete is special: likes to be called out specifically or it
> won't come out
> > weird?? Event loop blocked somehow?
> >
>
> QObject::deleteLater() generates a DeferredDelete event which is handled
> in a special way in QCoreApplication etc, unlike any other QEvent.
> There's code in there specifically for leaving it out of forced event
> processing (like sendPostedEvents) unless you explicitly ask for that
> particular event type.

Interesting, I didn't know that.