Merge lp://qastaging/~nick-dedekind/unity/lp1062107.preview-pre-caching into lp://qastaging/unity

530 + virtual ~PreviewPreCacher();
551 + virtual void OnPreCacheComplete();

Could we make those pure virtual fucntions?

Or if not could we just do:
530 + virtual ~PreviewPreCacher() {}
551 + virtual void OnPreCacheComplete() {}

347 + if (index < 0 || index >=(int)GetResultCount())

Does this need to be casted to an int?

758 + int top_social_info_max_width = MAX(details_width - style.GetAppIconAreaWidth() - style.GetSpaceBetweenIconAndDetails(), 0);
772 + int max_width = MAX(0, geo_cr.width - 2*(geo_cr.width*0.1));
773 + int max_height = MAX(0, (geo_cr.height - TAIL_HEIGHT) - 2*((geo_cr.height - TAIL_HEIGHT)*0.1));
786 + title_->SetMaximumWidth(MAX(0, GetGeometry().width - geo.height - style.GetMusicDurationWidth() - layout_spacing*2));

Lets use std::max() here.

bschaefer@bschaefer:unity$ bzr merge lp:~nick-dedekind/unity/lp1062107.preview-pre-caching
+N UnityCore/PreviewPreCacher.cpp
+N UnityCore/PreviewPreCacher.h
+N tests/test_previews_precacher.cpp
 M UnityCore/CMakeLists.txt
 M UnityCore/HomeLens.cpp
 M UnityCore/HomeLens.h
 M UnityCore/Lens.cpp
 M UnityCore/Lens.h
 M dash/DashView.cpp
 M dash/LensView.cpp
 M dash/LensView.h
 M dash/previews/PreviewContainer.cpp
 M dash/previews/SocialPreview.cpp
 M dash/previews/SocialPreviewContent.cpp
 M dash/previews/Track.cpp
 M tests/CMakeLists.txt
 M tests/test_lens.cpp
Text conflict in dash/LensView.cpp
Text conflict in dash/previews/Track.cpp
Text conflict in tests/CMakeLists.txt
3 conflicts encountered.

> 530 + virtual ~PreviewPreCacher();
> 551 + virtual void OnPreCacheComplete();
>
> Could we make those pure virtual fucntions?
>
> Or if not could we just do:
> 530 + virtual ~PreviewPreCacher() {}
> 551 + virtual void OnPreCacheComplete() {}
>

Can't be pure virtual because we need to instantiate a PreviewPreCacher.
Don't see any benefit in making them inline.

>
> 347 + if (index < 0 || index >=(int)GetResultCount())
>
> Does this need to be casted to an int?
>

Yes. GetResultCount returns an unsigned int. Comes from the dee model.

>
> 758 + int top_social_info_max_width = MAX(details_width -
> style.GetAppIconAreaWidth() - style.GetSpaceBetweenIconAndDetails(), 0);
> 772 + int max_width = MAX(0, geo_cr.width - 2*(geo_cr.width*0.1));
> 773 + int max_height = MAX(0, (geo_cr.height - TAIL_HEIGHT) -
> 2*((geo_cr.height - TAIL_HEIGHT)*0.1));
> 786 + title_->SetMaximumWidth(MAX(0, GetGeometry().width - geo.height -
> style.GetMusicDurationWidth() - layout_spacing*2));
>
> Lets use std::max() here.

Will do.

Use std::max for preview code.
Merged with trunk.

tests/test_previews_precacher.cpp

I don't like the way you're testing PreviewPreCacher because you're testing a mock of it (MockPreviewPreCacher).

Also PreviewPreCacher should inherit from sigc::trackable.

@andyrock: As stated in the description, the supplied test is only meant to check the preview pre-caching sequence (pretty much only the PreviewPreCacher::ContinuePreCaching function).

Getting a lens view into the test harness to test the pre-cacher interaction is somewhat controversial and not suited to unit testing at the moment (Lens Dee related code is "too tight" for testability).
I am hoping to get some refactoring into the lenses soon so they can be properly tested. Once this is done, we can revisit this.

Some previews (eg MusicPreview) are stateful inside libunity, therefore asking for subsiquent previews while viewing another will hose the state.
Until this is recitfied, this cannot be merged.