Merge into 5.2 : unity-launcher-integration-fixes : Code : Ubuntu Software Center

Reviewer	Date Requested	Status
Michael Vogt		Needs Fixing on 2012-06-29
Matthew Paul Thomas	2012-06-13	Needs Fixing on 2012-06-13
Review via email: mp+109976@code.launchpad.net

Revision history for this message

Michael Vogt (mvo) wrote on 2012-06-13:

#

Thanks for your work on this branch.

The code looks good, I like the fact that we have a fake PURCHASE_TRANSACTION_ID now
in enums for example.

One minor nitpick would be that I liked that we had a seperate function in 5.0 _register_unity_launcher_transaction_started() for on_transaction_started(). But
given how small it is now its probably not needed.

During manual testing I noticed a delay for up to 20s between the finish of the install and the fly-in animation. This needs to be investigated before this can be merged.
I reproduced this delay on both my quantal box and on precise (to ensure that its not a quantal artifact).

Revision history for this message

Matthew Paul Thomas (mpt) wrote on 2012-06-13:

#

Ideally, the Launcher item should appear as soon as the download begins. By the time installation finishes, you might have queued up several other things to install or remove, or even closed USC altogether.

In the short term, though, I guess for an item to show up in the Launcher five minutes late is still better than not showing up at all.

I tried r3054 of this branch with Qreator and with IntelliJ IDEA Community Edition.

It did not work at all with Qreator.

With IntelliJ, the Launcher item appeared about ten seconds after installation finished, but its tooltip said "Waiting to install".

review: Needs Fixing

Revision history for this message

Gary Lasker (gary-lasker) wrote on 2012-06-13:

#

Hey mpt and mvo! Thanks for the reviews.

Regarding the "waiting to install", that's a one line fix in Unity that Bilal already has a branch in review for (I mentioned it above, see the last paragraph of my MP description).

The dbus signal to Unity is fired by Software Center immediately at the end of install. Any delay from that moment is due to issues in the Unity side. To be specific, I have found that the animation on my machine doesn't happen until I actually "show" my hidden launcher. It's important to note that this is not a regression with this branch, I verified the exact same behavior with current software-center 5.2.2.2 in precise. So it appears to be a regression in Unity, which I will report. Is this the "delay" that you are seeing?

As I said, Software Center fires the dbus signal immediately at the end of the install. If Unity does its work, the animation will happen at that exact moment.

Michael, I didn't use the old code as a model but since I remember very well how it worked I just implemented it directly based on what we have now. While implementing I was thinking about making the registering an explicit method and I agree it's nicer, so I'll make that change. Thank you for the suggestion and for noticing it in the old code!

mpt, I don't see anything like "5 minutes later", unless the install takes 5 minutes (which I am assuming to be the case). But, let's be fair, quick installs add the launcher quickly, with the caveat that I mentioned above about the Launcher unhiding. But that is not related to this fix (as I mentioned above) as it also happens in the released USC. I'll test the packages that you tried to see why Qreater didn't get added.

For the record, I tested this carefully for many cases:

  1. normal installs from the Ubuntu archive
  2. multiple simultaneous installs from the Ubuntu archive
  3. a new purchase
  4. reinstalls of previous purchases

So, I'd like to be clear about what "needs fixing". Based on what I see above, this looks like:

  1. Make the _register_unity_launcher_transaction_started() improvement.
  2. Unity lands and SRUs the "Waiting to install" fix. However, that is outside the scope of this MP and I personally feel this should *not* delay our releasing this current fix as is as it takes care of so many high-target bugs for which we are understandably under a lot of pressure to fix. The Waiting to install message is annoying and ugly, but it's far less annoying and ugly than the bugs being fixed. And this message is cleared after a restart, so it's not permanent.
  3. Verify and figure out why Qreator is not being added. I did not hit this issue with any of the applications that I tested with so this may just be an issue with Qreator
  4. The launcher "unhide" needed before animation. Again, this is not in scope for this SRU as it is also the case even in released USC in Precise right now.

Sound good?

Hey mpt and mvo! Thanks for the reviews.

Regarding the "waiting to install", that's a one line fix in Unity that Bilal already has a branch in review for (I mentioned it above, see the last paragraph of my MP description).

