Merge lp:~mandel/unity/plan-b into lp:unity

Proposed by Manuel de la Peña
Status: Merged
Approved by: Marco Trevisan (Treviño)
Approved revision: 3280
Merged at revision: 3308
Proposed branch: lp:~mandel/unity/plan-b
Merge into: lp:unity
Diff against target: 234 lines (+103/-27)
4 files modified
dash/previews/MusicPreview.cpp (+79/-23)
dash/previews/MusicPreview.h (+6/-0)
unity-shared/PreviewStyle.cpp (+12/-0)
unity-shared/PreviewStyle.h (+6/-4)
To merge this branch: bzr merge lp:~mandel/unity/plan-b
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Marco Trevisan (Treviño) Approve
Nick Dedekind (community) Approve
Review via email: mp+158383@code.launchpad.net

Commit message

Add code to ensure that a label is shown when u1 creds are not present.

Description of the change

Add code to ensure that a label is shown when u1 creds are not present.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:3275
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~mandel/unity/plan-b/+merge/158383/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-ci/4/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-amd64-ci/4
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-armhf-ci/4
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-i386-ci/4

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/4/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

12 + for (dash::Preview::InfoHintPtr info_hint : hints)

dash::Preview::InfoHintPtr const& info_hint

Otherwise, LGTM

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

19 + glib::Variant data(g_variant_lookup_value(preview_data,
20 + "no_credentials_label", G_VARIANT_TYPE_ANY));

Not blocking, but shouldn't be G_VARIANT_TYPE_STRING, instead of ANY?

99 + warning_texture_ = new IconTexture(style.GetWarningIcon(), style.GetPaymentWarningWidth(),
100 + style.GetPaymentWarningHeight());

180 +nux::BaseTexture* Style::GetWarningIcon()
181 +{
182 + return nux::CreateTexture2DFromFile(
183 + PKGDATADIR"/warning_icon.png", -1, true);
184 +}

You're leaking here... GetWarningIcon() returns a nux::BaseTexture that has not been unreffed, so when passing it to IconTexture, it gets reffed and its reference count is 2 instead of being just 1. So it won't ever destructed.

Solutions: (1) use the LazyLoader for that as well, (2) manually unref it when returning. PreviewStyle's textures are intended to be used as they are, I think it's better to keep the same assumptions.

Also, if the texture is already 22x22 you don't really need to GetPaymentWarningWidth()/GetPaymentWarningHeight(); IconTexture will guess it for you.

review: Needs Fixing
Revision history for this message
Alejandro J. Cura (alecu) wrote :

According to the logs, the jenkins build is only failing on armhf:
https://jenkins.qa.ubuntu.com/job/unity-raring-armhf-ci/17/console

The unity build is being killed after two hours while running some headless tests, but after all of unity has been built.

