Bug 128915

Summary: Add type-checked casts for StyleImage and subclasses
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: CSSAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: allan.jensen, commit-queue, darin, ddkilzer, dino, dstockwell, esprehn+autocc, glenn, gyuyoung.kim, kling, koivisto, kondapallykalyan, macpherson, menard, simon.fraser, svillar
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 128671    
Bug Blocks:    
Attachments:
Description Flags
Patch v1 none

David Kilzer (:ddkilzer)
Reported 2014-02-17 09:38:40 PST
Add type-checked casts for StyleImage and its subclasses.
Attachments
Patch v1 (17.70 KB, patch)
2014-02-17 09:49 PST, David Kilzer (:ddkilzer)
no flags
David Kilzer (:ddkilzer)
Comment 1 2014-02-17 09:49:50 PST
Created attachment 224390 [details] Patch v1
Andreas Kling
Comment 2 2014-02-18 10:47:58 PST
Comment on attachment 224390 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=224390&action=review Sweet patch, just some stylistic comments. > Source/WebCore/css/CSSCursorImageValue.cpp:198 > + return toStyleCachedImage(m_image.get())->cachedImage()->url(); Since we're sure that m_image can't be null, I'd write this like so: return toStyleCachedImage(*m_image).cachedImage()->url(); > Source/WebCore/css/CSSImageSetValue.cpp:64 > + toStyleCachedImageSet(m_imageSet.get())->clearImageSetValue(); Same here. > Source/WebCore/css/CSSImageSetValue.cpp:194 > + CachedResource* cachedResource = toStyleCachedImageSet(m_imageSet.get())->cachedImage(); And here. > Source/WebCore/css/CSSImageValue.cpp:108 > + CachedResource* cachedResource = toStyleCachedImage(m_image.get())->cachedImage(); And here. > Source/WebCore/rendering/style/StyleImage.h:97 > + TYPE_CASTS_BASE(ToClassName, FromClassName, resource, resource->predicate(), resource.predicate()) Not that it matters much, but "resource" seems like a strange name for what's being passed around here.
Andreas Kling
Comment 3 2014-02-18 10:47:59 PST
Comment on attachment 224390 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=224390&action=review Sweet patch, just some stylistic comments. > Source/WebCore/css/CSSCursorImageValue.cpp:198 > + return toStyleCachedImage(m_image.get())->cachedImage()->url(); Since we're sure that m_image can't be null, I'd write this like so: return toStyleCachedImage(*m_image).cachedImage()->url(); > Source/WebCore/css/CSSImageSetValue.cpp:64 > + toStyleCachedImageSet(m_imageSet.get())->clearImageSetValue(); Same here. > Source/WebCore/css/CSSImageSetValue.cpp:194 > + CachedResource* cachedResource = toStyleCachedImageSet(m_imageSet.get())->cachedImage(); And here. > Source/WebCore/css/CSSImageValue.cpp:108 > + CachedResource* cachedResource = toStyleCachedImage(m_image.get())->cachedImage(); And here. > Source/WebCore/rendering/style/StyleImage.h:97 > + TYPE_CASTS_BASE(ToClassName, FromClassName, resource, resource->predicate(), resource.predicate()) Not that it matters much, but "resource" seems like a strange name for what's being passed around here.
WebKit Commit Bot
Comment 4 2014-02-18 11:05:25 PST
Comment on attachment 224390 [details] Patch v1 Clearing flags on attachment: 224390 Committed r164298: <http://trac.webkit.org/changeset/164298>
WebKit Commit Bot
Comment 5 2014-02-18 11:05:30 PST
All reviewed patches have been landed. Closing bug.
David Kilzer (:ddkilzer)
Comment 6 2014-02-18 13:22:12 PST
Addressed style issues in Comment #3 in: Committed r164310: <http://trac.webkit.org/changeset/164310>
Note You need to log in before you can comment on or make changes to this bug.