Summary: | Add type-checked casts for StyleImage and subclasses | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||
Component: | CSS | Assignee: | 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
David Kilzer (:ddkilzer)
2014-02-17 09:38:40 PST
Created attachment 224390 [details]
Patch v1
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. 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. Comment on attachment 224390 [details] Patch v1 Clearing flags on attachment: 224390 Committed r164298: <http://trac.webkit.org/changeset/164298> All reviewed patches have been landed. Closing bug. Addressed style issues in Comment #3 in: Committed r164310: <http://trac.webkit.org/changeset/164310> |