Bug 52162

<rdar://problem/10042838>

Created attachment 112832 [details] parse -webkit-cross-fade

Comment on attachment 112832 [details] parse -webkit-cross-fade View in context: https://bugs.webkit.org/attachment.cgi?id=112832&action=review > Source/WebCore/css/CSSCrossfadeValue.cpp:29 > +#include "RenderObject.h" It looks like a forward declaration of RenderObject would suffice here. > Source/WebCore/css/CSSCrossfadeValue.cpp:35 > + No need for this empty line. > Source/WebCore/css/CSSCrossfadeValue.cpp:41 > + // TODO: what do we show if these haven't been set yet!? WebKit style is to use “FIXME” in comments, not “TODO”. > Source/WebCore/css/CSSCrossfadeValue.cpp:61 > + UNUSED_PARAM(size); > + > + if (size.isEmpty()) size is not unused after all… > Source/WebCore/css/CSSCrossfadeValue.cpp:64 > + return 0; But maybe it should be? > Source/WebCore/css/CSSCrossfadeValue.h:46 > + virtual String cssText() const; > + > + virtual PassRefPtr<Image> image(RenderObject*, const IntSize&); > + virtual bool isFixedSize() const { return false; } > + virtual IntSize fixedSize(const RenderObject*); These should can all be marked OVERRIDE. > Source/WebCore/css/CSSParser.h:94 > + bool parseFillImage(CSSParserValueList *, RefPtr<CSSValue>&); Extra space before the star.

Comment on attachment 112832 [details] parse -webkit-cross-fade View in context: https://bugs.webkit.org/attachment.cgi?id=112832&action=review Also, patch doesn't apply. > LayoutTests/fast/css/getComputedStyle/computed-style-cross-fade-expected.txt:16 > +PASS testCrossfade("background-image: -webkit-cross-fade(20%, url(http://example.com/a.png), url(http://example.com/b.png))", "background-image") is "none" > +PASS successfullyParsed is true You should test -webkit-cross-fade(url(..),) -webkit-cross-fade(,) -webkit-cross-fade(,url(...)) as well. > Source/WebCore/css/CSSCrossfadeValue.h:50 > + void setFromImage(const PassRefPtr<CSSImageValue> fromImage) { m_fromImage = fromImage; } > + void setToImage(const PassRefPtr<CSSImageValue> toImage) { m_toImage = toImage; } > + void setPercentage(const PassRefPtr<CSSPrimitiveValue> percentage) { m_percentage = percentage; } No need for the consts. Perhaps you should just pass the two CSSImageValues into the create() method and ctor? > Source/WebCore/css/CSSParser.cpp:2791 > +bool CSSParser::parseFillImage(CSSParserValueList * valueList, RefPtr<CSSValue>& value) No space before the * for any of these. > Source/WebCore/css/CSSParser.cpp:6195 > + RefPtr<CSSCrossfadeValue> result = CSSCrossfadeValue::create(); Might as well delay this until you actually need it. > Source/WebCore/css/CSSParser.cpp:6229 > + if (!a || a->unit != CSSPrimitiveValue::CSS_PERCENTAGE) > + return false; Should we allow fractions here ("0.4")?

Created attachment 112928 [details] address changes The only thing (as far as I can see) that this patch doesn't address is Simon's last question about fractions. To which I say: I don't know! Should we? The spec says "<percentage>" but I don't know what that actually means.

Parsing part landed in r98775.

Fixed the expected result in http://trac.webkit.org/changeset/98797.

Created attachment 113523 [details] [preliminary] render cross-fade Concerns: 1. loader/cache/CachedResourceClient.h -- multiple inheritance of two things which inherit from CachedResourceClient seems not to work. I'll need some assistance there from someone who knows the finer points of C++.

Created attachment 113676 [details] [preliminary] render cross-fade - should apply now

