Cancel running requests when loading content in QgsNetworkContentFetc…

…her (fix #11332)
qgis · Oct 9, 2014 · ac71e60 · m-kuhn · Dec 2, 2014 · nyalldawson
1 parent 7a92293
commit ac71e60
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 30 deletions.
diff --git a/src/core/composer/qgscomposerhtml.cpp b/src/core/composer/qgscomposerhtml.cpp
@@ -30,18 +30,20 @@
 #include <QImage>
 #include <QNetworkReply>
 
-QgsComposerHtml::QgsComposerHtml( QgsComposition* c, bool createUndoCommands ): QgsComposerMultiFrame( c, createUndoCommands ),
-    mContentMode( QgsComposerHtml::Url ),
-    mWebPage( 0 ),
-    mLoaded( false ),
-    mHtmlUnitsToMM( 1.0 ),
-    mRenderedPage( 0 ),
-    mEvaluateExpressions( true ),
-    mUseSmartBreaks( true ),
-    mMaxBreakDistance( 10 ),
-    mExpressionFeature( 0 ),
-    mExpressionLayer( 0 ),
-    mEnableUserStylesheet( false )
+QgsComposerHtml::QgsComposerHtml( QgsComposition* c, bool createUndoCommands )
+    : QgsComposerMultiFrame( c, createUndoCommands )
+    , mContentMode( QgsComposerHtml::Url )
+    , mWebPage( 0 )
+    , mLoaded( false )
+    , mHtmlUnitsToMM( 1.0 )
+    , mRenderedPage( 0 )
+    , mEvaluateExpressions( true )
+    , mUseSmartBreaks( true )
+    , mMaxBreakDistance( 10 )
+    , mExpressionFeature( 0 )
+    , mExpressionLayer( 0 )
+    , mEnableUserStylesheet( false )
+    , mFetcher( 0 )
 {
   mHtmlUnitsToMM = htmlUnitsToMM();
   mWebPage = new QWebPage();
@@ -74,25 +76,33 @@ QgsComposerHtml::QgsComposerHtml( QgsComposition* c, bool createUndoCommands ):
   //to update the expression context
   connect( &mComposition->atlasComposition(), SIGNAL( featureChanged( QgsFeature* ) ), this, SLOT( refreshExpressionContext() ) );
 
+  mFetcher = new QgsNetworkContentFetcher();
+  connect( mFetcher, SIGNAL( finished() ), this, SLOT( frameLoaded() ) );
+
 }
 
-QgsComposerHtml::QgsComposerHtml(): QgsComposerMultiFrame( 0, false ),
-    mContentMode( QgsComposerHtml::Url ),
-    mWebPage( 0 ),
-    mLoaded( false ),
-    mHtmlUnitsToMM( 1.0 ),
-    mRenderedPage( 0 ),
-    mUseSmartBreaks( true ),
-    mMaxBreakDistance( 10 ),
-    mExpressionFeature( 0 ),
-    mExpressionLayer( 0 )
+QgsComposerHtml::QgsComposerHtml()
+    : QgsComposerMultiFrame( 0, false )
+    , mContentMode( QgsComposerHtml::Url )
+    , mWebPage( 0 )
+    , mLoaded( false )
+    , mHtmlUnitsToMM( 1.0 )
+    , mRenderedPage( 0 )
+    , mUseSmartBreaks( true )
+    , mMaxBreakDistance( 10 )
+    , mExpressionFeature( 0 )
+    , mExpressionLayer( 0 )
+    , mFetcher( 0 )
 {
+  mFetcher = new QgsNetworkContentFetcher();
+  connect( mFetcher, SIGNAL( finished() ), this, SLOT( frameLoaded() ) );
 }
 
 QgsComposerHtml::~QgsComposerHtml()
 {
   delete mWebPage;
   delete mRenderedPage;
+  mFetcher->deleteLater();
 }
 
 void QgsComposerHtml::setUrl( const QUrl& url )
@@ -257,19 +267,17 @@ void QgsComposerHtml::renderCachedImage()
 
 QString QgsComposerHtml::fetchHtml( QUrl url )
 {
-  QgsNetworkContentFetcher fetcher;
   //pause until HTML fetch
   mLoaded = false;
-  fetcher.fetchContent( url );
-  connect( &fetcher, SIGNAL( finished() ), this, SLOT( frameLoaded() ) );
+  mFetcher->fetchContent( url );
 
   while ( !mLoaded )
   {
     qApp->processEvents();
   }
 
-  mFetchedHtml = fetcher.contentAsString();
-  mActualFetchedUrl = fetcher.reply()->url().toString();
+  mFetchedHtml = mFetcher->contentAsString();
+  mActualFetchedUrl = mFetcher->reply()->url().toString();
   return mFetchedHtml;
 }
 

diff --git a/src/core/composer/qgscomposerhtml.h b/src/core/composer/qgscomposerhtml.h
@@ -23,6 +23,7 @@ class QWebPage;
 class QImage;
 class QgsFeature;
 class QgsVectorLayer;
+class QgsNetworkContentFetcher;
 
 class CORE_EXPORT QgsComposerHtml: public QgsComposerMultiFrame
 {
@@ -38,7 +39,10 @@ class CORE_EXPORT QgsComposerHtml: public QgsComposerMultiFrame
     };
 
     QgsComposerHtml( QgsComposition* c, bool createUndoCommands );
+
+    //should be private - fix for QGIS 3.0
     QgsComposerHtml();
+
     ~QgsComposerHtml();
 
     /**Sets the source mode for item's HTML content.
@@ -241,6 +245,8 @@ class CORE_EXPORT QgsComposerHtml: public QgsComposerMultiFrame
     QString mUserStylesheet;
     bool mEnableUserStylesheet;
 
+    QgsNetworkContentFetcher* mFetcher;
+
     double htmlUnitsToMM(); //calculate scale factor
 
     //renders a snapshot of the page to a cached image

diff --git a/src/core/qgsnetworkcontentfetcher.cpp b/src/core/qgsnetworkcontentfetcher.cpp
@@ -32,7 +32,12 @@ QgsNetworkContentFetcher::QgsNetworkContentFetcher()
 
 QgsNetworkContentFetcher::~QgsNetworkContentFetcher()
 {
-  delete mReply;
+  if ( mReply->isRunning() )
+  {
+    //cancel running request
+    mReply->abort();
+  }
+  mReply->deleteLater();
 }
 
 void QgsNetworkContentFetcher::fetchContent( const QUrl url )
@@ -43,6 +48,14 @@ void QgsNetworkContentFetcher::fetchContent( const QUrl url )
   //get contents
   QNetworkRequest request( nextUrlToFetch );
 
+  if ( mReply )
+  {
+    //cancel any in progress requests
+    mReply->abort();
+    mReply->deleteLater();
+    mReply = 0;
+  }
+
   mReply = QgsNetworkAccessManager::instance()->get( request );
   connect( mReply, SIGNAL( finished() ), this, SLOT( contentLoaded() ) );
 }

diff --git a/tests/src/core/testqgsnetworkcontentfetcher.cpp b/tests/src/core/testqgsnetworkcontentfetcher.cpp
@@ -31,6 +31,7 @@ class TestQgsNetworkContentFetcher: public QObject
     void fetchEmptyUrl(); //test fetching blank url
     void fetchBadUrl(); //test fetching bad url
     void fetchUrlContent(); //test fetching url content
+    void doubleFetch(); //fetch content while already fetching content
     void fetchEncodedContent(); //test fetching url content encoded as utf-8
 
     void contentLoaded();
@@ -93,7 +94,8 @@ void TestQgsNetworkContentFetcher::fetchBadUrl()
 void TestQgsNetworkContentFetcher::fetchUrlContent()
 {
   QgsNetworkContentFetcher fetcher;
-  //test fetching content from the QGIS homepage
+  //test fetching content from the QGIS homepage - ideally a dedicate page should be created for these tests so
+  //that we do not rely on content from the homepage
   mLoaded = false;
   fetcher.fetchContent( QUrl( "http://www.qgis.org/en/site/" ) );
   connect( &fetcher, SIGNAL( finished() ), this, SLOT( contentLoaded() ) );
@@ -108,10 +110,33 @@ void TestQgsNetworkContentFetcher::fetchUrlContent()
   QVERIFY( mFetchedHtml.contains( QString( "QGIS" ) ) );
 }
 
+void TestQgsNetworkContentFetcher::doubleFetch()
+{
+  QgsNetworkContentFetcher fetcher;
+  //fetch content from the QGIS homepage - ideally a dedicate page should be created for these tests so
+  //that we do not rely on content from the homepage
+  mLoaded = false;
+  fetcher.fetchContent( QUrl( "http://www.osgeo.org/" ) );
+  //double fetch - this should happen before previous request finishes
+  fetcher.fetchContent( QUrl( "http://www.qgis.org/en/site/" ) );
+
+  connect( &fetcher, SIGNAL( finished() ), this, SLOT( contentLoaded() ) );
+
+  while ( !mLoaded )
+  {
+    qApp->processEvents();
+  }
+  QVERIFY( fetcher.reply()->error() == QNetworkReply::NoError );
+
+  //test retrieved content
+  QString mFetchedHtml = fetcher.contentAsString();
+  QVERIFY( mFetchedHtml.contains( QString( "QGIS" ) ) );
+}
+
 void TestQgsNetworkContentFetcher::fetchEncodedContent()
 {
   QgsNetworkContentFetcher fetcher;
-  //test fetching content from the QGIS homepage
+  //test fetching encoded content as string
   mLoaded = false;
   fetcher.fetchContent( QUrl::fromLocalFile( QString( TEST_DATA_DIR ) + QDir::separator() +  "encoded_html.html" ) );
   connect( &fetcher, SIGNAL( finished() ), this, SLOT( contentLoaded() ) );