Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2025-02-05 05:30:29 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 816498: Fix some things about background attribute handling that are still broken. r=bz

Browse Source 
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
This commit is contained in:
Kyle Huey 2013-02-23 06:59:43 -08:00
parent 2990ef8378
commit 0660c18b2f
14 changed files with 147 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
layout/base/nsPresShell.cpp
View File

@ -1992,7 +1992,7 @@ PresShell::SetIgnoreFrameDestruction(bool aIgnore)
if (mDocument) {
// We need to tell the ImageLoader to drop all its references to frames
// because they're about to go away and it won't get notifications of that.
mDocument->StyleImageLoader()->ClearAll();
mDocument->StyleImageLoader()->ClearFrames();
}
mIgnoreFrameDestruction = aIgnore;
}

1
layout/reftests/backgrounds/reftest.list
View File

@ -133,6 +133,7 @@ random-if(bug685516) HTTP != root-background-1.html about:blank
random-if(bug685516||B2G) == really-big-background.html really-big-background-ref.html
random-if(bug685516) == body-background.html body-background-ref.html
random-if(bug685516) == table-background.html table-background-ref.html
random-if(bug685516) == table-background-print.html table-background-print-ref.html
random-if(bug685516) != div-background.html div-background-ref.html
random-if(bug685516) == background-repeat-1-ref.html background-repeat-1.html

41
layout/reftests/backgrounds/table-background-print-ref.html Normal file
View File

@ -0,0 +1,41 @@
<html class="reftest-print">
<body>
<table style="background-image: url('aqua-yellow-32x32.png')">
<thead style="background-image: url('blue-16x20-green-16x20.png')">
<tr>
<td>
Foo
</td>
<td style="background-image: url('yellow-32x32.png')">
Bar
</td>
</tr>
</thead>
<tbody style="background-image: url('red-32x32.png')">
<tr>
<th style="background-image: url('fuchsia-32x32.png')">
Foo
</th>
<th>
Bar
</th>
</tr>
<tr style="background-image: url('fuchsia-32x32.png')">
<td>
Foo
</td>
<td style="background-image: url('yellow-32x32.png')">
Bar
</td>
</tr>
</tbody>
<tfoot style="background-image: url('yellow-32x32.png')">
<tr>
<td>
Baz
</td>
</tr>
</tfoot>
</table>
</body>
</html>

41
layout/reftests/backgrounds/table-background-print.html Normal file
View File

@ -0,0 +1,41 @@
<html class="reftest-print">
<body>
<table background="aqua-yellow-32x32.png">
<thead background="blue-16x20-green-16x20.png">
<tr>
<td>
Foo
</td>
<td background="yellow-32x32.png">
Bar
</td>
</tr>
</thead>
<tbody background="red-32x32.png">
<tr>
<th background="fuchsia-32x32.png">
Foo
</th>
<th>
Bar
</th>
</tr>
<tr background="fuchsia-32x32.png">
<td>
Foo
</td>
<td background="yellow-32x32.png">
Bar
</td>
</tr>
</tbody>
<tfoot background="yellow-32x32.png">
<tr>
<td>
Baz
</td>
</tr>
</tfoot>
</table>
</body>
</html>

2
layout/reftests/backgrounds/table-background-ref.html
View File

@ -1,3 +1,4 @@
<html>
<body>
<table style="background-image: url('aqua-yellow-32x32.png')">
<thead style="background-image: url('blue-16x20-green-16x20.png')">
@ -37,3 +38,4 @@
</tfoot>
</table>
</body>
</html>

2
layout/reftests/backgrounds/table-background.html
View File

@ -1,3 +1,4 @@
<html>
<body>
<table background="aqua-yellow-32x32.png">
<thead background="blue-16x20-green-16x20.png">
@ -37,3 +38,4 @@
</tfoot>
</table>
</body>
</html>

6
layout/reftests/bugs/816948-1-ref.html Normal file
View File

@ -0,0 +1,6 @@
<html>
<body>
<iframe id="ifr" src="816948-iframe.html">
</iframe>
</body>
</html>

16
layout/reftests/bugs/816948-1.html Normal file
View File

@ -0,0 +1,16 @@
<html class="reftest-wait">
<script>
function boom() {
var ifr = document.getElementById("ifr");
ifr.style.display = "none";
// flush layout
document.documentElement.offsetLeft;
ifr.style.display = "block";
document.documentElement.removeAttribute("class");
}
</script>
<body onload="boom();">
<iframe id="ifr" src="816948-iframe.html">
</iframe>
</body>
</html>