Attachment 113676 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/WebCore.xcodeproj/project.p..." exit_code: 1 Source/WebCore/css/CSSCrossfadeValue.cpp:32: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/css/CSSCrossfadeValue.cpp:35: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/graphics/CrossfadeGeneratedImage.cpp:32: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/graphics/CrossfadeGeneratedImage.cpp:63: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/platform/graphics/CrossfadeGeneratedImage.cpp:67: Missing space before ( in if( [whitespace/parens] [5] Source/WebCore/platform/graphics/GeneratorGeneratedImage.h:31: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/graphics/GeneratorGeneratedImage.h:32: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/graphics/GeneratorGeneratedImage.h:44: Missing space inside { }. [whitespace/braces] [5] Source/WebCore/css/CSSCrossfadeValue.h:57: Missing space inside { }. [whitespace/braces] [5] Source/WebCore/css/CSSCrossfadeValue.h:58: Missing space inside { }. [whitespace/braces] [5] Source/WebCore/css/CSSCrossfadeValue.h:59: Missing space inside { }. [whitespace/braces] [5] Source/WebCore/platform/graphics/CrossfadeGeneratedImage.h:31: Alphabetical sorting problem. [build/include_order] [4] Source/WebCore/platform/graphics/CrossfadeGeneratedImage.h:53: The parameter name "observer" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/CrossfadeGeneratedImage.h:53: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 14 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 113676 [details] [preliminary] render cross-fade - should apply now Attachment 113676 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/10336017

Comment on attachment 113676 [details] [preliminary] render cross-fade - should apply now Attachment 113676 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10333020

Comment on attachment 113676 [details] [preliminary] render cross-fade - should apply now Attachment 113676 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10333022

Oh, the other build systems again. Someday I'll remember that without being reminded.

Comment on attachment 113676 [details] [preliminary] render cross-fade - should apply now Attachment 113676 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/10334021

Created attachment 114611 [details] [preliminary] render cross-fade I was hoping that this would be in a commit-able state today, but then I got to writing tests. For some reason, the images don't render in DRT, even though I followed the code through the debugger and can see that the images are valid and the rendering code is being called. This will require further investigation...

Do you need to wait for the image load events before testing?

