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
1=== modified file 'dash/previews/MusicPreview.cpp'
2--- dash/previews/MusicPreview.cpp 2013-01-18 16:02:50 +0000
3+++ dash/previews/MusicPreview.cpp 2013-04-17 15:41:26 +0000
4@@ -92,6 +92,31 @@
5 Preview::AddProperties(builder);
6 }
7
8+bool MusicPreview::HasUbuntuOneCredentials()
9+{
10+ dash::Preview::InfoHintPtrList hints = preview_model_->GetInfoHints();
11+ GVariant *preview_data = NULL;
12+ for (dash::Preview::InfoHintPtr const& info_hint : hints)
13+ {
14+ if (info_hint->id == "music_preview")
15+ {
16+ preview_data = info_hint->value;
17+ if (preview_data != NULL)
18+ {
19+ glib::Variant data(g_variant_lookup_value(preview_data,
20+ "no_credentials_label", G_VARIANT_TYPE_STRING));
21+
22+ if (!data)
23+ no_credentials_message_ = "";
24+ else
25+ no_credentials_message_ = data.GetString();
26+ }
27+ break;
28+ }
29+ }
30+ return no_credentials_message_.empty();
31+}
32+
33 void MusicPreview::SetupViews()
34 {
35 dash::MusicPreview* music_preview_model = dynamic_cast<dash::MusicPreview*>(preview_model_.get());
36@@ -164,29 +189,60 @@
37 // Hints && Actions
38 nux::VLayout* hints_layout = NULL;
39 nux::Layout* actions_layout = NULL;
40- if (!preview_model_->GetInfoHints().empty())
41- {
42- hints_layout = new nux::VLayout();
43- hints_layout->SetSpaceBetweenChildren(0);
44- hints_layout->AddSpace(0, 1);
45- preview_info_hints_ = new PreviewInfoHintWidget(preview_model_, style.GetInfoHintIconSizeWidth());
46- AddChild(preview_info_hints_.GetPointer());
47- preview_info_hints_->request_close().connect([this]() { preview_container_->request_close.emit(); });
48- hints_layout->AddView(preview_info_hints_.GetPointer(), 0);
49-
50- // If there are actions, we use a vertical layout
51- action_buttons_.clear();
52- actions_layout = BuildVerticalActionsLayout(preview_model_->GetActions(), action_buttons_);
53- actions_layout->SetLeftAndRightPadding(0, style.GetDetailsRightMargin());
54- }
55- else // otherwise we add a grid layout.
56- {
57- action_buttons_.clear();
58- actions_layout = BuildGridActionsLayout(preview_model_->GetActions(), action_buttons_);
59- if (action_buttons_.size() < 2)
60- hint_actions_layout->AddSpace(0, 1);
61- actions_layout->SetLeftAndRightPadding(0, style.GetDetailsRightMargin());
62- }
63+ bool has_u1_creds = HasUbuntuOneCredentials();
64+
65+ if (has_u1_creds)
66+ {
67+ if (!preview_model_->GetInfoHints().empty())
68+ {
69+ hints_layout = new nux::VLayout();
70+ hints_layout->SetSpaceBetweenChildren(0);
71+ hints_layout->AddSpace(0, 1);
72+ preview_info_hints_ = new PreviewInfoHintWidget(preview_model_, style.GetInfoHintIconSizeWidth());
73+ AddChild(preview_info_hints_.GetPointer());
74+ preview_info_hints_->request_close().connect([this]() { preview_container_->request_close.emit(); });
75+ hints_layout->AddView(preview_info_hints_.GetPointer(), 0);
76+
77+ // If there are actions, we use a vertical layout
78+ action_buttons_.clear();
79+ actions_layout = BuildVerticalActionsLayout(preview_model_->GetActions(), action_buttons_);
80+ actions_layout->SetLeftAndRightPadding(0, style.GetDetailsRightMargin());
81+ }
82+ else // otherwise we add a grid layout.
83+ {
84+ action_buttons_.clear();
85+ actions_layout = BuildGridActionsLayout(preview_model_->GetActions(), action_buttons_);
86+ if (action_buttons_.size() < 2)
87+ hint_actions_layout->AddSpace(0, 1);
88+ actions_layout->SetLeftAndRightPadding(0, style.GetDetailsRightMargin());
89+ }
90+ }
91+ else
92+ {
93+ // let the user know he needs to connect
94+ previews::Style& style = dash::previews::Style::Instance();
95+ actions_layout = new nux::HLayout();
96+ nux::VLayout* icon_layout = new nux::VLayout();
97+ icon_layout->SetLeftAndRightPadding(10);
98+
99+ warning_texture_ = new IconTexture(style.GetWarningIcon());
100+ icon_layout->AddView(warning_texture_.GetPointer(), 0, nux::MINOR_POSITION_START,
101+ nux::MINOR_SIZE_FULL, 100.0f, nux::NUX_LAYOUT_BEGIN);
102+ actions_layout->AddLayout(icon_layout, 0, nux::MINOR_POSITION_CENTER);
103+
104+ warning_msg_ = new StaticCairoText(
105+ no_credentials_message_, true,
106+ NUX_TRACKER_LOCATION);
107+ warning_msg_->SetFont(style.u1_warning_font().c_str());
108+ warning_msg_->SetLines(-2);
109+ warning_msg_->SetMinimumHeight(50);
110+ warning_msg_->SetMaximumWidth(300);
111+
112+ actions_layout->AddView(warning_msg_.GetPointer(), 0, nux::MINOR_POSITION_CENTER);
113+
114+ }
115+
116+
117 /////////////////////
118
119 if (hints_layout) hint_actions_layout->AddView(hints_layout, 1);
120
121=== modified file 'dash/previews/MusicPreview.h'
122--- dash/previews/MusicPreview.h 2012-12-20 18:51:09 +0000
123+++ dash/previews/MusicPreview.h 2013-04-17 15:41:26 +0000
124@@ -24,6 +24,7 @@
125 #define MUSICPREVIEW_H
126
127 #include "Preview.h"
128+#include "unity-shared/IconTexture.h"
129
130 namespace unity
131 {
132@@ -55,9 +56,14 @@
133
134 void OnPlayTrack(std::string const& uri);
135 void OnPauseTrack(std::string const& uri);
136+ bool HasUbuntuOneCredentials();
137
138 protected:
139 nux::ObjectPtr<Tracks> tracks_;
140+ nux::ObjectPtr<StaticCairoText> warning_msg_;
141+ nux::ObjectPtr<IconTexture> warning_texture_;
142+ std::string no_credentials_message_;
143+
144 };
145
146 }
147
148=== added file 'resources/warning_icon.png'
149Binary 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
150=== modified file 'unity-shared/PreviewStyle.cpp'
151--- unity-shared/PreviewStyle.cpp 2012-11-29 10:19:01 +0000
152+++ unity-shared/PreviewStyle.cpp 2013-04-17 15:41:26 +0000
153@@ -99,6 +99,7 @@
154 , preview_play_texture_("/preview_play.svg")
155 , preview_pause_texture_("/preview_pause.svg")
156 , preview_spin_texture_("/search_spin.svg")
157+ , warning_icon_texture_("/warning_icon.png")
158 {
159 }
160 ~Impl() {}
161@@ -110,6 +111,7 @@
162 LazyLoadTexture<32> preview_play_texture_;
163 LazyLoadTexture<32> preview_pause_texture_;
164 LazyLoadTexture<32> preview_spin_texture_;
165+ LazyLoadTexture<22> warning_icon_texture_;
166 };
167
168
169@@ -292,6 +294,11 @@
170 return 12;
171 }
172
173+std::string Style::u1_warning_font() const
174+{
175+ return "Ubuntu Bold 11.5";
176+}
177+
178 float Style::GetVideoImageAspectRatio() const
179 {
180 return float(540)/380;
181@@ -401,6 +408,11 @@
182 return pimpl->preview_pause_texture_.texture();
183 }
184
185+nux::BaseTexture* Style::GetWarningIcon()
186+{
187+ return pimpl->warning_icon_texture_.texture();
188+}
189+
190 nux::BaseTexture* Style::GetSearchSpinIcon(int size)
191 {
192 return pimpl->preview_spin_texture_.texture(size);
193
194=== modified file 'unity-shared/PreviewStyle.h'
195--- unity-shared/PreviewStyle.h 2012-11-29 10:19:01 +0000
196+++ unity-shared/PreviewStyle.h 2013-04-17 15:41:26 +0000
197@@ -25,7 +25,7 @@
198
199 #include <Nux/Nux.h>
200 #include <Nux/View.h>
201-
202+
203 #include <string>
204 #include <memory>
205
206@@ -54,12 +54,12 @@
207 public:
208 Style();
209 ~Style();
210-
211+
212 static Style& Instance();
213
214 int GetNavigatorWidth() const;
215 int GetNavigatorIconSize() const;
216-
217+
218 int GetPreviewWidth() const;
219 int GetPreviewHeight() const;
220 int GetPreviewTopPadding() const;
221@@ -120,10 +120,12 @@
222 ////////////////////////////////
223 // Music Preview
224 std::string track_font() const;
225+ std::string u1_warning_font() const;
226+ nux::BaseTexture* GetWarningIcon();
227
228 int GetTrackHeight() const;
229 ////////////////////////////////
230-
231+
232 ////////////////////////////////
233 // Movie Preview
234 float GetVideoImageAspectRatio() const;