mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-11-25 13:51:41 +00:00
Bug 1404422 - Part 1d. Ensure imgRequestProxy::PerformClone consistently adds the clone to the expected load group. r=tnikkel
Historically imgRequestProxy::PerformClone would only add the cloned request to the (original proxy's) document's load group if the request was still being validated. Now it adds the cloned request to the given document's load group before requesting the notifications, unless the request has already been completed. We ensure that any removals from the load group occur outside the current execution context. Legacy listeners may use imgRequestProxy::SyncClone to request notifications on the image state. Ideally they would not, but they do not work as expected with the asynchronous notifications all new callers must use. While in theory this would suggest their code is re-entrant, not all of it is. In particular we need to be sensitive about when we remove a request from a load group.
This commit is contained in:
parent
920629550f
commit
c57b395b1a
@ -828,6 +828,11 @@ imgRequestProxy::PerformClone(imgINotificationObserver* aObserver,
|
||||
*aClone = nullptr;
|
||||
RefPtr<imgRequestProxy> clone = NewClonedProxy();
|
||||
|
||||
nsCOMPtr<nsILoadGroup> loadGroup;
|
||||
if (aLoadingDocument) {
|
||||
loadGroup = aLoadingDocument->GetDocumentLoadGroup();
|
||||
}
|
||||
|
||||
// It is important to call |SetLoadFlags()| before calling |Init()| because
|
||||
// |Init()| adds the request to the loadgroup.
|
||||
// When a request is added to a loadgroup, its load flags are merged
|
||||
@ -835,7 +840,7 @@ imgRequestProxy::PerformClone(imgINotificationObserver* aObserver,
|
||||
// XXXldb That's not true anymore. Stuff from imgLoader adds the
|
||||
// request to the loadgroup.
|
||||
clone->SetLoadFlags(mLoadFlags);
|
||||
nsresult rv = clone->Init(mBehaviour->GetOwner(), mLoadGroup,
|
||||
nsresult rv = clone->Init(mBehaviour->GetOwner(), loadGroup,
|
||||
aLoadingDocument, mURI, aObserver);
|
||||
if (NS_FAILED(rv)) {
|
||||
return rv;
|
||||
@ -848,20 +853,44 @@ imgRequestProxy::PerformClone(imgINotificationObserver* aObserver,
|
||||
|
||||
if (GetOwner() && GetOwner()->GetValidator()) {
|
||||
// Note that if we have a validator, we don't want to issue notifications at
|
||||
// here because we want to defer until that completes.
|
||||
// here because we want to defer until that completes. AddProxy will add us
|
||||
// to the load group; we cannot avoid that in this case, because we don't
|
||||
// know when the validation will complete, and if it will cause us to
|
||||
// discard our cached state anyways. We are probably already blocked by the
|
||||
// original LoadImage(WithChannel) request in any event.
|
||||
clone->SetNotificationsDeferred(true);
|
||||
GetOwner()->GetValidator()->AddProxy(clone);
|
||||
} else if (aSyncNotify) {
|
||||
// This is wrong!!! We need to notify asynchronously, but there's code that
|
||||
// assumes that we don't. This will be fixed in bug 580466. Note that if we
|
||||
// have a validator, we won't issue notifications anyways because they are
|
||||
// deferred, so there is no point in requesting.
|
||||
clone->SyncNotifyListener();
|
||||
} else {
|
||||
// Without a validator, we can request asynchronous notifications
|
||||
// immediately. If there was a validator, this would override the deferral
|
||||
// and that would be incorrect.
|
||||
clone->NotifyListener();
|
||||
// We only want to add the request to the load group of the owning document
|
||||
// if it is still in progress. Some callers cannot handle a supurious load
|
||||
// group removal (e.g. print preview) so we must be careful. On the other
|
||||
// hand, if after cloning, the original request proxy is cancelled /
|
||||
// destroyed, we need to ensure that any clones still block the load group
|
||||
// if it is incomplete.
|
||||
bool addToLoadGroup = mIsInLoadGroup;
|
||||
if (!addToLoadGroup) {
|
||||
RefPtr<ProgressTracker> tracker = clone->GetProgressTracker();
|
||||
addToLoadGroup = tracker && !(tracker->GetProgress() & FLAG_LOAD_COMPLETE);
|
||||
}
|
||||
|
||||
if (addToLoadGroup) {
|
||||
clone->AddToLoadGroup();
|
||||
}
|
||||
|
||||
if (aSyncNotify) {
|
||||
// This is wrong!!! We need to notify asynchronously, but there's code
|
||||
// that assumes that we don't. This will be fixed in bug 580466. Note that
|
||||
// if we have a validator, we won't issue notifications anyways because
|
||||
// they are deferred, so there is no point in requesting.
|
||||
clone->mForceDispatchLoadGroup = true;
|
||||
clone->SyncNotifyListener();
|
||||
clone->mForceDispatchLoadGroup = false;
|
||||
} else {
|
||||
// Without a validator, we can request asynchronous notifications
|
||||
// immediately. If there was a validator, this would override the deferral
|
||||
// and that would be incorrect.
|
||||
clone->NotifyListener();
|
||||
}
|
||||
}
|
||||
|
||||
return NS_OK;
|
||||
|
Loading…
Reference in New Issue
Block a user