From 2a36368bda35aed7210c3285019b286581fc8786 Mon Sep 17 00:00:00 2001 From: Michael Lotz Date: Wed, 8 Apr 2015 10:16:54 +0200 Subject: [PATCH] HaikuDepot: Make BitmapView use SharedBitmaps directly. Instead of extracting a BBitmap out of the SharedBitmap and giving that to BitmapView, set SharedBitmaps directly. When using BBitmaps we circumvent the reference counting of the SharedBitmaps and it would be possible for the SharedBitmap and its BBitmaps to get deleted while one of them was still used in a BitmapView. Fixes use-after-free when icons are updated that are already used in BitmapViews. --- .../haikudepot/ui/FeaturedPackagesView.cpp | 15 ++++--- src/apps/haikudepot/ui/PackageInfoView.cpp | 41 ++++++++++--------- src/apps/haikudepot/ui/ScreenshotWindow.cpp | 6 +-- src/apps/haikudepot/ui/UserLoginWindow.cpp | 7 ++-- src/apps/haikudepot/ui_generic/BitmapView.cpp | 22 ++++++++-- src/apps/haikudepot/ui_generic/BitmapView.h | 9 +++- 6 files changed, 62 insertions(+), 38 deletions(-) diff --git a/src/apps/haikudepot/ui/FeaturedPackagesView.cpp b/src/apps/haikudepot/ui/FeaturedPackagesView.cpp index 80904ecb91..fc3e33eb05 100644 --- a/src/apps/haikudepot/ui/FeaturedPackagesView.cpp +++ b/src/apps/haikudepot/ui/FeaturedPackagesView.cpp @@ -151,16 +151,15 @@ public: fPackageListener->SetPackage(package); if (package->Icon().Get() != NULL) { - fIconView->SetBitmap( - package->Icon()->Bitmap(SharedBitmap::SIZE_64)); + fIconView->SetBitmap(package->Icon(), SharedBitmap::SIZE_64); } else - fIconView->SetBitmap(NULL); + fIconView->UnsetBitmap(); if (package->State() == ACTIVATED) { - fInstalledIconView->SetBitmap( - sInstalledIcon->Bitmap(SharedBitmap::SIZE_16)); + fInstalledIconView->SetBitmap(sInstalledIcon, + SharedBitmap::SIZE_16); } else - fInstalledIconView->SetBitmap(NULL); + fInstalledIconView->UnsetBitmap(); fTitleView->SetText(package->Title()); @@ -199,8 +198,8 @@ public: { fPackageListener->SetPackage(PackageInfoRef(NULL)); - fIconView->SetBitmap(NULL); - fInstalledIconView->SetBitmap(NULL); + fIconView->UnsetBitmap(); + fInstalledIconView->UnsetBitmap(); fTitleView->SetText(""); fPublisherView->SetText(""); fSummaryView->SetText(""); diff --git a/src/apps/haikudepot/ui/PackageInfoView.cpp b/src/apps/haikudepot/ui/PackageInfoView.cpp index 4e0db2fa5e..5d95ce4387 100644 --- a/src/apps/haikudepot/ui/PackageInfoView.cpp +++ b/src/apps/haikudepot/ui/PackageInfoView.cpp @@ -391,9 +391,9 @@ public: void SetPackage(const PackageInfo& package) { if (package.Icon().Get() != NULL) - fIconView->SetBitmap(package.Icon()->Bitmap(SharedBitmap::SIZE_32)); + fIconView->SetBitmap(package.Icon(), SharedBitmap::SIZE_32); else - fIconView->SetBitmap(NULL); + fIconView->UnsetBitmap(); fTitleView->SetText(package.Title()); @@ -431,7 +431,7 @@ public: void Clear() { - fIconView->SetBitmap(NULL); + fIconView->UnsetBitmap(); fTitleView->SetText(""); fPublisherView->SetText(""); fVersionInfo->SetText(""); @@ -797,31 +797,36 @@ public: fDescriptionView->SetText(package.ShortDescription(), package.FullDescription()); - fEmailIconView->SetBitmap(fEmailIcon.Bitmap(SharedBitmap::SIZE_16)); + fEmailIconView->SetBitmap(&fEmailIcon, SharedBitmap::SIZE_16); _SetContactInfo(fEmailLinkView, package.Publisher().Email()); - fWebsiteIconView->SetBitmap(fWebsiteIcon.Bitmap(SharedBitmap::SIZE_16)); + fWebsiteIconView->SetBitmap(&fWebsiteIcon, SharedBitmap::SIZE_16); _SetContactInfo(fWebsiteLinkView, package.Publisher().Website()); - const BBitmap* screenshot = NULL; + bool hasScreenshot = false; const BitmapList& screenShots = package.Screenshots(); if (screenShots.CountItems() > 0) { const BitmapRef& bitmapRef = screenShots.ItemAtFast(0); - if (bitmapRef.Get() != NULL) - screenshot = bitmapRef->Bitmap(SharedBitmap::SIZE_ANY); + if (bitmapRef.Get() != NULL) { + hasScreenshot = true; + fScreenshotView->SetBitmap(bitmapRef); + } } - fScreenshotView->SetBitmap(screenshot); - fScreenshotView->SetEnabled(screenshot != NULL); + + if (!hasScreenshot) + fScreenshotView->UnsetBitmap(); + + fScreenshotView->SetEnabled(hasScreenshot); } void Clear() { fDescriptionView->SetText(""); - fEmailIconView->SetBitmap(NULL); + fEmailIconView->UnsetBitmap(); fEmailLinkView->SetText(""); - fWebsiteIconView->SetBitmap(NULL); + fWebsiteIconView->UnsetBitmap(); fWebsiteLinkView->SetText(""); - fScreenshotView->SetBitmap(NULL); + fScreenshotView->UnsetBitmap(); fScreenshotView->SetEnabled(false); } @@ -867,8 +872,8 @@ public: fAvatarView = new BitmapView("avatar view"); if (rating.User().Avatar().Get() != NULL) { - fAvatarView->SetBitmap( - rating.User().Avatar()->Bitmap(SharedBitmap::SIZE_16)); + fAvatarView->SetBitmap(rating.User().Avatar(), + SharedBitmap::SIZE_16); } fAvatarView->SetExplicitMinSize(BSize(16.0f, 16.0f)); @@ -908,10 +913,8 @@ public: // fVoteDownIconView = new BitmapButton("vote down icon", voteDownMessage); // fDownVoteCountView = new BStringView("up vote count", ""); // -// fVoteUpIconView->SetBitmap( -// voteUpIcon->Bitmap(SharedBitmap::SIZE_16)); -// fVoteDownIconView->SetBitmap( -// voteDownIcon->Bitmap(SharedBitmap::SIZE_16)); +// fVoteUpIconView->SetBitmap(voteUpIcon, SharedBitmap::SIZE_16); +// fVoteDownIconView->SetBitmap(voteDownIcon, SharedBitmap::SIZE_16); // // fUpVoteCountView->SetFont(&versionFont); // fUpVoteCountView->SetHighColor(kLightBlack); diff --git a/src/apps/haikudepot/ui/ScreenshotWindow.cpp b/src/apps/haikudepot/ui/ScreenshotWindow.cpp index 175e132327..5e6eb7caa2 100644 --- a/src/apps/haikudepot/ui/ScreenshotWindow.cpp +++ b/src/apps/haikudepot/ui/ScreenshotWindow.cpp @@ -175,8 +175,8 @@ ScreenshotWindow::_DownloadThread() return; } - fScreenshotView->SetBitmap(NULL); - + fScreenshotView->UnsetBitmap(); + ScreenshotInfoList screenshotInfos; if (fPackage.Get() != NULL) screenshotInfos = fPackage->ScreenshotInfos(); @@ -202,7 +202,7 @@ ScreenshotWindow::_DownloadThread() if (status == B_OK && Lock()) { printf("got screenshot"); fScreenshot = BitmapRef(new(std::nothrow)SharedBitmap(buffer), true); - fScreenshotView->SetBitmap(fScreenshot->Bitmap(SharedBitmap::SIZE_ANY)); + fScreenshotView->SetBitmap(fScreenshot); _ResizeToFitAndCenter(); Unlock(); } else { diff --git a/src/apps/haikudepot/ui/UserLoginWindow.cpp b/src/apps/haikudepot/ui/UserLoginWindow.cpp index 8836922b75..89cfd85ee7 100644 --- a/src/apps/haikudepot/ui/UserLoginWindow.cpp +++ b/src/apps/haikudepot/ui/UserLoginWindow.cpp @@ -235,10 +235,9 @@ UserLoginWindow::MessageReceived(BMessage* message) case MSG_CAPTCHA_OBTAINED: if (fCaptchaImage.Get() != NULL) { - fCaptchaView->SetBitmap( - fCaptchaImage->Bitmap(SharedBitmap::SIZE_ANY)); + fCaptchaView->SetBitmap(fCaptchaImage); } else { - fCaptchaView->SetBitmap(NULL); + fCaptchaView->UnsetBitmap(); } fCaptchaResultField->SetText(""); break; @@ -442,7 +441,7 @@ UserLoginWindow::_RequestCaptcha() { if (Lock()) { fCaptchaToken = ""; - fCaptchaView->SetBitmap(NULL); + fCaptchaView->UnsetBitmap(); fCaptchaImage.Unset(); Unlock(); } diff --git a/src/apps/haikudepot/ui_generic/BitmapView.cpp b/src/apps/haikudepot/ui_generic/BitmapView.cpp index 0c7f77eca8..0bb85976ed 100644 --- a/src/apps/haikudepot/ui_generic/BitmapView.cpp +++ b/src/apps/haikudepot/ui_generic/BitmapView.cpp @@ -131,14 +131,16 @@ BitmapView::MaxSize() void -BitmapView::SetBitmap(const BBitmap* bitmap) +BitmapView::SetBitmap(SharedBitmap* bitmap, SharedBitmap::Size bitmapSize) { - if (bitmap == fBitmap) + if (bitmap == fReference && bitmapSize == fBitmapSize) return; BSize size = MinSize(); - fBitmap = bitmap; + fReference.SetTo(bitmap); + fBitmapSize = bitmapSize; + fBitmap = bitmap->Bitmap(bitmapSize); BSize newSize = MinSize(); if (size != newSize) @@ -148,6 +150,20 @@ BitmapView::SetBitmap(const BBitmap* bitmap) } +void +BitmapView::UnsetBitmap() +{ + if (fReference.Get() == NULL) + return; + + fBitmap = NULL; + fReference.Unset(); + + InvalidateLayout(); + Invalidate(); +} + + void BitmapView::SetScaleBitmap(bool scaleBitmap) { diff --git a/src/apps/haikudepot/ui_generic/BitmapView.h b/src/apps/haikudepot/ui_generic/BitmapView.h index ba7ddc54bd..327f35fa14 100644 --- a/src/apps/haikudepot/ui_generic/BitmapView.h +++ b/src/apps/haikudepot/ui_generic/BitmapView.h @@ -6,6 +6,8 @@ #define BITMAP_VIEW_H +#include "SharedBitmap.h" + #include @@ -22,7 +24,10 @@ public: virtual BSize PreferredSize(); virtual BSize MaxSize(); - void SetBitmap(const BBitmap* bitmap); + void SetBitmap(SharedBitmap* bitmap, + SharedBitmap::Size bitmapSize + = SharedBitmap::SIZE_ANY); + void UnsetBitmap(); void SetScaleBitmap(bool scaleBitmap); protected: @@ -30,6 +35,8 @@ protected: BRect updateRect); private: + BitmapRef fReference; + SharedBitmap::Size fBitmapSize; const BBitmap* fBitmap; bool fScaleBitmap; };