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

Bug 1383682 - Part 1. Split off imgRequestProxy notification deferrals for validation. r=tnikkel

Browse Source 
When cache validation is in progress, imgRequestProxy defers its
notifications to its listener until the validation is complete. This is
because the cache may be discarded, and the current state will change.
It attempted to share the same flags with notification deferrals used by
ProgressTracker to indicate that there is a pending notification, but
this has problematic/confusing. Hence this patch creates dedicated flags
for notification deferrals due to cache validation.
This commit is contained in:
Andrew Osmond 2018-02-07 07:27:27 -05:00
parent a4c0f8161c
commit dab9b6216c
5 changed files with 127 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
image/ProgressTracker.cpp
View File

@ -171,6 +171,11 @@ ProgressTracker::Notify(IProgressObserver* aObserver)
{
MOZ_ASSERT(NS_IsMainThread());
if (aObserver->NotificationsDeferred()) {
// There is a pending notification, or the observer isn't ready yet.
return;
}
if (MOZ_LOG_TEST(gImgLog, LogLevel::Debug)) {
RefPtr<Image> image = GetImage();
if (image && image->GetURI()) {
@ -241,6 +246,11 @@ ProgressTracker::NotifyCurrentState(IProgressObserver* aObserver)
{
MOZ_ASSERT(NS_IsMainThread());
if (aObserver->NotificationsDeferred()) {
// There is a pending notification, or the observer isn't ready yet.
return;
}
if (MOZ_LOG_TEST(gImgLog, LogLevel::Debug)) {
RefPtr<Image> image = GetImage();
nsAutoCString spec;

73
image/imgLoader.cpp
View File

@ -1768,7 +1768,7 @@ imgLoader::ValidateRequestWithNewChannel(imgRequest* request,
// In the mean time, we must defer notifications because we are added to
// the imgRequest's proxy list, and we can get extra notifications
// resulting from methods such as StartDecoding(). See bug 579122.
proxy->SetNotificationsDeferred(true);
proxy->MarkValidating();
// Attach the proxy without notifying
request->GetValidator()->AddProxy(proxy);
@ -1834,7 +1834,7 @@ imgLoader::ValidateRequestWithNewChannel(imgRequest* request,
// In the mean time, we must defer notifications because we are added to
// the imgRequest's proxy list, and we can get extra notifications
// resulting from methods such as StartDecoding(). See bug 579122.
req->SetNotificationsDeferred(true);
req->MarkValidating();
// Add the proxy without notifying
hvc->AddProxy(req);
@ -2936,13 +2936,45 @@ imgCacheValidator::AddProxy(imgRequestProxy* aProxy)
// the network.
aProxy->AddToLoadGroup();
mProxies.AppendObject(aProxy);
mProxies.AppendElement(aProxy);
}
void
imgCacheValidator::RemoveProxy(imgRequestProxy* aProxy)
{
mProxies.RemoveObject(aProxy);
mProxies.RemoveElement(aProxy);
}
void
imgCacheValidator::UpdateProxies()
{
// We have finished validating the request, so we can safely take ownership
// of the proxy list. imgRequestProxy::SyncNotifyListener can mutate the list
// if imgRequestProxy::CancelAndForgetObserver is called by its owner. Note
// that any potential notifications should still be suppressed in
// imgRequestProxy::ChangeOwner because we haven't cleared the validating
// flag yet, and thus they will remain deferred.
AutoTArray<RefPtr<imgRequestProxy>, 4> proxies(Move(mProxies));
for (auto& proxy : proxies) {
// First update the state of all proxies before notifying any of them
// to ensure a consistent state (e.g. in case the notification causes
// other proxies to be touched indirectly.)
MOZ_ASSERT(proxy->IsValidating());
MOZ_ASSERT(proxy->NotificationsDeferred(),
"Proxies waiting on cache validation should be "
"deferring notifications!");
if (mNewRequest) {
proxy->ChangeOwner(mNewRequest);
}
proxy->ClearValidating();
}
for (auto& proxy : proxies) {
// Notify synchronously, because we're already in OnStartRequest, an
// asynchronously-called function.
proxy->SyncNotifyListener();
}
}
/** nsIRequestObserver methods **/
@ -2982,25 +3014,11 @@ imgCacheValidator::OnStartRequest(nsIRequest* aRequest, nsISupports* ctxt)
}
if (isFromCache && sameURI) {
uint32_t count = mProxies.Count();
for (int32_t i = count-1; i>=0; i--) {
imgRequestProxy* proxy = static_cast<imgRequestProxy*>(mProxies[i]);
// Proxies waiting on cache validation should be deferring
// notifications. Undefer them.
MOZ_ASSERT(proxy->NotificationsDeferred(),
"Proxies waiting on cache validation should be "
"deferring notifications!");
proxy->SetNotificationsDeferred(false);
// Notify synchronously, because we're already in OnStartRequest, an
// asynchronously-called function.
proxy->SyncNotifyListener();
}
// We don't need to load this any more.
aRequest->Cancel(NS_BINDING_ABORTED);
// Clear the validator before updating the proxies. The notifications may
// clone an existing request, and its state could be inconsistent.
mRequest->SetLoadId(context);
mRequest->SetValidator(nullptr);
@ -3009,6 +3027,7 @@ imgCacheValidator::OnStartRequest(nsIRequest* aRequest, nsISupports* ctxt)
mNewRequest = nullptr;
mNewEntry = nullptr;
UpdateProxies();
return NS_OK;
}
}
@ -3035,6 +3054,8 @@ imgCacheValidator::OnStartRequest(nsIRequest* aRequest, nsISupports* ctxt)
// Doom the old request's cache entry
mRequest->RemoveFromCache();
// Clear the validator before updating the proxies. The notifications may
// clone an existing request, and its state could be inconsistent.
mRequest->SetValidator(nullptr);
mRequest = nullptr;
@ -3055,17 +3076,7 @@ imgCacheValidator::OnStartRequest(nsIRequest* aRequest, nsISupports* ctxt)
// changes the caching behaviour for imgRequests.
mImgLoader->PutIntoCache(mNewRequest->CacheKey(), mNewEntry);
uint32_t count = mProxies.Count();
for (int32_t i = count-1; i>=0; i--) {
imgRequestProxy* proxy = static_cast<imgRequestProxy*>(mProxies[i]);
proxy->ChangeOwner(mNewRequest);
// Notify synchronously, because we're already in OnStartRequest, an
// asynchronously-called function.
proxy->SetNotificationsDeferred(false);
proxy->SyncNotifyListener();
}
UpdateProxies();
mNewRequest = nullptr;
mNewEntry = nullptr;

3
image/imgLoader.h
View File

@ -568,6 +568,7 @@ public:
NS_DECL_NSIASYNCVERIFYREDIRECTCALLBACK
private:
void UpdateProxies();
virtual ~imgCacheValidator();
nsCOMPtr<nsIStreamListener> mDestListener;
@ -576,7 +577,7 @@ private:
nsCOMPtr<nsIChannel> mRedirectChannel;
RefPtr<imgRequest> mRequest;
nsCOMArray<imgIRequest> mProxies;
AutoTArray<RefPtr<imgRequestProxy>, 4> mProxies;
RefPtr<imgRequest> mNewRequest;
RefPtr<imgCacheEntry> mNewEntry;

92
image/imgRequestProxy.cpp
View File

@ -121,6 +121,7 @@ imgRequestProxy::imgRequestProxy() :
mListenerIsStrongRef(false),
mDecodeRequested(false),
mDeferNotifications(false),
mValidating(false),
mHadListener(false),
mHadDispatch(false)
{
@ -163,15 +164,13 @@ imgRequestProxy::~imgRequestProxy()
// above assert.
NullOutListener();
if (GetOwner()) {
/* Call RemoveProxy with a successful status. This will keep the
channel, if still downloading data, from being canceled if 'this' is
the last observer. This allows the image to continue to download and
be cached even if no one is using it currently.
*/
mCanceled = true;
GetOwner()->RemoveProxy(this, NS_OK);
}
/* Call RemoveProxy with a successful status. This will keep the
channel, if still downloading data, from being canceled if 'this' is
the last observer. This allows the image to continue to download and
be cached even if no one is using it currently.
*/
mCanceled = true;
RemoveFromOwner(NS_OK);
RemoveFromLoadGroup();
LOG_FUNC(gImgLog, "imgRequestProxy::~imgRequestProxy");
@ -237,6 +236,7 @@ imgRequestProxy::ChangeOwner(imgRequest* aNewOwner)
GetOwner()->RemoveProxy(this, NS_OK);
mBehaviour->SetOwner(aNewOwner);
MOZ_ASSERT(!GetValidator(), "New owner cannot be validating!");
// If we were locked, apply the locks here
for (uint32_t i = 0; i < oldLockCount; i++) {
@ -251,14 +251,29 @@ imgRequestProxy::ChangeOwner(imgRequest* aNewOwner)
}
AddToOwner(nullptr);
return NS_OK;
}
void
imgRequestProxy::MarkValidating()
{
MOZ_ASSERT(GetValidator());
mValidating = true;
}
void
imgRequestProxy::ClearValidating()
{
MOZ_ASSERT(mValidating);
MOZ_ASSERT(!GetValidator());
mValidating = false;
// If we'd previously requested a synchronous decode, request a decode on the
// new image.
if (mDecodeRequested) {
mDecodeRequested = false;
StartDecoding(imgIContainer::FLAG_NONE);
}
return NS_OK;
}
bool
@ -351,11 +366,28 @@ imgRequestProxy::AddToOwner(nsIDocument* aLoadingDocument)
mEventTarget = do_GetMainThread();
}
if (!GetOwner()) {
imgRequest* owner = GetOwner();
if (!owner) {
return;
}
GetOwner()->AddProxy(this);
owner->AddProxy(this);
}
void
imgRequestProxy::RemoveFromOwner(nsresult aStatus)
{
imgRequest* owner = GetOwner();
if (owner) {
if (mValidating) {
imgCacheValidator* validator = owner->GetValidator();
MOZ_ASSERT(validator);
validator->RemoveProxy(this);
mValidating = false;
}
owner->RemoveProxy(this, aStatus);
}
}
void
@ -494,10 +526,7 @@ imgRequestProxy::Cancel(nsresult status)
void
imgRequestProxy::DoCancel(nsresult status)
{
if (GetOwner()) {
GetOwner()->RemoveProxy(this, status);
}
RemoveFromOwner(status);
RemoveFromLoadGroup();
NullOutListener();
}
@ -519,17 +548,7 @@ imgRequestProxy::CancelAndForgetObserver(nsresult aStatus)
mCanceled = true;
mForceDispatchLoadGroup = true;
imgRequest* owner = GetOwner();
if (owner) {
imgCacheValidator* validator = owner->GetValidator();
if (validator) {
validator->RemoveProxy(this);
}
owner->RemoveProxy(this, aStatus);
}
RemoveFromOwner(aStatus);
RemoveFromLoadGroup();
mForceDispatchLoadGroup = false;
@ -859,15 +878,16 @@ imgRequestProxy::PerformClone(imgINotificationObserver* aObserver,
// surprised.
NS_ADDREF(*aClone = clone);
if (GetOwner() && GetOwner()->GetValidator()) {
imgCacheValidator* validator = GetValidator();
if (validator) {
// Note that if we have a validator, we don't want to issue notifications at
// 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);
clone->MarkValidating();
validator->AddProxy(clone);
} else {
// 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
@ -1270,6 +1290,16 @@ imgRequestProxy::GetOwner() const
return mBehaviour->GetOwner();
}
imgCacheValidator*
imgRequestProxy::GetValidator() const
{
imgRequest* owner = GetOwner();
if (!owner) {
return nullptr;
}
return owner->GetValidator();
}
////////////////// imgRequestProxyStatic methods
class StaticBehaviour : public ProxyBehaviour

14
image/imgRequestProxy.h
View File

@ -30,6 +30,7 @@
{0x8f, 0x65, 0x9c, 0x46, 0x2e, 0xe2, 0xbc, 0x95} \
}
class imgCacheValidator;
class imgINotificationObserver;
class imgStatusNotifyRunnable;
class ProxyBehaviour;
@ -111,15 +112,21 @@ public:
virtual void SetHasImage() override;
// Whether we want notifications from ProgressTracker to be deferred until
// an event it has scheduled has been fired.
// an event it has scheduled has been fired and/or validation is complete.
virtual bool NotificationsDeferred() const override
{
return mDeferNotifications;
return IsValidating() || mDeferNotifications;
}
virtual void SetNotificationsDeferred(bool aDeferNotifications) override
{
mDeferNotifications = aDeferNotifications;
}
bool IsValidating() const
{
return mValidating;
}
void MarkValidating();
void ClearValidating();
bool IsOnEventTarget() const;
already_AddRefed<nsIEventTarget> GetEventTarget() const override;
@ -194,6 +201,7 @@ protected:
already_AddRefed<Image> GetImage() const;
bool HasImage() const;
imgRequest* GetOwner() const;
imgCacheValidator* GetValidator() const;
nsresult PerformClone(imgINotificationObserver* aObserver,
nsIDocument* aLoadingDocument,
@ -212,6 +220,7 @@ private:
friend class imgCacheValidator;
void AddToOwner(nsIDocument* aLoadingDocument);
void RemoveFromOwner(nsresult aStatus);
nsresult DispatchWithTargetIfAvailable(already_AddRefed<nsIRunnable> aEvent);
void DispatchWithTarget(already_AddRefed<nsIRunnable> aEvent);
@ -242,6 +251,7 @@ private:
// Whether we want to defer our notifications by the non-virtual Observer
// interfaces as image loads proceed.
bool mDeferNotifications : 1;
bool mValidating : 1;
bool mHadListener : 1;
bool mHadDispatch : 1;
};