The dbus signal to Unity is fired by Software Center immediately at the end of install. Any delay from that moment is due to issues in the Unity side. To be specific, I have found that the animation on my machine doesn't happen until I actually "show" my hidden launcher. It's important to note that this is not a regression with this branch, I verified the exact same behavior with current software-center 5.2.2.2 in precise. So it appears to be a regression in Unity, which I will report. Is this the "delay" that you are seeing?

As I said, Software Center fires the dbus signal immediately at the end of the install. If Unity does its work, the animation will happen at that exact moment.

Michael, I didn't use the old code as a model but since I remember very well how it worked I just implemented it directly based on what we have now. While implementing I was thinking about making the registering an explicit method and I agree it's nicer, so I'll make that change. Thank you for the suggestion and for noticing it in the old code!

mpt, I don't see anything like "5 minutes later", unless the install takes 5 minutes (which I am assuming to be the case). But, let's be fair, quick installs add the launcher quickly, with the caveat that I mentioned above about the Launcher unhiding. But that is not related to this fix (as I mentioned above) as it also happens in the released USC. I'll test the packages that you tried to see why Qreater didn't get added.

For the record, I tested this carefully for many cases:

1. normal installs from the Ubuntu archive
  2. multiple simultaneous installs from the Ubuntu archive
  3. a new purchase
  4. reinstalls of previous purchases

So, I'd like to be clear about what "needs fixing". Based on what I see above, this looks like:

1. Make the _register_unity_launcher_transaction_started() improvement.
  2. Unity lands and SRUs the "Waiting to install" fix. However, that is outside the scope of this MP and I personally feel this should *not* delay our releasing this current fix as is as it takes care of so many high-target bugs for which we are understandably under a lot of pressure to fix. The Waiting to install message is annoying and ugly, but it's far less annoying and ugly than the bugs being fixed. And this message is cleared after a restart, so it's not permanent.
  3. Verify and figure out why Qreator is not being added. I did not hit this issue with any of the applications that I tested with so this may just be an issue with Qreator
  4. The launcher "unhide" needed before animation. Again, this is not in scope for this SRU as it is also the case even in released USC in Precise right now.

Sound good?

Revision history for this message

Michael Vogt (mvo) wrote on 2012-06-13:

#

Download full text (4.0 KiB)

On Wed, Jun 13, 2012 at 01:39:28PM -0000, Gary Lasker wrote:
> Hey mpt and mvo! Thanks for the reviews.
Hi Gary,

thanks for your feedback.

> Regarding the "waiting to install", that's a one line fix in Unity
> that Bilal already has a branch in review for (I mentioned it above,
> see the last paragraph of my MP description).

Yeah, we need to case the unity team to see if/why that merge is
blocked and if there is anything we can do to help getting this
unblocked.

> The dbus signal to Unity is fired by Software Center immediately at
> the end of install. Any delay from that moment is due to issues in
> the Unity side. To be specific, I have found that the animation on
> my machine doesn't happen until I actually "show" my hidden
> launcher. It's important to note that this is not a regression with
> this branch, I verified the exact same behavior with current
> software-center 5.2.2.2 in precise. So it appears to be a regression
> in Unity, which I will report. Is this the "delay" that you are
> seeing?

Yes, that is the delay I'm seeing. Its rather odd and needs
investigation and a bugreport for unity. With that delay the UI looks
rather odd, I had a 20s delay between USC telling me that the app is
installed before it was going to the launcher. Before this was also
there but it was mitigated because the install was usually still in
progress.

> Michael, I didn't use the old code as a model but since I remember
> very well how it worked I just implemented it directly based on what
> we have now. While implementing I was thinking about making the
> registering an explicit method and I agree it's nicer, so I'll make
> that change. Thank you for the suggestion and for noticing it in the
> old code!

Like I said in the review, its really minor and not a blocker.

> So, I'd like to be clear about what "needs fixing". Based on what I see above, this looks like:
>
> 1. Make the _register_unity_launcher_transaction_started()
> improvement.

Low priority, not a blocker (but a low hanging fruit).