(In reply to comment #17) > Do you need to wait for the image load events before testing? Maybe? I tried data-urls too, which other people seem to use without waiting for any sort of load events, and they don't help any. (I'd also somewhat expect DRT to wait for images to finish loading before spitting out an image, but maybe not). The ones that use -webkit-canvas inputs do work, though, so it's definitely related to image loading. Making gratuitous use of layoutTestController.display() doesn't help like I expected it to, either (not that I was going to keep it that way, just for trying to see *something*).

Created attachment 115002 [details] Render Cross-fade Have at it! I have a feeling this is going to need a good bit of review.

Comment on attachment 115002 [details] Render Cross-fade Attachment 115002 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10484022

Created attachment 115011 [details] Render Cross-Fade (plus build system changes for other ports)

Comment on attachment 115011 [details] Render Cross-Fade (plus build system changes for other ports) Attachment 115011 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10487026

Comment on attachment 115011 [details] Render Cross-Fade (plus build system changes for other ports) Attachment 115011 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/10485041

Comment on attachment 115011 [details] Render Cross-Fade (plus build system changes for other ports) Attachment 115011 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/10487042

Created attachment 115025 [details] Render Cross-Fade (plus build fix for non-Mac)

Comment on attachment 115025 [details] Render Cross-Fade (plus build fix for non-Mac) Attachment 115025 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10372237

Created attachment 115027 [details] Render Cross-Fade (plus build fix for non-debug) Perhaps I should get out my VMs.

Comment on attachment 115027 [details] Render Cross-Fade (plus build fix for non-debug) View in context: https://bugs.webkit.org/attachment.cgi?id=115027&action=review This is a good start! I'd like to see a test case with a tiled cross-fade image. > LayoutTests/css3/images/cross-fade-invalidation.html:16 > + if(window.layoutTestController) > + layoutTestController.notifyDone(); Need to indent there > Source/WebCore/css/CSSCrossfadeValue.cpp:71 > + if (value->isImageValue()) > + return static_cast<CSSImageValue*>(value.get())->cachedOrPendingImage()->isPendingImage(); > + > + if (value->isImageGeneratorValue()) > + return static_cast<CSSImageGeneratorValue*>(value.get())->isPending(); These two bare functions look like candidates for a virtual method. > Source/WebCore/css/CSSCrossfadeValue.cpp:117 > + CachedImage* fromImage, * toImage; We don't declare multiple variables on one line like this. > Source/WebCore/css/CSSCrossfadeValue.cpp:120 > + fromImage = cachedImageForCSSValue(m_fromImage, renderer, size); > + toImage = cachedImageForCSSValue(m_toImage, renderer, size); Initialize at declaration! > Source/WebCore/css/CSSCrossfadeValue.h:42 > + static PassRefPtr<CSSCrossfadeValue> create(PassRefPtr<CSSValue> fromImage, PassRefPtr<CSSValue> toImage) { return adoptRef(new CSSCrossfadeValue(fromImage, toImage)); } Break this onto 3 lines. > Source/WebCore/css/CSSCrossfadeValue.h:43 > + ~CSSCrossfadeValue() { } You can remove this. > Source/WebCore/css/CSSImageGeneratorValue.cpp:117 > + if (isPending()) > + m_image = StylePendingImage::create(this).get(); It seems odd that calling this method has the side-effect of changing m_image. > Source/WebCore/css/CSSImageGeneratorValue.cpp:198 > + case CrossfadeClass: > + return static_cast<const CSSCrossfadeValue*>(this)->isPending(); > + case CanvasClass: > + return static_cast<const CSSCanvasValue*>(this)->isPending(); > + case LinearGradientClass: > + return static_cast<const CSSLinearGradientValue*>(this)->isPending(); > + case RadialGradientClass: > + return static_cast<const CSSRadialGradientValue*>(this)->isPending(); > + default: Erm, why not use virtual methods? > Source/WebCore/platform/graphics/CrossfadeGeneratedImage.cpp:45 > + m_size = size; Does the superclass not have a ctor that sets the size? > Source/WebCore/platform/graphics/CrossfadeGeneratedImage.cpp:64 > + float inversePercentage = 1.0f - m_percentage; Use 1, not 1.0f > Source/WebCore/platform/graphics/CrossfadeGeneratedImage.cpp:69 > + fromImage = m_fromImage->image(); > + fromImageSize = fromImage->size(); > + toImage = m_toImage->image(); > + toImageSize = toImage->size(); Declare at first use. This is C++, not C :) > Source/WebCore/platform/graphics/CrossfadeGeneratedImage.cpp:76 > + crossfadeImageSize = IntSize(fromImageSize.width() * inversePercentage + toImageSize.width() * m_percentage, > + fromImageSize.height() * inversePercentage + toImageSize.height() * m_percentage); Does the spec say anything about interpolating sizes? I'm not sure it's correct to animate the size too. > Source/WebCore/platform/graphics/CrossfadeGeneratedImage.h:73 > + : m_ownerValue(ownerValue) > + , m_ready(false) { } These should be indented. > Source/WebCore/platform/graphics/CrossfadeGeneratedImage.h:76 > + virtual void imageChanged(CachedImage* image, const IntRect* rect = 0) OVERRIDE { { on new line.

(In reply to comment #28) > (From update of attachment 115027 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=115027&action=review > > Source/WebCore/platform/graphics/CrossfadeGeneratedImage.cpp:76 > > + crossfadeImageSize = IntSize(fromImageSize.width() * inversePercentage + toImageSize.width() * m_percentage, > > + fromImageSize.height() * inversePercentage + toImageSize.height() * m_percentage); > > Does the spec say anything about interpolating sizes? I'm not sure it's correct to animate the size too. Yep, that's what it says to do with the size.

(In reply to comment #29) > Yep, that's what it says to do with the size. More precisely, given ''cross-fade(A,B,p)'', where A and B are images and p is a percentage between 0% and 100%, the function represents an image with width equal to widthA × (1-p) + widthB × p and height equal to heightA × (1-p) + heightB × p. The contents of the image must be constructed by first scaling A and B to the size of the generated image, then applying dissolve(A,1-p) plus dissolve(B,p).

As discussed on IRC, we can't use SrcOver compositing to do this effect. We need additive blending.

(In reply to comment #31) > As discussed on IRC, we can't use SrcOver compositing to do this effect. We need additive blending. I wonder (based on our conversation before)... is it not equivalent to paint opaque white behind the images and then continue on exactly as-is?

(In reply to comment #32) > (In reply to comment #31) > > As discussed on IRC, we can't use SrcOver compositing to do this effect. We need additive blending. > > I wonder (based on our conversation before)... is it not equivalent to paint opaque white behind the images and then continue on exactly as-is? Of course not, in the case of images with alpha. Nevermind.

Created attachment 115275 [details] Render Cross-Fade

(In reply to comment #28) > (From update of attachment 115027 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=115027&action=review > > This is a good start! I'd like to see a test case with a tiled cross-fade image. I've made one in -simple and one in -tiled do actual tiling. This was an excellent call, because it was actually broken. Fixed. > > Source/WebCore/css/CSSCrossfadeValue.cpp:71 > > + if (value->isImageValue()) > > + return static_cast<CSSImageValue*>(value.get())->cachedOrPendingImage()->isPendingImage(); > > + > > + if (value->isImageGeneratorValue()) > > + return static_cast<CSSImageGeneratorValue*>(value.get())->isPending(); > > These two bare functions look like candidates for a virtual method. The only common ancestor that CSSImageValue and CSSImageGeneratorValue share is CSSValue, not an ideal place for this. We could add another class here in between and make these virtual, but it's unclear that there's a point, with the devirtualization efforts that seem to be going on. > > Source/WebCore/css/CSSImageGeneratorValue.cpp:117 > > + if (isPending()) > > + m_image = StylePendingImage::create(this).get(); > > It seems odd that calling this method has the side-effect of changing m_image. That's how generatedImage() worked before, I believe. It is a little strange though; I'm open to suggestions! > > Source/WebCore/platform/graphics/CrossfadeGeneratedImage.cpp:45 > > + m_size = size; > > Does the superclass not have a ctor that sets the size? The superclass doesn't have any constructors. Should it? Can you not initialize members of the superclass in initializer lists? (it seems that you can't) > > Source/WebCore/platform/graphics/CrossfadeGeneratedImage.cpp:69 > > + fromImage = m_fromImage->image(); > > + fromImageSize = fromImage->size(); > > + toImage = m_toImage->image(); > > + toImageSize = toImage->size(); > > Declare at first use. This is C++, not C :) My background shining through! (In reply to comment #31) > As discussed on IRC, we can't use SrcOver compositing to do this effect. We need additive blending. Fixed, we now use a transparency layer and CompositePlusLighter.

>> Source/WebCore/platform/graphics/CrossfadeGeneratedImage.h:76 >> + virtual void imageChanged(CachedImage* image, const IntRect* rect = 0) OVERRIDE { > > { on new line. Style bot is red because I made this change, so ignore it, I guess. Unless it's right?

+levin: Maybe a style-checker bug?

Comment on attachment 115275 [details] Render Cross-Fade Attachment 115275 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10487536 New failing tests: css3/images/cross-fade-simple.html css3/images/cross-fade-invalidation.html fast/css/getComputedStyle/computed-style-cross-fade.html css3/images/cross-fade-tiled.html css3/images/cross-fade-blending.html css3/images/cross-fade-sizing.html

(In reply to comment #37) > +levin: Maybe a style-checker bug? A style checker bug, but it shouldn't be hit often and shouldn't be hit in this case (because it is only hit if someone is inlining a virtual function). (In reply to comment #36) > >> Source/WebCore/platform/graphics/CrossfadeGeneratedImage.h:76 > >> + virtual void imageChanged(CachedImage* image, const IntRect* rect = 0) OVERRIDE { > > > > { on new line. > > Style bot is red because I made this change, so ignore it, I guess. Unless it's right? The code is defining an inline function but since it is virtual and never called directly, the function will never be inlined, so it doesn't make sense to write it this way. In the past, I've been told (by ap) not to write functions inline in WebKit if they aren't meant to be inlined. If this function isn't inlined, the style checker will no longer complain.

Created attachment 115306 [details] Render Cross-Fade (and allow fractional cross-fade amounts, and clamp them to 0-1)

Created attachment 115308 [details] Render Cross-Fade (Windows build fix, hopefully)

Comment on attachment 115308 [details] Render Cross-Fade (Windows build fix, hopefully) Attachment 115308 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10372836 New failing tests: css3/images/cross-fade-tiled.html css3/images/cross-fade-invalidation.html css3/images/cross-fade-blending.html css3/images/cross-fade-simple.html css3/images/cross-fade-sizing.html

Comment on attachment 115308 [details] Render Cross-Fade (Windows build fix, hopefully) View in context: https://bugs.webkit.org/attachment.cgi?id=115308&action=review > LayoutTests/css3/images/cross-fade-tiled.html:9 > + -webkit-transform:translateZ(0); Why the translateZ()? > Source/WebCore/css/CSSCrossfadeValue.cpp:42 > +CachedImage* cachedImageForCSSValue(PassRefPtr<CSSValue>, const RenderObject*, const IntSize&); > +void loadSubimage(PassRefPtr<CSSValue>, CachedResourceLoader*); > +bool subimageIsPending(PassRefPtr<CSSValue>); You could just move the functions to the stop of the file and make them static. > Source/WebCore/css/CSSCrossfadeValue.cpp:62 > + IntSize size; > + > + CachedImage* fromImage = cachedImageForCSSValue(m_fromImage, renderer, size); > + CachedImage* toImage = cachedImageForCSSValue(m_toImage, renderer, size); Seems weird to send in an empty size here. > Source/WebCore/css/CSSCrossfadeValue.cpp:85 > +bool subimageIsPending(PassRefPtr<CSSValue> value) You should only use PassRefPtr<CSSValue> if you're passing ownership, which doesn't seem to be the case here. Can you pass a const CSSValue*? > Source/WebCore/css/CSSCrossfadeValue.cpp:98 > +void loadSubimage(PassRefPtr<CSSValue> value, CachedResourceLoader* cachedResourceLoader) Ditto. > Source/WebCore/css/CSSCrossfadeValue.cpp:113 > +CachedImage* cachedImageForCSSValue(PassRefPtr<CSSValue> value, const RenderObject* renderer, const IntSize& size) Ditto. > Source/WebCore/css/CSSCrossfadeValue.cpp:115 > UNUSED_PARAM(size); Are you going to use size? > Source/WebCore/css/CSSCrossfadeValue.h:65 > + // NOTE: We put the ImageObserver in a member instead of inheriting from it No need for "NOTE:". > Source/WebCore/platform/graphics/CrossfadeGeneratedImage.cpp:76 > + context->beginTransparencyLayer(1.0); 1.0 -> 1 > Source/WebCore/platform/graphics/CrossfadeGeneratedImage.h:62 > + CachedImage* m_fromImage; > + CachedImage* m_toImage; I'd like to see a comment about who owns these CachedImages.

(In reply to comment #39) > (In reply to comment #37) > > +levin: Maybe a style-checker bug? > A style checker bug, but it shouldn't be hit often. fwiw, I did fix the style checker bug as well (even though I don't think it should appear hardly ever... but if someone inlines a virtual function, I don't want to give them an obtuse error. If we want to flag it, the error should be more specific.) https://bugs.webkit.org/show_bug.cgi?id=72515

Landed in r100535, with Simon's changes.

It broke fast/css/getComputedStyle/computed-style-cross-fade.html on the Qt bot: (In reply to comment #45) > Landed in r100535, with Simon's changes. --- /ramdisk/qt-linux-64-release/build/layout-test-results/fast/css/getComputedStyle/computed-style-cross-fade-expected.txt +++ /ramdisk/qt-linux-64-release/build/layout-test-results/fast/css/getComputedStyle/computed-style-cross-fade-actual.txt @@ -1,22 +1,7 @@ -Blocked access to external URL http://example.com/example.png Blocked access to external URL http://example.com/example.png Blocked access to external URL http://example.com/a.png Blocked access to external URL http://example.com/b.png -Blocked access to external URL http://example.com/example.png Blocked access to external URL http://example.com/c.png -Blocked access to external URL http://example.com/a.png -Blocked access to external URL http://example.com/b.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -webkit-cross-fade

It fails on GTK with similar diff: --- /var/lib/buildbot/build/gtk-linux-32-release/build/layout-test-results/fast/css/getComputedStyle/computed-style-cross-fade-expected.txt +++ /var/lib/buildbot/build/gtk-linux-32-release/build/layout-test-results/fast/css/getComputedStyle/computed-style-cross-fade-actual.txt @@ -1,22 +1,11 @@ -Blocked access to external URL http://example.com/example.png Blocked access to external URL http://example.com/example.png Blocked access to external URL http://example.com/a.png Blocked access to external URL http://example.com/b.png -Blocked access to external URL http://example.com/example.png Blocked access to external URL http://example.com/c.png -Blocked access to external URL http://example.com/a.png -Blocked access to external URL http://example.com/b.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png -Blocked access to external URL http://example.com/example.png +Blocked access to external URL http://www.iana.org/domains/example/ +Blocked access to external URL http://www.iana.org/domains/example/ +Blocked access to external URL http://www.iana.org/domains/example/ +Blocked access to external URL http://www.iana.org/domains/example/ -webkit-cross-fade

I skipped fast/css/getComputedStyle/computed-style-cross-fade.html on Qt temporarily to make buildbot happier: http://trac.webkit.org/changeset/100633 . Please unskip it when a proper fix landing.

(In reply to comment #47) > It fails on GTK with similar diff: > > --- /var/lib/buildbot/build/gtk-linux-32-release/build/layout-test-results/fast/css/getComputedStyle/computed-style-cross-fade-expected.txt > +++ /var/lib/buildbot/build/gtk-linux-32-release/build/layout-test-results/fast/css/getComputedStyle/computed-style-cross-fade-actual.txt > @@ -1,22 +1,11 @@ > -Blocked access to external URL http://example.com/example.png Oh boy; I was hoping those wouldn't be a problem (would be consistent) but I was wrong. What do we use for images? I can't use anything on the local filesystem, as it resolves it to an absolute path which will certainly be broken on many systems. It looks like some of the other tests use "dummy://test.png" -- is that what I should be using? I'll try it out.

Created attachment 115659 [details] Fix the getComputedStyle test to use dummy://

Created attachment 115678 [details] Fix layering violation

Comment on attachment 115659 [details] Fix the getComputedStyle test to use dummy:// Clearing flags on attachment: 115659 Committed r100687: <http://trac.webkit.org/changeset/100687>

Comment on attachment 115678 [details] Fix layering violation View in context: https://bugs.webkit.org/attachment.cgi?id=115678&action=review > Source/WebCore/css/CSSCrossfadeValue.cpp:58 > + > + if (!styleCachedImage) I'd omit the blank line here. > Source/WebCore/css/CSSCrossfadeValue.cpp:158 > + UNUSED_PARAM(image); You could just omit the parameter name instead.

I just noticed something weird about refreshing when the page is in the cache (looks like freeing something we shouldn't be, but it's not a crash, the image shows up as the nullImage tiled over the whole space), so I'm going to look at that tomorrow and not land this as-is.

Created attachment 117250 [details] fix layering violation, take 2 More thorough testing this time, and it seems to work perfectly.

Landed in r101546.

Not sure why this is still open.