From cd477f5dcf437af9ab0bd46780303acc07656470 Mon Sep 17 00:00:00 2001 From: Benjamin Smedberg Date: Wed, 23 Feb 2011 16:04:35 -0500 Subject: [PATCH] Revert bug 634534 because of test failures (test_visibility.html timing out, and neverending.sjs data leaking into other plugin tests?) --- .../base/src/nsNPAPIPluginStreamListener.cpp | 28 ++-- .../base/src/nsNPAPIPluginStreamListener.h | 45 +----- modules/plugin/base/src/nsPluginHost.cpp | 2 - .../base/src/nsPluginStreamListenerPeer.cpp | 138 +++--------------- .../base/src/nsPluginStreamListenerPeer.h | 5 +- modules/plugin/test/mochitest/Makefile.in | 1 - .../test_pluginstream_seek_close.html | 38 ----- modules/plugin/test/testplugin/nptest.cpp | 18 +-- modules/plugin/test/testplugin/nptest.h | 1 - 9 files changed, 46 insertions(+), 230 deletions(-) delete mode 100644 modules/plugin/test/mochitest/test_pluginstream_seek_close.html diff --git a/modules/plugin/base/src/nsNPAPIPluginStreamListener.cpp b/modules/plugin/base/src/nsNPAPIPluginStreamListener.cpp index b6789ec3e962..53ea6684c5fb 100644 --- a/modules/plugin/base/src/nsNPAPIPluginStreamListener.cpp +++ b/modules/plugin/base/src/nsNPAPIPluginStreamListener.cpp @@ -361,7 +361,7 @@ nsNPAPIPluginStreamListener::OnStartBinding(nsIPluginStreamInfo* pluginInfo) return NS_OK; } -void +nsresult nsNPAPIPluginStreamListener::SuspendRequest() { NS_ASSERTION(!mIsSuspended, @@ -369,18 +369,19 @@ nsNPAPIPluginStreamListener::SuspendRequest() nsCOMPtr pluginInfoNPAPI = do_QueryInterface(mStreamInfo); + nsIRequest *request; - if (!pluginInfoNPAPI) { - return; + if (!pluginInfoNPAPI || !(request = pluginInfoNPAPI->GetRequest())) { + NS_ERROR("Trying to suspend a non-suspendable stream!"); + return NS_ERROR_FAILURE; } nsresult rv = StartDataPump(); - if (NS_FAILED(rv)) - return; + NS_ENSURE_SUCCESS(rv, rv); mIsSuspended = PR_TRUE; - pluginInfoNPAPI->SuspendRequests(); + return request->Suspend(); } void @@ -389,7 +390,11 @@ nsNPAPIPluginStreamListener::ResumeRequest() nsCOMPtr pluginInfoNPAPI = do_QueryInterface(mStreamInfo); - pluginInfoNPAPI->ResumeRequests(); + nsIRequest *request = pluginInfoNPAPI->GetRequest(); + + // request can be null if the network stream is done. + if (request) + request->Resume(); mIsSuspended = PR_FALSE; } @@ -604,7 +609,7 @@ nsNPAPIPluginStreamListener::OnDataAvailable(nsIPluginStreamInfo* pluginInfo, if (numtowrite <= 0 || (!mIsPluginInitJSStream && PluginInitJSLoadInProgress())) { if (!mIsSuspended) { - SuspendRequest(); + rv = SuspendRequest(); } // Break out of the inner loop, but keep going through the @@ -671,7 +676,7 @@ nsNPAPIPluginStreamListener::OnDataAvailable(nsIPluginStreamInfo* pluginInfo, // resume the request. if (mIsSuspended || ++zeroBytesWriteCount == 3) { if (!mIsSuspended) { - SuspendRequest(); + rv = SuspendRequest(); } // Break out of the for loop, but keep going through the @@ -753,8 +758,9 @@ nsNPAPIPluginStreamListener::OnStopBinding(nsIPluginStreamInfo* pluginInfo, nsCOMPtr pluginInfoNPAPI = do_QueryInterface(mStreamInfo); - if (pluginInfoNPAPI) { - pluginInfoNPAPI->CancelRequests(status); + nsIRequest *request; + if (pluginInfoNPAPI && (request = pluginInfoNPAPI->GetRequest())) { + request->Cancel(status); } } diff --git a/modules/plugin/base/src/nsNPAPIPluginStreamListener.h b/modules/plugin/base/src/nsNPAPIPluginStreamListener.h index 7c20c12b03d7..8cd8ac1c69f5 100644 --- a/modules/plugin/base/src/nsNPAPIPluginStreamListener.h +++ b/modules/plugin/base/src/nsNPAPIPluginStreamListener.h @@ -45,7 +45,7 @@ #include "nsIRequest.h" #include "nsITimer.h" #include "nsAutoPtr.h" -#include "nsCOMArray.h" +#include "nsCOMPtr.h" #include "nsIOutputStream.h" #include "nsIPluginInstanceOwner.h" #include "nsString.h" @@ -68,49 +68,14 @@ class nsINPAPIPluginStreamInfo : public nsIPluginStreamInfo { public: NS_DECLARE_STATIC_IID_ACCESSOR(NS_INPAPIPLUGINSTREAMINFO_IID) - - void TrackRequest(nsIRequest* request) - { - mRequests.AppendObject(request); - } - - void ReplaceRequest(nsIRequest* oldRequest, nsIRequest* newRequest) - { - PRInt32 i = mRequests.IndexOfObject(oldRequest); - if (i == -1) { - NS_ASSERTION(mRequests.Count() == 0, - "Only our initial stream should be unknown!"); - mRequests.AppendObject(oldRequest); - } - else { - mRequests.ReplaceObjectAt(newRequest, i); - } - } - void CancelRequests(nsresult status) + nsIRequest *GetRequest() { - // Copy the array to avoid modification during the loop. - nsCOMArray requestsCopy(mRequests); - for (PRInt32 i = 0; i < requestsCopy.Count(); ++i) - requestsCopy[i]->Cancel(status); - } - - void SuspendRequests() { - nsCOMArray requestsCopy(mRequests); - for (PRInt32 i = 0; i < requestsCopy.Count(); ++i) - requestsCopy[i]->Suspend(); - } - - void ResumeRequests() { - nsCOMArray requestsCopy(mRequests); - for (PRInt32 i = 0; i < requestsCopy.Count(); ++i) - requestsCopy[i]->Resume(); + return mRequest; } protected: - friend class nsPluginByteRangeStreamListener; - - nsCOMArray mRequests; + nsCOMPtr mRequest; }; NS_DEFINE_STATIC_IID_ACCESSOR(nsINPAPIPluginStreamInfo, @@ -156,7 +121,7 @@ public: nsresult CleanUpStream(NPReason reason); void CallURLNotify(NPReason reason); void SetCallNotify(PRBool aCallNotify) { mCallNotify = aCallNotify; } - void SuspendRequest(); + nsresult SuspendRequest(); void ResumeRequest(); nsresult StartDataPump(); void StopDataPump(); diff --git a/modules/plugin/base/src/nsPluginHost.cpp b/modules/plugin/base/src/nsPluginHost.cpp index dd92396cbf31..0ef1eef5e713 100644 --- a/modules/plugin/base/src/nsPluginHost.cpp +++ b/modules/plugin/base/src/nsPluginHost.cpp @@ -3341,8 +3341,6 @@ nsresult nsPluginHost::NewPluginURLStream(const nsString& aURL, rv = AddHeadersToChannel(aHeadersData, aHeadersDataLen, httpChannel); } rv = channel->AsyncOpen(listenerPeer, nsnull); - if (NS_SUCCEEDED(rv)) - listenerPeer->TrackRequest(channel); return rv; } diff --git a/modules/plugin/base/src/nsPluginStreamListenerPeer.cpp b/modules/plugin/base/src/nsPluginStreamListenerPeer.cpp index b09b63200634..78b3460633e9 100644 --- a/modules/plugin/base/src/nsPluginStreamListenerPeer.cpp +++ b/modules/plugin/base/src/nsPluginStreamListenerPeer.cpp @@ -51,7 +51,6 @@ #include "nsIURI.h" #include "nsPluginHost.h" #include "nsIByteRangeRequest.h" -#include "nsIMultiPartChannel.h" #include "nsIInputStreamTee.h" #include "nsPrintfCString.h" #include "nsIContentUtils.h" @@ -63,10 +62,7 @@ // nsPluginByteRangeStreamListener -class nsPluginByteRangeStreamListener - : public nsIStreamListener - , public nsIInterfaceRequestor -{ +class nsPluginByteRangeStreamListener : public nsIStreamListener { public: nsPluginByteRangeStreamListener(nsIWeakReference* aWeakPtr); virtual ~nsPluginByteRangeStreamListener(); @@ -74,7 +70,6 @@ public: NS_DECL_ISUPPORTS NS_DECL_NSIREQUESTOBSERVER NS_DECL_NSISTREAMLISTENER - NS_DECL_NSIINTERFACEREQUESTOR private: nsCOMPtr mStreamConverter; @@ -82,11 +77,7 @@ private: PRBool mRemoveMagicNumber; }; -NS_IMPL_ISUPPORTS3(nsPluginByteRangeStreamListener, - nsIRequestObserver, - nsIStreamListener, - nsIInterfaceRequestor) - +NS_IMPL_ISUPPORTS1(nsPluginByteRangeStreamListener, nsIStreamListener) nsPluginByteRangeStreamListener::nsPluginByteRangeStreamListener(nsIWeakReference* aWeakPtr) { mWeakPtrPluginStreamListenerPeer = aWeakPtr; @@ -99,22 +90,6 @@ nsPluginByteRangeStreamListener::~nsPluginByteRangeStreamListener() mWeakPtrPluginStreamListenerPeer = 0; } -/** - * Unwrap any byte-range requests so that we can check whether the base channel - * is being tracked properly. - */ -static nsCOMPtr -GetBaseRequest(nsIRequest* r) -{ - nsCOMPtr mp = do_QueryInterface(r); - if (!mp) - return r; - - nsCOMPtr base; - mp->GetBaseChannel(getter_AddRefs(base)); - return already_AddRefed(base.forget()); -} - NS_IMETHODIMP nsPluginByteRangeStreamListener::OnStartRequest(nsIRequest *request, nsISupports *ctxt) { @@ -124,12 +99,6 @@ nsPluginByteRangeStreamListener::OnStartRequest(nsIRequest *request, nsISupports if (!finalStreamListener) return NS_ERROR_FAILURE; - nsPluginStreamListenerPeer *pslp = - static_cast(finalStreamListener.get()); - - NS_ASSERTION(pslp->mRequests.IndexOfObject(GetBaseRequest(request)) != -1, - "Untracked byte-range request?"); - nsCOMPtr serv = do_GetService(NS_STREAMCONVERTERSERVICE_CONTRACTID, &rv); if (NS_SUCCEEDED(rv)) { rv = serv->AsyncConvertData(MULTIPART_BYTERANGES, @@ -156,6 +125,10 @@ nsPluginByteRangeStreamListener::OnStartRequest(nsIRequest *request, nsISupports return NS_ERROR_FAILURE; } + // get nsPluginStreamListenerPeer* ptr from finalStreamListener + nsPluginStreamListenerPeer *pslp = + reinterpret_cast(finalStreamListener.get()); + if (responseCode != 200) { PRBool bWantsAllNetworkStreams = PR_FALSE; pslp->GetPluginInstance()-> @@ -186,13 +159,6 @@ nsPluginByteRangeStreamListener::OnStopRequest(nsIRequest *request, nsISupports if (!finalStreamListener) return NS_ERROR_FAILURE; - nsPluginStreamListenerPeer *pslp = - static_cast(finalStreamListener.get()); - PRBool found = pslp->mRequests.RemoveObject(request); - if (!found) { - NS_ERROR("OnStopRequest received for untracked byte-range request!"); - } - if (mRemoveMagicNumber) { // remove magic number from container nsCOMPtr container = do_QueryInterface(ctxt); @@ -256,18 +222,6 @@ nsPluginByteRangeStreamListener::OnDataAvailable(nsIRequest *request, nsISupport return mStreamConverter->OnDataAvailable(request, ctxt, inStr, sourceOffset, count); } -NS_IMETHODIMP -nsPluginByteRangeStreamListener::GetInterface(const nsIID& aIID, void** result) -{ - // Forward interface requests to our parent - nsCOMPtr finalStreamListener = do_QueryReferent(mWeakPtrPluginStreamListenerPeer); - if (!finalStreamListener) - return NS_ERROR_FAILURE; - - return finalStreamListener->GetInterface(aIID, result); -} - - // nsPRUintKey class nsPRUintKey : public nsHashKey { @@ -343,7 +297,8 @@ nsPluginStreamListenerPeer::~nsPluginStreamListenerPeer() // Called as a result of GetURL and PostURL nsresult nsPluginStreamListenerPeer::Initialize(nsIURI *aURL, nsNPAPIPluginInstance *aInstance, - nsIPluginStreamListener* aListener) + nsIPluginStreamListener* aListener, + PRInt32 requestCount) { #ifdef PLUGIN_LOGGING nsCAutoString urlSpec; @@ -362,7 +317,7 @@ nsresult nsPluginStreamListenerPeer::Initialize(nsIURI *aURL, mPStreamListener = static_cast(aListener); mPStreamListener->SetStreamListenerPeer(this); - mPendingRequests = 1; + mPendingRequests = requestCount; mDataForwardToRequest = new nsHashtable(16, PR_FALSE); if (!mDataForwardToRequest) @@ -516,12 +471,6 @@ nsPluginStreamListenerPeer::OnStartRequest(nsIRequest *request, nsISupports* aContext) { nsresult rv = NS_OK; - - if (mRequests.IndexOfObject(GetBaseRequest(request)) == -1) { - NS_ASSERTION(mRequests.Count() == 0, - "Only our initial stream should be unknown!"); - TrackRequest(request); - } if (mHaveFiredOnStartRequest) { return NS_OK; @@ -601,6 +550,8 @@ nsPluginStreamListenerPeer::OnStartRequest(nsIRequest *request, mLength = length; } + mRequest = request; + nsCAutoString aContentType; // XXX but we already got the type above! rv = channel->GetContentType(aContentType); if (NS_FAILED(rv)) @@ -824,10 +775,7 @@ nsPluginStreamListenerPeer::RequestRead(NPByteRange* rangeList) if (NS_FAILED(rv)) return rv; - rv = channel->AsyncOpen(converter, container); - if (NS_SUCCEEDED(rv)) - TrackRequest(channel); - return rv; + return channel->AsyncOpen(converter, container); } NS_IMETHODIMP @@ -916,9 +864,6 @@ NS_IMETHODIMP nsPluginStreamListenerPeer::OnDataAvailable(nsIRequest *request, PRUint32 sourceOffset, PRUint32 aLength) { - NS_ASSERTION(mRequests.IndexOfObject(GetBaseRequest(request)) != -1, - "Received OnDataAvailable for untracked request."); - if (mRequestFailed) return NS_ERROR_FAILURE; @@ -940,6 +885,8 @@ NS_IMETHODIMP nsPluginStreamListenerPeer::OnDataAvailable(nsIRequest *request, if (!mPStreamListener) return NS_ERROR_FAILURE; + mRequest = request; + const char * url = nsnull; GetURL(&url); @@ -1021,14 +968,6 @@ NS_IMETHODIMP nsPluginStreamListenerPeer::OnStopRequest(nsIRequest *request, nsresult aStatus) { nsresult rv = NS_OK; - - nsCOMPtr mp = do_QueryInterface(request); - if (!mp) { - PRBool found = mRequests.RemoveObject(request); - if (!found) { - NS_ERROR("Received OnStopRequest for untracked request."); - } - } PLUGIN_LOG(PLUGIN_LOG_NOISY, ("nsPluginStreamListenerPeer::OnStopRequest this=%p aStatus=%d request=%p\n", @@ -1134,6 +1073,7 @@ NS_IMETHODIMP nsPluginStreamListenerPeer::OnStopRequest(nsIRequest *request, if (NS_SUCCEEDED(aStatus)) { mStreamComplete = PR_TRUE; } + mRequest = NULL; return NS_OK; } @@ -1344,47 +1284,6 @@ nsPluginStreamListenerPeer::GetInterface(const nsIID& aIID, void** result) return GetInterfaceGlobal(aIID, result); } -/** - * Proxy class which forwards async redirect notifications back to the necko - * callback, keeping nsPluginStreamListenerPeer::mRequests in sync with - * which channel is active. - */ -class ChannelRedirectProxyCallback : public nsIAsyncVerifyRedirectCallback -{ -public: - ChannelRedirectProxyCallback(nsINPAPIPluginStreamInfo* listener, - nsIAsyncVerifyRedirectCallback* parent, - nsIChannel* oldChannel, - nsIChannel* newChannel) - : mWeakListener(do_GetWeakReference(listener)) - , mParent(parent) - , mOldChannel(oldChannel) - , mNewChannel(newChannel) - { - } - - NS_DECL_ISUPPORTS - - NS_IMETHODIMP OnRedirectVerifyCallback(nsresult result) - { - if (NS_SUCCEEDED(result)) { - nsCOMPtr listener = do_QueryReferent(mWeakListener); - if (listener) - listener->ReplaceRequest(mOldChannel, mNewChannel); - } - return mParent->OnRedirectVerifyCallback(result); - } - -private: - nsWeakPtr mWeakListener; - nsCOMPtr mParent; - nsCOMPtr mOldChannel; - nsCOMPtr mNewChannel; -}; - -NS_IMPL_ISUPPORTS1(ChannelRedirectProxyCallback, nsIAsyncVerifyRedirectCallback) - - NS_IMETHODIMP nsPluginStreamListenerPeer::AsyncOnChannelRedirect(nsIChannel *oldChannel, nsIChannel *newChannel, PRUint32 flags, nsIAsyncVerifyRedirectCallback* callback) @@ -1394,11 +1293,8 @@ nsPluginStreamListenerPeer::AsyncOnChannelRedirect(nsIChannel *oldChannel, nsICh return NS_ERROR_FAILURE; } - nsCOMPtr proxyCallback = - new ChannelRedirectProxyCallback(this, callback, oldChannel, newChannel); - // Give NPAPI a chance to control redirects. - bool notificationHandled = mPStreamListener->HandleRedirectNotification(oldChannel, newChannel, proxyCallback); + bool notificationHandled = mPStreamListener->HandleRedirectNotification(oldChannel, newChannel, callback); if (notificationHandled) { return NS_OK; } @@ -1435,5 +1331,5 @@ nsPluginStreamListenerPeer::AsyncOnChannelRedirect(nsIChannel *oldChannel, nsICh return rv; } - return channelEventSink->AsyncOnChannelRedirect(oldChannel, newChannel, flags, proxyCallback); + return channelEventSink->AsyncOnChannelRedirect(oldChannel, newChannel, flags, callback); } diff --git a/modules/plugin/base/src/nsPluginStreamListenerPeer.h b/modules/plugin/base/src/nsPluginStreamListenerPeer.h index 7cba1e4de86f..2d04f31a8451 100644 --- a/modules/plugin/base/src/nsPluginStreamListenerPeer.h +++ b/modules/plugin/base/src/nsPluginStreamListenerPeer.h @@ -109,14 +109,15 @@ public: // Called by GetURL and PostURL (via NewStream) nsresult Initialize(nsIURI *aURL, nsNPAPIPluginInstance *aInstance, - nsIPluginStreamListener *aListener); + nsIPluginStreamListener *aListener, + PRInt32 requestCount = 1); nsresult InitializeEmbedded(nsIURI *aURL, nsNPAPIPluginInstance* aInstance, nsIPluginInstanceOwner *aOwner = nsnull); nsresult InitializeFullPage(nsIURI* aURL, nsNPAPIPluginInstance *aInstance); - + nsresult OnFileAvailable(nsIFile* aFile); nsresult ServeStreamAsFile(nsIRequest *request, nsISupports *ctxt); diff --git a/modules/plugin/test/mochitest/Makefile.in b/modules/plugin/test/mochitest/Makefile.in index ac98f2acca99..2d88fe01b951 100644 --- a/modules/plugin/test/mochitest/Makefile.in +++ b/modules/plugin/test/mochitest/Makefile.in @@ -70,7 +70,6 @@ _MOCHITEST_FILES = \ test_pluginstream_poststream.html \ test_pluginstream_seek.html \ test_pluginstream_newstream.html \ - test_pluginstream_seek_close.html \ test_fullpage.html \ loremipsum.xtest \ loremipsum.xtest^headers^ \ diff --git a/modules/plugin/test/mochitest/test_pluginstream_seek_close.html b/modules/plugin/test/mochitest/test_pluginstream_seek_close.html deleted file mode 100644 index 091b85c64776..000000000000 --- a/modules/plugin/test/mochitest/test_pluginstream_seek_close.html +++ /dev/null @@ -1,38 +0,0 @@ - - - NPAPI Seekable NPStream Test - - - - - -

- - - - - - - - - diff --git a/modules/plugin/test/testplugin/nptest.cpp b/modules/plugin/test/testplugin/nptest.cpp index b7e6e7c46bca..7317d7d1ad52 100644 --- a/modules/plugin/test/testplugin/nptest.cpp +++ b/modules/plugin/test/testplugin/nptest.cpp @@ -739,7 +739,6 @@ NPP_New(NPMIMEType pluginType, NPP instance, uint16_t mode, int16_t argc, char* instanceData->focusState = ACTIVATION_STATE_UNKNOWN; instanceData->focusEventCount = 0; instanceData->eventModel = 0; - instanceData->closeStream = false; instance->pdata = instanceData; TestNPObject* scriptableObject = (TestNPObject*)NPN_CreateObject(instance, &sNPClass); @@ -855,9 +854,6 @@ NPP_New(NPMIMEType pluginType, NPP instance, uint16_t mode, int16_t argc, char* strcmp(argv[i], "false") == 0) { instanceData->cleanupWidget = false; } - if (!strcmp(argn[i], "closestream")) { - instanceData->closeStream = true; - } } if (!browserSupportsWindowless || !pluginSupportsWindowlessMode()) { @@ -1120,6 +1116,7 @@ NPP_WriteReady(NPP instance, NPStream* stream) int32_t NPP_Write(NPP instance, NPStream* stream, int32_t offset, int32_t len, void* buffer) { + printf("NPP_Write, offset=%d, len=%d, end=%d\n", offset, len, stream->end); InstanceData* instanceData = (InstanceData*)(instance->pdata); instanceData->writeCount++; @@ -1166,18 +1163,11 @@ NPP_Write(NPP instance, NPStream* stream, int32_t offset, int32_t len, void* buf return len; } - if (instanceData->closeStream) { - instanceData->closeStream = false; - if (instanceData->testrange != NULL) { - NPError err = NPN_RequestRead(stream, instanceData->testrange); - } - NPN_DestroyStream(instance, stream, NPRES_USER_BREAK); - } - else if (instanceData->streamMode == NP_SEEK && + // If the complete stream has been written, and we're doing a seek test, + // then call NPN_RequestRead. + if (instanceData->streamMode == NP_SEEK && stream->end != 0 && stream->end == ((uint32_t)instanceData->streamBufSize + len)) { - // If the complete stream has been written, and we're doing a seek test, - // then call NPN_RequestRead. // prevent recursion instanceData->streamMode = NP_NORMAL; diff --git a/modules/plugin/test/testplugin/nptest.h b/modules/plugin/test/testplugin/nptest.h index a198d07fd474..4babae1cf535 100644 --- a/modules/plugin/test/testplugin/nptest.h +++ b/modules/plugin/test/testplugin/nptest.h @@ -137,7 +137,6 @@ typedef struct InstanceData { ActivationState focusState; int32_t focusEventCount; int32_t eventModel; - bool closeStream; } InstanceData; void notifyDidPaint(InstanceData* instanceData);