> 2. Unity lands and SRUs the "Waiting to install" fix. However,
> that is outside the scope of this MP and I personally feel this
> should *not* delay our releasing this current fix as is as it takes
> care of so many high-target bugs for which we are understandably
> under a lot of pressure to fix. The Waiting to install message is
> annoying and ugly, but it's far less annoying and ugly than the bugs
> being fixed. And this message is cleared after a restart, so it's
> not permanent.

It makes the experience look unpolished, so we need to get in touch
with the unity guys and get information about if there is anything
that is blocking that merge and what we can do to help (and also what
SRU this can go into).

> 3. Verify and figure out why Qreator is not being added. I did not
> hit this issue with any of the applications that I tested with so
> this may just be an issue with Qreator

Yes, that needs investigation, I think its just the fact that it comes
from extras and that our guessing code is not smart enough in this
case. A good fix would be to look at /var/lib/dpkg/info/$pkgname.list
for any desktop file in there if $pkgname.desktop ...

On Wed, Jun 13, 2012 at 01:39:28PM -0000, Gary Lasker wrote:
> Hey mpt and mvo! Thanks for the reviews. 
Hi Gary,

thanks for your feedback.
 
> Regarding the "waiting to install", that's a one line fix in Unity
> that Bilal already has a branch in review for (I mentioned it above,
> see the last paragraph of my MP description).

Yeah, we need to case the unity team to see if/why that merge is
blocked and if there is anything we can do to help getting this
unblocked.
 
> The dbus signal to Unity is fired by Software Center immediately at
> the end of install. Any delay from that moment is due to issues in
> the Unity side. To be specific, I have found that the animation on
> my machine doesn't happen until I actually "show" my hidden
> launcher. It's important to note that this is not a regression with
> this branch, I verified the exact same behavior with current
> software-center 5.2.2.2 in precise. So it appears to be a regression
> in Unity, which I will report. Is this the "delay" that you are
> seeing?

Yes, that is the delay I'm seeing. Its rather odd and needs
investigation and a bugreport for unity. With that delay the UI looks
rather odd, I had a 20s delay between USC telling me that the app is
installed before it was going to the launcher. Before this was also
there but it was mitigated because the install was usually still in
progress.

> Michael, I didn't use the old code as a model but since I remember
> very well how it worked I just implemented it directly based on what
> we have now. While implementing I was thinking about making the
> registering an explicit method and I agree it's nicer, so I'll make
> that change. Thank you for the suggestion and for noticing it in the
> old code!

Like I said in the review, its really minor and not a blocker.

> So, I'd like to be clear about what "needs fixing". Based on what I see above, this looks like:
> 
>   1. Make the _register_unity_launcher_transaction_started()
> improvement.

Low priority, not a blocker (but a low hanging fruit).

>   2. Unity lands and SRUs the "Waiting to install" fix. However,
> that is outside the scope of this MP and I personally feel this
> should *not* delay our releasing this current fix as is as it takes
> care of so many high-target bugs for which we are understandably
> under a lot of pressure to fix. The Waiting to install message is
> annoying and ugly, but it's far less annoying and ugly than the bugs
> being fixed. And this message is cleared after a restart, so it's
> not permanent.

It makes the experience look unpolished, so we need to get in touch
with the unity guys and get information about if there is anything
that is blocking that merge and what we can do to help (and also what
SRU this can go into).

>   3. Verify and figure out why Qreator is not being added. I did not
>    hit this issue with any of the applications that I tested with so
>    this may just be an issue with Qreator

Yes, that needs investigation, I think its just the fact that it comes
from extras and that our guessing code is not smart enough in this
case. A good fix would be to look at /var/lib/dpkg/info/$pkgname.list
for any desktop file in there if $pkgname.desktop is not found. I'm
confident this would fix the issue. It should be done in a seperate
branch though.

> 4. The launcher "unhide" needed before animation. Again, this is not
>    in scope for this SRU as it is also the case even in released USC
>    in Precise right now.

I'm not sure that I understand this correctly, if it means that there
is a delay between firing the dbus signal and the animation then the
next step is a bugreport for unity and some investigation what might
cause this and a estitmate about a fix. While its not part of this MP
going with the delay make the UI look (more) unpolished so I would
like to have a plan and a timeline.

So to me the code in the MP is fine, but there are some external
dependencies on unity that we also need to track and push
forward.

Mpt, you are welcome to comment as well if I missed anything.

Cheers,
 Michael

