Code review comment for lp:~mandel/unity/plan-b

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

« Back to merge proposal