3
layout/reftests/bugs/816948-iframe.html Normal file
View File

@ -0,0 +1,3 @@
<body background="mozilla-banner.gif">
foopy
</body>

1
layout/reftests/bugs/reftest.list
View File

@ -1739,6 +1739,7 @@ skip-if(B2G) == 815593-1.html 815593-1-ref.html
skip-if(B2G) == 814952-1.html 814952-1-ref.html
== 816458-1.html 816458-1-ref.html
== 816359-1.html 816359-1-ref.html
== 816948-1.html 816948-1-ref.html
== 817019-1.html about:blank
skip-if(B2G) == 818276-1.html 818276-1-ref.html
== 825999.html 825999-ref.html

38
layout/style/ImageLoader.cpp
View File

@ -43,10 +43,27 @@ ImageLoader::SetAnimationModeEnumerator(nsISupports* aKey, FrameSet* aValue,
return PL_DHASH_NEXT;
}
static PLDHashOperator
ClearImageHashSet(nsPtrHashKey<ImageLoader::Image>* aKey, void* aClosure)
{
nsIDocument* doc = static_cast<nsIDocument*>(aClosure);
ImageLoader::Image* image = aKey->GetKey();
imgIRequest* request = image->mRequests.GetWeak(doc);
if (request) {
request->CancelAndForgetObserver(NS_BINDING_ABORTED);
}
image->mRequests.Remove(doc);
return PL_DHASH_REMOVE;
}
void
ImageLoader::DropDocumentReference()
{
ClearAll();
ClearFrames();
mImages.EnumerateEntries(&ClearImageHashSet, mDocument);
mDocument = nullptr;
}
@ -221,28 +238,11 @@ ImageLoader::SetAnimationMode(uint16_t aMode)
mRequestToFrameMap.EnumerateRead(SetAnimationModeEnumerator, &aMode);
}
static PLDHashOperator
ClearImageHashSet(nsPtrHashKey<ImageLoader::Image>* aKey, void* aClosure)
{
nsIDocument* doc = static_cast<nsIDocument*>(aClosure);
ImageLoader::Image* image = aKey->GetKey();
imgIRequest* request = image->mRequests.GetWeak(doc);
if (request) {
request->CancelAndForgetObserver(NS_BINDING_ABORTED);
}
image->mRequests.Remove(doc);
return PL_DHASH_REMOVE;
}
void
ImageLoader::ClearAll()
ImageLoader::ClearFrames()
{
mRequestToFrameMap.Clear();
mFrameToRequestMap.Clear();
mImages.EnumerateEntries(&ClearImageHashSet, mDocument);
}
void

2
layout/style/ImageLoader.h
View File

@ -59,7 +59,7 @@ public:
void SetAnimationMode(uint16_t aMode);
void ClearAll();
void ClearFrames();
void LoadImage(nsIURI* aURI, nsIPrincipal* aPrincipal, nsIURI* aReferrer,
Image* aCSSValue);

16
layout/style/nsCSSValue.cpp
View File

@ -1721,14 +1721,22 @@ css::ImageValue::ImageValue(nsIURI* aURI, nsStringBuffer* aString,
nsIDocument* aDocument)
: URLValue(aURI, aString, aReferrer, aOriginPrincipal)
{
if (aDocument->GetOriginalDocument()) {
aDocument = aDocument->GetOriginalDocument();
// NB: If aDocument is not the original document, we may not be able to load
// images from aDocument. Instead we do the image load from the original doc
// and clone it to aDocument.
nsIDocument* loadingDoc = aDocument->GetOriginalDocument();
if (!loadingDoc) {
loadingDoc = aDocument;
}
mRequests.Init();
aDocument->StyleImageLoader()->LoadImage(aURI, aOriginPrincipal, aReferrer,
this);
loadingDoc->StyleImageLoader()->LoadImage(aURI, aOriginPrincipal, aReferrer,
this);
if (loadingDoc != aDocument) {
aDocument->StyleImageLoader()->MaybeRegisterCSSImage(this);
}
}
static PLDHashOperator

2
layout/style/nsCSSValue.h
View File

@ -125,7 +125,7 @@ struct ImageValue : public URLValue {
// Inherit operator== from URLValue
nsRefPtrHashtable<nsISupportsHashKey, imgRequestProxy> mRequests;
nsRefPtrHashtable<nsPtrHashKey<nsISupports>, imgRequestProxy> mRequests;
// Override AddRef and Release to not only log ourselves correctly, but
// also so that we delete correctly without a virtual destructor