lp:~gary-lasker/software-center/unity-launcher-integration-fixes updated on 2012-06-13

3055. By Gary Lasker on 2012-06-13: REFACTOR: just make a _register_unity_launcher_transaction_started() method. it's nicer

Revision history for this message

Gary Lasker (gary-lasker) wrote on 2012-06-13:

#

Interestingly, Qreator's desktop file is named "extras-qreator.desktop", and so yes, our guessing code is not good enough, but it seems that all (two) of the current packages in "Independent" have their desktop file name prepended with "extras-" in the form:

extras-<pkgname>.desktop

So it seems a convention (sample size of 2) and I'll just add this to the guessing code as this ony applies to purchased and ARB apps only and this has been reliable for purchased apps for cycles now, so adding the extras check should take care of those cases similarly. I've added this now and it seems to work fine, and will write a new bug for this case.

Indeed, /var/lib/dpkg/info/qreator.list has the correct name for the desktop file, so if we decide we do need to parse these files we can do that in a separate branch, as you suggested.

lp:~gary-lasker/software-center/unity-launcher-integration-fixes updated on 2012-06-13

3056. By Gary Lasker on 2012-06-13: also fix the bug where apps in extras.ubuntu.com are not being added to the Unity launcher, thanks mpt for finding this
3057. By Gary Lasker on 2012-06-13: add the actual bug number in the comment

Revision history for this message

Gary Lasker (gary-lasker) wrote on 2012-06-13:

#

I added bug 1012877 for the case of apps in the extras.ubuntu.com section, this is how also fixed in this branch.

lp:~gary-lasker/software-center/unity-launcher-integration-fixes updated on 2012-06-13

3058. By Gary Lasker on 2012-06-13: send an empty string for the transaction ID value, this is expected in Bilal's corresponding Unity-side fix at lp:~bilalakhtar/unity/waiting-to-install-tooltip-925014

Revision history for this message

Gary Lasker (gary-lasker) wrote on 2012-06-13:

#

I took at look at Bilal's Unity side fix at https://code.launchpad.net/~bilalakhtar/unity/waiting-to-install-tooltip-925014/+merge/108858, and it looks to me to be approved now and ready for merging.

I noticed that his code is expecting an empty value for the transaction ID in the launcher dbus call in order to avoid displaying the "Waiting to install" tooltip, so I made that supporting change in this branch. Note that since we are firing the dbus call at the very end of the transaction now, this value will have no use at the launcher side anyway.

I've included a TODO in the code to document this for when we need to change this back.

Revision history for this message

Gary Lasker (gary-lasker) wrote on 2012-06-13:

#

I've submitted a new bug for the Unity team describing the delayed "fly to launcher" behavior when the Unity launcher is set to auto-hide. It's bug 1012896.

Revision history for this message

Bilal Akhtar (bilalakhtar) wrote on 2012-06-14:

#

I've fixed the delay bug in this branch:

https://code.launchpad.net/~bilalakhtar/unity/5.0series-sru-software-center-integration-fixes/+merge/110214

That branch also contains the fix to the Waiting to install tooltip and the "launcher stays stuck in revealed mode" bug.

Revision history for this message

Michael Vogt (mvo) wrote on 2012-06-14:

#

On Wed, Jun 13, 2012 at 09:20:26PM -0000, Gary Lasker wrote:
> Interestingly, Qreator's desktop file is named "extras-qreator.desktop", and so yes, our guessing code is not good enough, but it seems that all (two) of the current packages in "Independent" have their desktop file name prepended with "extras-" in the form:
>
> extras-<pkgname>.desktop
>
> So it seems a convention (sample size of 2) and I'll just add this to the guessing code as this ony applies to purchased and ARB apps only and this has been reliable for purchased apps for cycles now, so adding the extras check should take care of those cases similarly. I've added this now and it seems to work fine, and will write a new bug for this case.

Yeah, that is the convention for extras.ubuntu.com, thanks for fixing it.

> Indeed, /var/lib/dpkg/info/qreator.list has the correct name for the desktop file, so if we decide we do need to parse these files we can do that in a separate branch, as you suggested.

Could you please file a bug about this so that we can keep track of
it? This would fix it for all cases when we can't guess the desktop
file from the pkgname (unless there are multiple desktop files in the
package in which case we will have to guess or add them all ;).

