Bug 1346501. Don't mark every image as visible when a frame is created for it

This commit is contained in:
janekptacijarabaci 2018-07-12 18:56:16 +02:00 committed by Roy Tam
commit ece6d716f4
4 changed files with 37 additions and 11 deletions

View file

@ -491,8 +491,8 @@ nsImageLoadingContent::FrameCreated(nsIFrame* aFrame)
mFrameCreateCalled = true;
TrackImage(mCurrentRequest);
TrackImage(mPendingRequest);
TrackImage(mCurrentRequest, aFrame);
TrackImage(mPendingRequest, aFrame);
// We need to make sure that our image request is registered, if it should
// be registered.
@ -1486,7 +1486,8 @@ nsImageLoadingContent::OnVisibilityChange(Visibility aNewVisibility,
}
void
nsImageLoadingContent::TrackImage(imgIRequest* aImage)
nsImageLoadingContent::TrackImage(imgIRequest* aImage,
nsIFrame* aFrame /*= nullptr */)
{
if (!aImage)
return;
@ -1499,13 +1500,21 @@ nsImageLoadingContent::TrackImage(imgIRequest* aImage)
return;
}
// We only want to track this request if we're visible. Ordinarily we check
// the visible count, but that requires a frame; in cases where
// GetOurPrimaryFrame() cannot obtain a frame (e.g. <feImage>), we assume
// we're visible if FrameCreated() was called.
nsIFrame* frame = GetOurPrimaryFrame();
if ((frame && frame->GetVisibility() == Visibility::APPROXIMATELY_NONVISIBLE) ||
(!frame && !mFrameCreateCalled)) {
if (!aFrame) {
aFrame = GetOurPrimaryFrame();
}
/* This line is deceptively simple. It hides a lot of subtlety. Before we
* create an nsImageFrame we call nsImageFrame::ShouldCreateImageFrameFor
* to determine if we should create an nsImageFrame or create a frame based
* on the display of the element (ie inline, block, etc). Inline, block, etc
* frames don't register for visibility tracking so they will return UNTRACKED
* from GetVisibility(). So this line is choosing to mark such images as
* visible. Once the image loads we will get an nsImageFrame and the proper
* visibility. This is a pitfall of tracking the visibility on the frames
* instead of the content node.
*/
if (!aFrame || aFrame->GetVisibility() == Visibility::APPROXIMATELY_NONVISIBLE) {
return;
}

View file

@ -364,6 +364,11 @@ protected:
*
* No-op if aImage is null.
*
* @param aFrame If called from FrameCreated the frame passed to FrameCreated.
* This is our frame, but at the time of the FrameCreated call
* our primary frame pointer hasn't been set yet, so this is
* only way to get our frame.
*
* @param aNonvisibleAction A requested action if the frame has become
* nonvisible. If Nothing(), no action is
* requested. If DISCARD_IMAGES is specified, the
@ -371,7 +376,7 @@ protected:
* associated with to discard their surfaces if
* possible.
*/
void TrackImage(imgIRequest* aImage);
void TrackImage(imgIRequest* aImage, nsIFrame* aFrame = nullptr);
void UntrackImage(imgIRequest* aImage,
const Maybe<OnNonvisible>& aNonvisibleAction = Nothing());

View file

@ -107,6 +107,12 @@ SVGFEImageFrame::Init(nsIContent* aContent,
nsFrame::Init(aContent, aParent, aPrevInFlow);
// We assume that feImage's are always visible.
// This call must happen before the FrameCreated. This is because the
// primary frame pointer on our content node isn't set until after this
// function ends, so there is no way for the resulting OnVisibilityChange
// notification to get a frame. FrameCreated has a workaround for this in
// that it passes our frame around so it can be accessed. OnVisibilityChange
// doesn't have that workaround.
IncApproximateVisibleCount();
nsCOMPtr<nsIImageLoadingContent> imageLoader =

View file

@ -159,6 +159,12 @@ nsSVGImageFrame::Init(nsIContent* aContent,
if (GetStateBits() & NS_FRAME_IS_NONDISPLAY) {
// Non-display frames are likely to be patterns, masks or the like.
// Treat them as always visible.
// This call must happen before the FrameCreated. This is because the
// primary frame pointer on our content node isn't set until after this
// function ends, so there is no way for the resulting OnVisibilityChange
// notification to get a frame. FrameCreated has a workaround for this in
// that it passes our frame around so it can be accessed. OnVisibilityChange
// doesn't have that workaround.
IncApproximateVisibleCount();
}