Perhaps for unity builds on armhf, the two hours limit should be increased even further (!)

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Thanks for fixing it, approved!

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'dash/previews/MusicPreview.cpp'
--- dash/previews/MusicPreview.cpp 2013-01-18 16:02:50 +0000
+++ dash/previews/MusicPreview.cpp 2013-04-17 15:41:26 +0000
@@ -92,6 +92,31 @@
92 Preview::AddProperties(builder);92 Preview::AddProperties(builder);
93}93}
9494
95bool MusicPreview::HasUbuntuOneCredentials()
96{
97 dash::Preview::InfoHintPtrList hints = preview_model_->GetInfoHints();
98 GVariant *preview_data = NULL;
99 for (dash::Preview::InfoHintPtr const& info_hint : hints)
100 {
101 if (info_hint->id == "music_preview")
102 {
103 preview_data = info_hint->value;
104 if (preview_data != NULL)
105 {
106 glib::Variant data(g_variant_lookup_value(preview_data,
107 "no_credentials_label", G_VARIANT_TYPE_STRING));
108
109 if (!data)
110 no_credentials_message_ = "";
111 else
112 no_credentials_message_ = data.GetString();
113 }
114 break;
115 }
116 }
117 return no_credentials_message_.empty();
118}
119
95void MusicPreview::SetupViews()120void MusicPreview::SetupViews()
96{121{
97 dash::MusicPreview* music_preview_model = dynamic_cast<dash::MusicPreview*>(preview_model_.get());122 dash::MusicPreview* music_preview_model = dynamic_cast<dash::MusicPreview*>(preview_model_.get());
@@ -164,29 +189,60 @@
164 // Hints && Actions189 // Hints && Actions
165 nux::VLayout* hints_layout = NULL;190 nux::VLayout* hints_layout = NULL;
166 nux::Layout* actions_layout = NULL;191 nux::Layout* actions_layout = NULL;
167 if (!preview_model_->GetInfoHints().empty())192 bool has_u1_creds = HasUbuntuOneCredentials();
168 {193
169 hints_layout = new nux::VLayout();194 if (has_u1_creds)
170 hints_layout->SetSpaceBetweenChildren(0);195 {
171 hints_layout->AddSpace(0, 1);196 if (!preview_model_->GetInfoHints().empty())
172 preview_info_hints_ = new PreviewInfoHintWidget(preview_model_, style.GetInfoHintIconSizeWidth());197 {
173 AddChild(preview_info_hints_.GetPointer());198 hints_layout = new nux::VLayout();
174 preview_info_hints_->request_close().connect([this]() { preview_container_->request_close.emit(); });199 hints_layout->SetSpaceBetweenChildren(0);
175 hints_layout->AddView(preview_info_hints_.GetPointer(), 0);200 hints_layout->AddSpace(0, 1);
176201 preview_info_hints_ = new PreviewInfoHintWidget(preview_model_, style.GetInfoHintIconSizeWidth());
177 // If there are actions, we use a vertical layout202 AddChild(preview_info_hints_.GetPointer());
178 action_buttons_.clear();203 preview_info_hints_->request_close().connect([this]() { preview_container_->request_close.emit(); });
179 actions_layout = BuildVerticalActionsLayout(preview_model_->GetActions(), action_buttons_);204 hints_layout->AddView(preview_info_hints_.GetPointer(), 0);
180 actions_layout->SetLeftAndRightPadding(0, style.GetDetailsRightMargin());205
181 }206 // If there are actions, we use a vertical layout
182 else // otherwise we add a grid layout.207 action_buttons_.clear();
183 {208 actions_layout = BuildVerticalActionsLayout(preview_model_->GetActions(), action_buttons_);
184 action_buttons_.clear();209 actions_layout->SetLeftAndRightPadding(0, style.GetDetailsRightMargin());
185 actions_layout = BuildGridActionsLayout(preview_model_->GetActions(), action_buttons_);210 }
186 if (action_buttons_.size() < 2)211 else // otherwise we add a grid layout.
187 hint_actions_layout->AddSpace(0, 1);212 {
188 actions_layout->SetLeftAndRightPadding(0, style.GetDetailsRightMargin());213 action_buttons_.clear();
189 }214 actions_layout = BuildGridActionsLayout(preview_model_->GetActions(), action_buttons_);
215 if (action_buttons_.size() < 2)
216 hint_actions_layout->AddSpace(0, 1);
217 actions_layout->SetLeftAndRightPadding(0, style.GetDetailsRightMargin());
218 }
219 }
220 else
221 {
222 // let the user know he needs to connect
223 previews::Style& style = dash::previews::Style::Instance();
224 actions_layout = new nux::HLayout();
225 nux::VLayout* icon_layout = new nux::VLayout();
226 icon_layout->SetLeftAndRightPadding(10);
227
228 warning_texture_ = new IconTexture(style.GetWarningIcon());
229 icon_layout->AddView(warning_texture_.GetPointer(), 0, nux::MINOR_POSITION_START,
230 nux::MINOR_SIZE_FULL, 100.0f, nux::NUX_LAYOUT_BEGIN);
231 actions_layout->AddLayout(icon_layout, 0, nux::MINOR_POSITION_CENTER);
232
233 warning_msg_ = new StaticCairoText(
234 no_credentials_message_, true,
235 NUX_TRACKER_LOCATION);
236 warning_msg_->SetFont(style.u1_warning_font().c_str());
237 warning_msg_->SetLines(-2);
238 warning_msg_->SetMinimumHeight(50);
239 warning_msg_->SetMaximumWidth(300);
240
241 actions_layout->AddView(warning_msg_.GetPointer(), 0, nux::MINOR_POSITION_CENTER);
242
243 }
244
245
190 /////////////////////246 /////////////////////
191247
192 if (hints_layout) hint_actions_layout->AddView(hints_layout, 1);248 if (hints_layout) hint_actions_layout->AddView(hints_layout, 1);
193249
=== modified file 'dash/previews/MusicPreview.h'
--- dash/previews/MusicPreview.h 2012-12-20 18:51:09 +0000
+++ dash/previews/MusicPreview.h 2013-04-17 15:41:26 +0000
@@ -24,6 +24,7 @@
24#define MUSICPREVIEW_H24#define MUSICPREVIEW_H
2525
26#include "Preview.h"26#include "Preview.h"
27#include "unity-shared/IconTexture.h"
2728
28namespace unity29namespace unity
29{30{
@@ -55,9 +56,14 @@
5556
56 void OnPlayTrack(std::string const& uri);57 void OnPlayTrack(std::string const& uri);
57 void OnPauseTrack(std::string const& uri);58 void OnPauseTrack(std::string const& uri);
59 bool HasUbuntuOneCredentials();
5860
59protected:61protected:
60 nux::ObjectPtr<Tracks> tracks_;62 nux::ObjectPtr<Tracks> tracks_;
63 nux::ObjectPtr<StaticCairoText> warning_msg_;
64 nux::ObjectPtr<IconTexture> warning_texture_;
65 std::string no_credentials_message_;
66
61 };67 };
6268
63}69}
6470
=== added file 'resources/warning_icon.png'
65Binary files resources/warning_icon.png 1970-01-01 00:00:00 +0000 and resources/warning_icon.png 2013-04-17 15:41:26 +0000 differ71Binary files resources/warning_icon.png 1970-01-01 00:00:00 +0000 and resources/warning_icon.png 2013-04-17 15:41:26 +0000 differ
=== modified file 'unity-shared/PreviewStyle.cpp'
--- unity-shared/PreviewStyle.cpp 2012-11-29 10:19:01 +0000
+++ unity-shared/PreviewStyle.cpp 2013-04-17 15:41:26 +0000
@@ -99,6 +99,7 @@
99 , preview_play_texture_("/preview_play.svg")99 , preview_play_texture_("/preview_play.svg")
100 , preview_pause_texture_("/preview_pause.svg")100 , preview_pause_texture_("/preview_pause.svg")
101 , preview_spin_texture_("/search_spin.svg")101 , preview_spin_texture_("/search_spin.svg")
102 , warning_icon_texture_("/warning_icon.png")
102 {103 {
103 }104 }
104 ~Impl() {}105 ~Impl() {}
@@ -110,6 +111,7 @@
110 LazyLoadTexture<32> preview_play_texture_;111 LazyLoadTexture<32> preview_play_texture_;
111 LazyLoadTexture<32> preview_pause_texture_;112 LazyLoadTexture<32> preview_pause_texture_;
112 LazyLoadTexture<32> preview_spin_texture_;113 LazyLoadTexture<32> preview_spin_texture_;
114 LazyLoadTexture<22> warning_icon_texture_;
113};115};
114116
115117
@@ -292,6 +294,11 @@
292 return 12;294 return 12;
293}295}
294296
297std::string Style::u1_warning_font() const
298{
299 return "Ubuntu Bold 11.5";
300}
301
295float Style::GetVideoImageAspectRatio() const302float Style::GetVideoImageAspectRatio() const
296{303{
297 return float(540)/380;304 return float(540)/380;
@@ -401,6 +408,11 @@
401 return pimpl->preview_pause_texture_.texture();408 return pimpl->preview_pause_texture_.texture();
402}409}
403410
411nux::BaseTexture* Style::GetWarningIcon()
412{
413 return pimpl->warning_icon_texture_.texture();
414}
415
404nux::BaseTexture* Style::GetSearchSpinIcon(int size)416nux::BaseTexture* Style::GetSearchSpinIcon(int size)
405{417{
406 return pimpl->preview_spin_texture_.texture(size);418 return pimpl->preview_spin_texture_.texture(size);
407419
=== modified file 'unity-shared/PreviewStyle.h'
--- unity-shared/PreviewStyle.h 2012-11-29 10:19:01 +0000
+++ unity-shared/PreviewStyle.h 2013-04-17 15:41:26 +0000
@@ -25,7 +25,7 @@
2525
26#include <Nux/Nux.h>26#include <Nux/Nux.h>
27#include <Nux/View.h>27#include <Nux/View.h>
28 28
29#include <string>29#include <string>
30#include <memory>30#include <memory>
3131
@@ -54,12 +54,12 @@
54public:54public:
55 Style();55 Style();
56 ~Style();56 ~Style();
57 57
58 static Style& Instance();58 static Style& Instance();
5959
60 int GetNavigatorWidth() const;60 int GetNavigatorWidth() const;
61 int GetNavigatorIconSize() const;61 int GetNavigatorIconSize() const;
62 62
63 int GetPreviewWidth() const;63 int GetPreviewWidth() const;
64 int GetPreviewHeight() const;64 int GetPreviewHeight() const;
65 int GetPreviewTopPadding() const;65 int GetPreviewTopPadding() const;
@@ -120,10 +120,12 @@
120 ////////////////////////////////120 ////////////////////////////////
121 // Music Preview121 // Music Preview
122 std::string track_font() const;122 std::string track_font() const;
123 std::string u1_warning_font() const;
124 nux::BaseTexture* GetWarningIcon();
123125
124 int GetTrackHeight() const;126 int GetTrackHeight() const;
125 ////////////////////////////////127 ////////////////////////////////
126 128
127 ////////////////////////////////129 ////////////////////////////////
128 // Movie Preview130 // Movie Preview
129 float GetVideoImageAspectRatio() const;131 float GetVideoImageAspectRatio() const;