Cheers,
Michael

Revision history for this message

Gary Lasker (gary-lasker) wrote on 2012-06-14:

#

Bilal, I just want to say thank you very much for your super-fast response in making the needed fixes on the Unity side. :) As always, I very much appreciate your dedication and your good work!!

My best to you,
Gary

Revision history for this message

Gary Lasker (gary-lasker) wrote on 2012-06-14:

#

I have a note in to didrocks to try to help get Bilal's branch reviewed and to get an idea about the timeline for when we can expect the fix to appear in a Unity Precise SRU.

Revision history for this message

Michael Vogt (mvo) wrote on 2012-06-15:

#

Thanks Gary, that is great to hear.

I will unset the "Needs fixing" as the branch is fine and ready. Given that we need to wait for the current SRU I think the approach to get the timeline for a unity SRU and sync with that (if its not in the too distant future) is the best approach.

Revision history for this message

Michael Vogt (mvo) on 2012-06-15:

#

review: Approve

Revision history for this message

Michael Vogt (mvo) wrote on 2012-06-15:

#

Just for completness, here are the branch from Bilal:
https://code.launchpad.net/~bilalakhtar/unity/5.0series-sru-software-center-integration-fixes

Revision history for this message

Gary Lasker (gary-lasker) wrote on 2012-06-19:

#

I removed bug 946789 and bug 993224 from the changelog set and from the description as, although these are fixed as a side effect of the fix in the branch, the proper and final fix for these will be to have the Unity launcher service monitor all of the installation events from aptdaemon and to react to them accordingly. Since this work on the Unity side is being tracked in bug 1011681, I have marked the above two bugs as duplicates of that one.

Revision history for this message

Michael Vogt (mvo) wrote on 2012-06-29:

#

While we are waiting for the unity side, it would be great if you could address the points in:
https://code.launchpad.net/~gary-lasker/software-center/tech-items-to-launcher-fix-lp1006483/+merge/111135/comments/238731 to update the test to the new even sending behavior.

review: Needs Fixing

lp:~gary-lasker/software-center/unity-launcher-integration-fixes updated on 2012-07-16

3059. By Gary Lasker on 2012-07-16: sync to latest 5.2 branch

Revision history for this message

Gary Lasker (gary-lasker) wrote on 2012-07-16:

#

Hi Michael, please check the separate MP for the enhanced set of unit tests here:

https://code.launchpad.net/~gary-lasker/software-center/launcher-integration-unit-tests/+merge/115236

Thanks!

Revision history for this message

Michael Vogt (mvo) wrote on 2012-07-17:

#

On Mon, Jul 16, 2012 at 10:03:18PM -0000, Gary Lasker wrote:
> Hi Michael, please check the separate MP for the enhanced set of unit tests here:
>
> https://code.launchpad.net/~gary-lasker/software-center/launcher-integration-unit-tests/+merge/115236

Thanks for your work on this. I commented in the above
merge-proposal. Its good that the tests are covering the start/end now
and that more of the desktop file cases are checked now. Unfortunately
there seems to be a issue with the patch not quite patching the right
spot (see the MP) so that _fake_send_application_to_launcher_and_check
is not called. That will need addressing before this can land.

Thanks,
Michael

Ubuntu Software Center

Merge lp:~gary-lasker/software-center/unity-launcher-integration-fixes into lp:software-center/5.2

Commit message

Description of the change

Preview Diff

Subscribers

 === modified file 'softwarecenter/backend/installbackend_impl/aptd.py'
 --- softwarecenter/backend/installbackend_impl/aptd.py	2012-06-28 08:10:33 +0000
 +++ softwarecenter/backend/installbackend_impl/aptd.py	2012-07-16 16:45:26 +0000
@@ -29,7 +29,7 @@
                                    obfuscate_private_ppa_details,
                                    utf8,
+                                   )
--from softwarecenter.enums import TransactionTypes
++from softwarecenter.enums import TransactionTypes, PURCHASE_TRANSACTION_ID
  from aptdaemon import client
  from aptdaemon import enums
@@ -67,11 +67,13 @@
  class FakePurchaseTransaction(object):
