As filed the bug is about table backgrounds failing to print. The root cause of this is that we load images from the original document, because print/print-preview documents (which are static clones of the original) cannot do loads. This results in an entry in the css::ImageValue's request table under the original document, but entry under the printing document. Then we do layout, try to get the request, and fail, and nothing is displayed. The solution to this is to force us to clone the request for the printing document if we're loading off the original document. I manually verified that this does not regress Bug 560235.
While writing a test for this, we discovered another problem. The reftest print code does not actually use the printing codepath. Instead it takes an existing document, tears down its presshell, and creates a new presshell for printing. Fixing the above bug did not make the reftest print test pass because ImageLoader does not deal properly with presshell destruction/recreation. It assumes that when the presshell is destroyed all of the css::ImageValues can be cleared of their entries for that document. This fails for ImageValues for mapped attributes because they are held alive by the content tree. When a new presshell for this document is then created there is no entry for the document and thus no image request to paint. The fix for this is to only clear the frame to request and request to frame mapping hashtables and not to clear the document's entry on the ImageValue when the presshell is destroyed. The destruction of the ImageLoader (which is held from a strong reference on the document) is when those entries are removed. The final change is to change css::ImageValue's hashtable from holding a strong reference to the keys (which are documents) to holding raw pointer references. This is safe because we clear the relevant entry when the corresponding ImageLoader dies, and is needed to prevent a reference cycle that was being broken by presshell destruction.
--HG--
rename : layout/reftests/backgrounds/table-background-ref.html => layout/reftests/backgrounds/table-background-print-ref.html
rename : layout/reftests/backgrounds/table-background.html => layout/reftests/backgrounds/table-background-print.html
Since this directory has FAIL_ON_WARNINGS set, this will cause any
shadowing warnings triggered by headers included in layout/style to
cause errors.
Once this patch is reviewed, I'll write and attach patches to fix the
warnings needed to land this.
Except for the changes in:
layout/generic/nsIFrame.h (part)
layout/style/nsComputedDOMStyle.h (all)
layout/style/nsRuleNode.cpp (part)
layout/style/nsStyleContext.cpp (part)
layout/style/nsStyleContext.h (part)
(see patch 3b in the bug), this patch was written with the sed script:
s/\<GetStyle\(Font\|Color\|List\|Text\|Visibility\|Quotes\|UserInterface\|TableBorder\|SVG\|Background\|Position\|TextReset\|Display\|Content\|UIReset\|Table\|Margin\|Padding\|Border\|Outline\|XUL\|SVGReset\|Column\)\>/Style\1/g
This makes it conform to our convention that getters returning pointers
that can never be null do not begin with "Get".
nsStyleContext's rule node is never null because we require a rule node
in order to construct a style context.
Note that this does not change the IID of nsIDOMMozCSSKeyframesRule
since neither the method signature nor semantics have changed; only the
name is different.
Our gfxPattern cache is currently way too specific, which causes lots of
unnecessary cache misses. The only thing that we actually need to key on is the
color stops.
This switches the cache to hold GradientStops instead of gfxPatterns. This
improves our cache hit rate and is simpler. It also avoids doing caching when
not using Azure, which currently has no benefit.
Average paint times results (in ms)
(on windows 7 x64, i7-3630qm, HD4000)
-------------------------------------
m-c
open: 3.5 close: 3.3
m-c cache-key
open: 2.6 close: 2.4
ux
open: 7.3 close: 5.2
ux cache-key
open: 6.9 close: 5.3
This implements the proposed spec clarification in
http://lists.w3.org/Archives/Public/www-style/2013Feb/0229.html which
makes us compatible with WebKit on the insertRule tests in this patch.
I confirmed that the test reports 7 failures without the patch, but
passes with the patch. (I'm a little disturbed by the way our
testharness.js integration elides runs of successive passes.)
No changes to the transition manager are needed since
nsTransitionManager::StyleContextChanged already makes the appropriate
calls to ConsiderStartingTransition for 'all', and that existing code
should work fine within a loop (just like the existing code to handle
shorthands or other property duplication works).
