Merge lp:~stolowski/unity-scopes-shell/diff-updates into lp:unity-scopes-shell
| Status: | Merged |
|---|---|
| Approved by: | Marcus Tomlinson on 2015-10-23 |
| Approved revision: | 283 |
| Merged at revision: | 276 |
| Proposed branch: | lp:~stolowski/unity-scopes-shell/diff-updates |
| Merge into: | lp:unity-scopes-shell |
| Prerequisite: | lp:~stolowski/unity-scopes-shell/in-card-activation |
| Diff against target: |
1077 lines (+555/-134) 16 files modified
debian/control.in (+1/-1) po/POTFILES.in (+0/-10) src/Unity/CMakeLists.txt (+1/-0) src/Unity/categories.cpp (+35/-50) src/Unity/categories.h (+5/-2) src/Unity/resultsmap.cpp (+65/-0) src/Unity/resultsmap.h (+48/-0) src/Unity/resultsmodel.cpp (+75/-2) src/Unity/resultsmodel.h (+4/-0) src/Unity/scope.cpp (+40/-30) src/Unity/scope.h (+3/-3) tests/data/CMakeLists.txt (+1/-0) tests/data/mock-scope-manyresults/CMakeLists.txt (+6/-0) tests/data/mock-scope-manyresults/mock-scope-manyresults.cpp (+185/-0) tests/data/mock-scope-manyresults/mock-scope-manyresults.ini.in (+14/-0) tests/resultstest.cpp (+72/-36) |
| To merge this branch: | bzr merge lp:~stolowski/unity-scopes-shell/diff-updates |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Marcus Tomlinson (community) | 2015-10-06 | Approve on 2015-10-23 | |
| PS Jenkins bot | continuous-integration | Needs Fixing on 2015-10-21 | |
|
Review via email:
|
|||
Commit Message
Apply updates to results model instead of clearing. Removed obsolete code which deals with special categories.
Description of the Change
Apply updates to results model instead of clearing.
These changes are available for testing in silo 20.
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:277
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 278. By Paweł Stołowski on 2015-10-12
-
Minor test improvement
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:278
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 279. By Paweł Stołowski on 2015-10-12
-
Oops, forgot to commit
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:279
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| Marcus Tomlinson (marcustomlinson) wrote : | # |
Hmmm, some scopes seems to crash now. I'm testing on krillin rc-proposed #147, between the standard shell and unity-api packages and those from silo 20.
With your silo, BBC, BBC Sport, and cnet scopes (probably more) crash as soon as you change the department.
| Paweł Stołowski (stolowski) wrote : | # |
> Hmmm, some scopes seems to crash now. I'm testing on krillin rc-proposed #147,
> between the standard shell and unity-api packages and those from silo 20.
>
> With your silo, BBC, BBC Sport, and cnet scopes (probably more) crash as soon
> as you change the department.
Yeah, I can reproduce. Looks like it's related to vertical journal, the last message in the dash log before it crashes comes from unity8 and says:
"VerticalJournal only supports removal from the end of the model"
Need to check with Albert about what can be done about it. Unfortunately, according to him, veritcal journal is "unfixable" in that respect, so I may need to resort to full model clearing for this kind of renderer...
| Paweł Stołowski (stolowski) wrote : | # |
Okay, silo 20 now has the fix for unity8 crash caused by unexpected model updates from plugin.
- 280. By Paweł Stołowski on 2015-10-15
-
Merged trunk
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:280
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| Marcus Tomlinson (marcustomlinson) wrote : | # |
There seems to be a few issues with aggregator scopes now. It looks like while new results are being loaded the first category's results get duplicated. Then when the refresh completes, the duplicates disappear.
The most noticeable scope I see this with is the Photos scope. All results in the My Photos category at the top get duplicated while the refresh is busy, then suddenly disappear when its done.
For aggregators that load quickly I think we get away with not revealing this issue because the duplicates get removed before the new set is displayed. Perhaps we also get away with duplicating results in a top category that is full as we don't see the extras being appended to the end.
Other scope's I've seen this on (although harder to catch) are:
* Food (duplicate "Add your fitbit account" item)
* Nearby (deplete "Where am I" result)
| Marcus Tomlinson (marcustomlinson) wrote : | # |
> There seems to be a few issues with aggregator scopes now. It looks like while
> new results are being loaded the first category's results get duplicated. Then
> when the refresh completes, the duplicates disappear.
>
> The most noticeable scope I see this with is the Photos scope. All results in
> the My Photos category at the top get duplicated while the refresh is busy,
> then suddenly disappear when its done.
>
> For aggregators that load quickly I think we get away with not revealing this
> issue because the duplicates get removed before the new set is displayed.
> Perhaps we also get away with duplicating results in a top category that is
> full as we don't see the extras being appended to the end.
>
> Other scope's I've seen this on (although harder to catch) are:
>
> * Food (duplicate "Add your fitbit account" item)
> * Nearby (deplete "Where am I" result)
That last "deplete" should have been "duplicate" as well (auto-correct fail)
- 281. By Paweł Stołowski on 2015-10-19
-
Use only one timer
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:281
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| Paweł Stołowski (stolowski) wrote : | # |
> There seems to be a few issues with aggregator scopes now. It looks like while
> new results are being loaded the first category's results get duplicated. Then
> when the refresh completes, the duplicates disappear.
>
> The most noticeable scope I see this with is the Photos scope. All results in
> the My Photos category at the top get duplicated while the refresh is busy,
> then suddenly disappear when its done.
>
> For aggregators that load quickly I think we get away with not revealing this
> issue because the duplicates get removed before the new set is displayed.
> Perhaps we also get away with duplicating results in a top category that is
> full as we don't see the extras being appended to the end.
>
> Other scope's I've seen this on (although harder to catch) are:
>
> * Food (duplicate "Add your fitbit account" item)
> * Nearby (deplete "Where am I" result)
I debugged this and it turned out it's due to the way these scopes work... They create *unique* category ids on every search (an id, plus a timestamp each time), e.g. "nominatim-
Now, this creates a serious issue with the existing implementation of the plugin, where we never remove categories, only move them down (and hide) in the model when they are empty - effectively a leak of resources. I think we could stop shell plugin from caching category objects indefinitely, but I would definately leave it for a separate MP if we deem it a good idea.
I talked to Kyle about this problem and he is going to fix the aggregator scopes (or ask respective people from their team to do that), which should fix the issue you saw. Apparently this MP needs to wait till aggregators are updated, otherwise the experience will be suboptimal.
| Paweł Stołowski (stolowski) wrote : | # |
> > There seems to be a few issues with aggregator scopes now. It looks like
> while
> > new results are being loaded the first category's results get duplicated.
> Then
> > when the refresh completes, the duplicates disappear.
> >
> > The most noticeable scope I see this with is the Photos scope. All results
> in
> > the My Photos category at the top get duplicated while the refresh is busy,
> > then suddenly disappear when its done.
> >
> > For aggregators that load quickly I think we get away with not revealing
> this
> > issue because the duplicates get removed before the new set is displayed.
> > Perhaps we also get away with duplicating results in a top category that is
> > full as we don't see the extras being appended to the end.
> >
> > Other scope's I've seen this on (although harder to catch) are:
> >
> > * Food (duplicate "Add your fitbit account" item)
> > * Nearby (deplete "Where am I" result)
>
> I debugged this and it turned out it's due to the way these scopes work...
> They create *unique* category ids on every search (an id, plus a timestamp
> each time), e.g. "nominatim-
> how:declared:
>
> Now, this creates a serious issue with the existing implementation of the
> plugin, where we never remove categories, only move them down (and hide) in
> the model when they are empty - effectively a leak of resources. I think we
> could stop shell plugin from caching category objects indefinitely, but I
> would definately leave it for a separate MP if we deem it a good idea.
>
> I talked to Kyle about this problem and he is going to fix the aggregator
> scopes (or ask respective people from their team to do that), which should fix
> the issue you saw. Apparently this MP needs to wait till aggregators are
> updated, otherwise the experience will be suboptimal.
Related bug for Kyle's aggregators: https:/
| Michi Henning (michihenning) wrote : | # |
In effect, this constitutes a memory leak in the shell, so it needs fixing regardless. There has to be some upper limit on the number of categories that cached for each scope. Some largish number (maybe 200?) per scope should do.
We also should add something to the scopes api doc to day "don't do this".
- 282. By Paweł Stołowski on 2015-10-20
-
Increse typing timeout to 700ms
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:282
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 283. By Paweł Stołowski on 2015-10-21
-
Merged trunk
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:283
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| Marcus Tomlinson (marcustomlinson) wrote : | # |
I'm happy with these changes, looks really nice. Good job Pawel!
- 284. By Paweł Stołowski on 2015-10-30
-
Merged trunk
- 285. By Paweł Stołowski on 2015-11-02
-
Bump unity-api dep

FAILED: Continuous integration, rev:275 jenkins. qa.ubuntu. com/job/ unity-scopes- shell-ci/ 387/ jenkins. qa.ubuntu. com/job/ unity-scopes- shell-wily- amd64-ci/ 44/console jenkins. qa.ubuntu. com/job/ unity-scopes- shell-wily- armhf-ci/ 44/console jenkins. qa.ubuntu. com/job/ unity-scopes- shell-wily- i386-ci/ 44/console
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/unity- scopes- shell-ci/ 387/rebuild
http://