++
      def __init__(self, app, iconname):
          self.pkgname = app.pkgname
          self.appname = app.appname
          self.iconname = iconname
          self.progress = 0
++        self.tid = PURCHASE_TRANSACTION_ID
  class AptdaemonTransaction(BaseTransaction):
@@ -545,8 +547,6 @@
          and finally installing the specified application once the
          package list reload has completed.
          """
--        self.emit("transaction-started", app.pkgname, app.appname,
--            "FIXME-NEED-ID-HERE", TransactionTypes.INSTALL)
          self._logger.info("add_repo_add_key_and_install_app() '%s' '%s' '%s'" %
              (re.sub("deb https://.*@", "", deb_line),  # strip out password
              signing_key_id,
@@ -565,6 +565,8 @@
                  return
              # done
              fake_trans = FakePurchaseTransaction(app, iconname)
++            self.emit("transaction-started", app.pkgname, app.appname,
++                fake_trans.tid, TransactionTypes.INSTALL)
              self.pending_purchases[app.pkgname] = fake_trans
          else:
              # FIXME: add authenticate_for_added_repo here
 === modified file 'softwarecenter/backend/unitylauncher.py'
 --- softwarecenter/backend/unitylauncher.py	2012-04-24 14:22:29 +0000
 +++ softwarecenter/backend/unitylauncher.py	2012-07-16 16:45:26 +0000
@@ -33,7 +33,7 @@
                   icon_x,
                   icon_y,
                   icon_size,
--                 app_install_desktop_file_path,
++                 installed_desktop_file_path,
                   trans_id):
          self.name = name
          self.icon_name = icon_name
@@ -41,7 +41,7 @@
          self.icon_x = icon_x
          self.icon_y = icon_y
          self.icon_size = icon_size
--        self.app_install_desktop_file_path = app_install_desktop_file_path
++        self.installed_desktop_file_path = installed_desktop_file_path
          self.trans_id = trans_id
@@ -73,8 +73,8 @@
                    launcher_info.name)
          LOG.debug("  launcher_info.icon_file_path: ",
                       launcher_info.icon_file_path)
--        LOG.debug("  launcher_info.app_install_desktop_file_path: ",
--                     launcher_info.app_install_desktop_file_path)
++        LOG.debug("  launcher_info.installed_desktop_file_path: ",
++                     launcher_info.installed_desktop_file_path)
          LOG.debug("  launcher_info.trans_id: ", launcher_info.trans_id)
          try:
@@ -89,7 +89,7 @@
                      launcher_info.icon_x,
                      launcher_info.icon_y,
                      launcher_info.icon_size,
--                    launcher_info.app_install_desktop_file_path,
++                    launcher_info.installed_desktop_file_path,
                      launcher_info.trans_id)
          except Exception as e:
              LOG.warn("could not send dbus signal to the Unity launcher: (%s)",
 === modified file 'softwarecenter/enums.py'
 --- softwarecenter/enums.py	2012-05-22 14:36:03 +0000
 +++ softwarecenter/enums.py	2012-07-16 16:45:26 +0000
@@ -258,6 +258,9 @@
  #carousel app limit to override limit in .menu file for category
  TOP_RATED_CAROUSEL_LIMIT = 12
++# Transaction ID for the "fake" purchase transaction
++PURCHASE_TRANSACTION_ID = "FakePurchaseTransactionID"
++
  from .version import VERSION, DISTRO, RELEASE, CODENAME
  USER_AGENT = "Software Center/%s (N;) %s/%s (%s)" % (
      VERSION, DISTRO, RELEASE, CODENAME)
 === modified file 'softwarecenter/ui/gtk3/panes/availablepane.py'
 --- softwarecenter/ui/gtk3/panes/availablepane.py	2012-06-28 08:10:33 +0000
 +++ softwarecenter/ui/gtk3/panes/availablepane.py	2012-07-16 16:45:26 +0000
@@ -37,7 +37,8 @@
  from softwarecenter.paths import APP_INSTALL_PATH
  from softwarecenter.utils import (wait_for_apt_cache_ready,
                                    get_exec_line_from_desktop,
--                                  get_file_path_from_iconname)
++                                  get_file_path_from_iconname,
++                                  convert_desktop_file_to_installed_location)
  from softwarecenter.db.appfilter import AppFilter
  from softwarecenter.db.database import Application
@@ -381,13 +382,8 @@
      def on_transaction_started(self, backend, pkgname, appname, trans_id,
                                 trans_type):
--        # we only care about installs for the launcher, queue here for
--        # later, see #972710
--        if trans_type == TransactionTypes.INSTALL:
--            transaction_details = TransactionDetails(
--                    pkgname, appname, trans_id, trans_type)
--            self.unity_launcher_transaction_queue[pkgname] = (
--                    transaction_details)
++        self._register_unity_launcher_transaction_started(pkgname, appname,
++                                                          trans_id, trans_type)
      def on_transactions_changed(self, backend, pending_transactions):
          """internal helper that keeps the action bar up-to-date by
