registerApp is called on UI thread

Bug #1394734 reported by Michał Karnicki
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Canonical System Image
Fix Released
High
Canonical Devices Products
Telegram app
Fix Released
Critical
Michał Karnicki
Ubuntu Push QML
Fix Released
Critical
Roberto Alsina
ubuntu-push-qml (Ubuntu)
Fix Released
Undecided
Unassigned
ubuntu-push-qml (Ubuntu RTM)
Fix Released
Undecided
Unassigned

Bug Description

Steps to reproduce:
- Flash krillin
- Launch Telegram
- Sign in

What should happen:
- The chat list should be populated almost instantly (depending on connection)

What actually happens:
- "No chats" is displayed, sometimes for as long as 5-8 seconds, and only then the data appears. Problem persists over app launches (not just initial sign in), so kill and re-launch to reproduce again.

Problem:
Ubuntu Push Client does a synchronous registration call, blocking the UI thread (on which the component is instantiated).

--- More details:

Instantiation of a PushClient component (even through a Loader) is causing a visible, 1-1.4 second delay in Telegram app launch, because setting appId from QML triggers a synchronous call to registerApp, that does a synchronous dbus call (which in turn probably does networking for that matter). I measured times of code execution of different parts of Telegram and found that it is the instantiation of PushClient that causes the visible short freeze when starting the app. I also commented the "appId: ..." line and confirmed the delay was no longer visible, which seems to support my thesis.

http://bazaar.launchpad.net/~ubuntu-push-hackers/ubuntu-push-qml/rtm/view/head:/src/Ubuntu/PushNotifications/pushclient.h#L38

Q_PROPERTY(QString appId WRITE registerApp READ getAppId NOTIFY appIdChanged);

when appId is set, this invokes registerApp, which contains the following code:

    // Register to the push client
    QDBusMessage message = QDBusMessage::createMethodCall(PUSH_SERVICE, register_path , PUSH_IFACE, "Register");
    message << appId;
    QDBusMessage token = bus.call(message);
[...]
    this->token = token.arguments()[0].toStringList()[0];

which looks like blocking code.

I'm marking this as Critical, as it affects Telegram, but feel free to consult the urgency with victorp. I'm rather confident he will support that importance.

Related branches

Michał Karnicki (karni)
Changed in libqtelegram:
importance: Undecided → Critical
assignee: nobody → Michał Karnicki (karni)
status: New → Triaged
Michał Karnicki (karni)
tags: added: push-client
tags: added: rtm14
tags: added: ota-1
Michał Karnicki (karni)
Changed in ubuntu-push-qml:
status: New → In Progress
Changed in libqtelegram:
status: Triaged → In Progress
Olli Ries (ories)
Changed in canonical-devices-system-image:
status: New → Confirmed
assignee: nobody → Canonical Devices Products (canonical-devices-products-team)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ubuntu-push-qml - 0.1+15.04.20141127-0ubuntu1

---------------
ubuntu-push-qml (0.1+15.04.20141127-0ubuntu1) vivid; urgency=low

  [ Roberto Alsina ]
  * Make registration async. (LP: #1394734)
 -- Ubuntu daily release <email address hidden> Thu, 27 Nov 2014 17:05:58 +0000

Changed in ubuntu-push-qml (Ubuntu):
status: New → Fix Released
Roberto Alsina (ralsina)
Changed in ubuntu-push-qml:
status: In Progress → Fix Released
Michał Karnicki (karni)
Changed in libqtelegram:
status: In Progress → Fix Committed
Michał Karnicki (karni)
Changed in libqtelegram:
status: Fix Committed → Fix Released
Michał Karnicki (karni)
description: updated
Changed in canonical-devices-system-image:
importance: Undecided → High
milestone: none → ww51-2014
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ubuntu-push-qml - 0.1+15.04.20141212~rtm-0ubuntu1

---------------
ubuntu-push-qml (0.1+15.04.20141212~rtm-0ubuntu1) 14.09; urgency=low

  [ Roberto Alsina ]
  * Make registration async (LP: #1394734)
 -- Ubuntu daily release <email address hidden> Fri, 12 Dec 2014 15:05:49 +0000

Changed in ubuntu-push-qml (Ubuntu RTM):
status: New → Fix Released
Changed in canonical-devices-system-image:
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.