@@ -395,61 +391,82 @@
          """
          if self._is_custom_list_search(self.state.search_term):
              self._update_action_bar()
--        # add app to unity launcher on the first sign of progress
--        # and remove from the pending queue once that is done
--        for pkgname in pending_transactions:
--            if pkgname in self.unity_launcher_transaction_queue:
--                transaction_details = (
--                    self.unity_launcher_transaction_queue.pop(pkgname))
--                self._add_application_to_unity_launcher(transaction_details)
      def on_transaction_complete(self, backend, result):
          if result.pkgname in self.unity_launcher_transaction_queue:
--            self.unity_launcher_transaction_queue.pop(result.pkgname)
++            transaction_details = (
++                self.unity_launcher_transaction_queue.pop(result.pkgname))
++            self._add_application_to_unity_launcher(transaction_details)
++
++    def _register_unity_launcher_transaction_started(self, pkgname, appname,
++                                                     trans_id, trans_type):
++        # at the start of the transaction, we gather details for use later
++        # when it is time to add the installed app to the Unity launcher,
++        # (see #972710). we only care about installs for the launcher
++        # mvo: use softwarecenter.utils explicitly here so that we can monkey
++        #      patch it in the test
++        if (trans_type == TransactionTypes.INSTALL and
++                self.add_to_launcher_enabled and
++                softwarecenter.utils.is_unity_running()):
++            transaction_details = TransactionDetails(
++                    pkgname, appname, trans_id, trans_type)
++            self.unity_launcher_transaction_queue[pkgname] = (
++                    transaction_details)
      def _add_application_to_unity_launcher(self, transaction_details):
--        if not self.add_to_launcher_enabled:
--            return
--        # mvo: use use softwarecenter.utils explicitly so that we can monkey
--        #      patch it in the test
--        if not softwarecenter.utils.is_unity_running():
--            return
--
          app = Application(pkgname=transaction_details.pkgname,
                            appname=transaction_details.appname)
          appdetails = app.get_details(self.db)
++        # convert the app-install desktop file location to the actual installed
++        # desktop file location (or in the case of a purchased item from the
++        # agent, generate the correct installed desktop file location)
++        installed_desktop_file_path = (
++            convert_desktop_file_to_installed_location(appdetails.desktop_file,
++                                                       app.pkgname))
          # we only add items to the launcher that have a desktop file
--        if not appdetails.desktop_file:
++        if not installed_desktop_file_path:
              return
          # do not add apps that have no Exec entry in their desktop file
          # (e.g. wine, see LP: #848437 or ubuntu-restricted-extras,
          # see LP: #913756)
--        if (os.path.exists(appdetails.desktop_file) and
--            not get_exec_line_from_desktop(appdetails.desktop_file)):
++        if (os.path.exists(installed_desktop_file_path) and
++            not get_exec_line_from_desktop(installed_desktop_file_path)):
              return
          # now gather up the unity launcher info items and send the app to the
          # launcher service
          launcher_info = self._get_unity_launcher_info(app, appdetails,
++                installed_desktop_file_path,
                  transaction_details.trans_id)
          self.unity_launcher.send_application_to_launcher(
                  transaction_details.pkgname,
                  launcher_info)
--    def _get_unity_launcher_info(self, app, appdetails, trans_id):
++    def _get_unity_launcher_info(
++            self, app, appdetails, installed_desktop_file_path, trans_id):
          (icon_size, icon_x, icon_y) = (
                  self._get_onscreen_icon_details_for_launcher_service(app))
          icon_path = get_file_path_from_iconname(
                                  self.icons,
--                                iconname=appdetails.icon_file_name)
++                                iconname=appdetails.icon)
++        # Note that the transaction ID is not specified because we are firing
++        # the dbus signal at the very end of the installation now, and the
++        # corresponding fix in Unity is expecting this value to be empty (it
++        # would not be useful to the Unity launcher at this point anyway,
++        # as the corresponding transaction would be complete).
++        # Please see bug LP: #925014 and corresponding Unity branch for
++        # further details.
++        # TODO: If and when we re-implement firing the dbus signal at the
++        # start of the transaction, at that time we will need to replace
++        # the empty string below with the trans_id value.
          launcher_info = UnityLauncherInfo(app.name,
                                            appdetails.icon,
                                            icon_path,
                                            icon_x,
                                            icon_y,
                                            icon_size,
--                                          appdetails.desktop_file,
--                                          trans_id)
++                                          installed_desktop_file_path,
++                                          "")
          return launcher_info
      def _get_onscreen_icon_details_for_launcher_service(self, app):
 === modified file 'softwarecenter/utils.py'
 --- softwarecenter/utils.py	2012-05-24 08:55:44 +0000
 +++ softwarecenter/utils.py	2012-07-16 16:45:26 +0000
@@ -430,12 +430,18 @@
          if os.path.exists(installed_desktop_file_path):
              return installed_desktop_file_path
      # lastly, just try checking directly for the desktop file based on the
--    # pkgname itself
++    # pkgname itself for the case of for-purchase items, etc.
      if pkgname:
          installed_desktop_file_path = "/usr/share/applications/%s.desktop" %\
              pkgname
          if os.path.exists(installed_desktop_file_path):
              return installed_desktop_file_path
++        # files in the extras archive have their desktop filenames prepended
++        # by "extras-", so we check for that also (LP: #1012877)
++        extras_desktop_file_path = ("/usr/share/applications/"
++            "extras-%s.desktop" % pkgname)
++        if os.path.exists(extras_desktop_file_path):
++            return extras_desktop_file_path
      LOG.warn("Could not determine the installed desktop file path for "
               "app-install desktop file: '%s'" % app_install_data_file_path)
      return ""
 === modified file 'test/gtk3/test_unity_launcher_integration.py'
 --- test/gtk3/test_unity_launcher_integration.py	2012-03-07 06:06:59 +0000
 +++ test/gtk3/test_unity_launcher_integration.py	2012-07-16 16:45:26 +0000
@@ -87,8 +87,8 @@
          # list view case) or 96 pixels (for the details view case)
          self.assertTrue(launcher_info.icon_size == 32 or
                          launcher_info.icon_size == 96)
--        self.assertEqual(launcher_info.app_install_desktop_file_path,
--                self.expected_launcher_info.app_install_desktop_file_path)
++        self.assertEqual(launcher_info.installed_desktop_file_path,
++                self.expected_launcher_info.installed_desktop_file_path)
          self.assertEqual(launcher_info.trans_id,
                  self.expected_launcher_info.trans_id)
 === modified file 'test/test_utils.py'
 --- test/test_utils.py	2012-05-24 08:55:44 +0000
 +++ test/test_utils.py	2012-07-16 16:45:26 +0000
@@ -18,8 +18,11 @@
  from softwarecenter.utils import (decode_xml_char_reference,
                                    release_filename_in_lists_from_deb_line,
                                    get_http_proxy_string_from_libproxy,
++                                  get_file_path_from_iconname,
+                                   )
--from softwarecenter.testutils import do_events
++from softwarecenter.testutils import (do_events,
++                                      get_test_gtk3_icon_cache,
++                                      )
  class TestSCUtils(unittest.TestCase):
@@ -215,6 +218,16 @@
          # cleanup
          shutil.rmtree(tmp)
++    def test_get_file_path_from_iconname(self):
++        icons = get_test_gtk3_icon_cache()
++        icon_path = get_file_path_from_iconname(
++                icons,
++                "softwarecenter")
++        self.assertEqual(
++                icon_path,
++                "/usr/share/icons/hicolor/48x48/apps/softwarecenter.svg")
++
++
  class TestExpungeCache(unittest.TestCase):
      def test_expunge_cache(self):