Better handle failure on read in custom url handlers

Chromium 49 expects url request failures to be reported as a response
to either start or on read. So we should report failures during read
and make sure we can handle QIODevice calling fail on the job.

Change-Id: Id8f619d3cb6f187c99da580c6923130a95ca91ee
Reviewed-by: Michael Brüning <michael.bruning@theqtcompany.com>

3 files changed, 65 insertions, 9 deletions

diff --git a/src/core/url_request_custom_job.cpp b/src/core/url_request_custom_job.cpp
index 921be0b41..887222285 100644
--- a/src/core/url_request_custom_job.cpp
+++ b/src/core/url_request_custom_job.cpp
@@ -143,8 +143,12 @@ int URLRequestCustomJob::ReadRawData(IOBuffer *buf, int bufSize)
     qint64 rv = m_shared->m_device ? m_shared->m_device->read(buf->data(), bufSize) : -1;
     if (rv >= 0)
         return static_cast<int>(rv);
-    else
+    else {
+        // QIODevice::read might have called fail on us.
+        if (m_shared->m_error)
+            return m_shared->m_error;
         return ERR_FAILED;
+    }
 }
 
 
@@ -281,9 +285,11 @@ void URLRequestCustomJobShared::notifyStarted()
 
 void URLRequestCustomJobShared::fail(int error)
 {
-    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     QMutexLocker lock(&m_mutex);
     m_error = error;
+    if (content::BrowserThread::CurrentlyOn(content::BrowserThread::IO))
+        return;
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     if (!m_job)
         return;
     content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE, base::Bind(&URLRequestCustomJobShared::notifyFailure, m_weakFactory.GetWeakPtr()));
@@ -297,11 +303,9 @@ void URLRequestCustomJobShared::notifyFailure()
         return;
     if (m_device)
         m_device->close();
-    const URLRequestStatus status(URLRequestStatus::FAILED, m_error);
-    if (m_started)
-        m_job->SetStatus(status);
-    else
-        m_job->NotifyStartError(status);
+    if (!m_started)
+        m_job->NotifyStartError(URLRequestStatus::FromError(m_error));
+    // else we fail on the next read, or the read that might already be in progress
 }
 
 GURL URLRequestCustomJobShared::requestUrl()
diff --git a/src/core/url_request_custom_job.h b/src/core/url_request_custom_job.h
index 226c39e68..3fce76d36 100644
--- a/src/core/url_request_custom_job.h
+++ b/src/core/url_request_custom_job.h
@@ -61,7 +61,7 @@ public:
     URLRequestCustomJob(net::URLRequest *request, net::NetworkDelegate *networkDelegate, const std::string &scheme, QWeakPointer<const BrowserContextAdapter> adapter);
     virtual void Start() Q_DECL_OVERRIDE;
     virtual void Kill() Q_DECL_OVERRIDE;
-    virtual int ReadRawData(net::IOBuffer* buf, int buf_size)  Q_DECL_OVERRIDE;;
+    virtual int ReadRawData(net::IOBuffer *buf, int buf_size)  Q_DECL_OVERRIDE;
     virtual bool GetMimeType(std::string *mimeType) const Q_DECL_OVERRIDE;
     virtual bool GetCharset(std::string *charset) Q_DECL_OVERRIDE;
     virtual bool IsRedirectResponse(GURL* location, int* http_status_code) Q_DECL_OVERRIDE;
diff --git a/tests/auto/widgets/qwebengineprofile/tst_qwebengineprofile.cpp b/tests/auto/widgets/qwebengineprofile/tst_qwebengineprofile.cpp
index 95d5cf16a..a399f5565 100644
--- a/tests/auto/widgets/qwebengineprofile/tst_qwebengineprofile.cpp
+++ b/tests/auto/widgets/qwebengineprofile/tst_qwebengineprofile.cpp
@@ -47,6 +47,7 @@ private Q_SLOTS:
     void disableCache();
     void urlSchemeHandlers();
     void urlSchemeHandlerFailRequest();
+    void urlSchemeHandlerFailOnRead();
     void customUserAgent();
     void httpAcceptLanguage();
 };
@@ -237,10 +238,47 @@ class FailingUrlSchemeHandler : public QWebEngineUrlSchemeHandler
 public:
     void requestStarted(QWebEngineUrlRequestJob *job) override
     {
-        job->fail(QWebEngineUrlRequestJob::RequestFailed);
+        job->fail(QWebEngineUrlRequestJob::UrlInvalid);
     }
 };
 
+class FailingIODevice : public QIODevice
+{
+public:
+    FailingIODevice(QWebEngineUrlRequestJob *job) : m_job(job)
+    {
+    }
+
+    qint64 readData(char *, qint64) Q_DECL_OVERRIDE
+    {
+        m_job->fail(QWebEngineUrlRequestJob::RequestFailed);
+        return -1;
+    }
+    qint64 writeData(const char *, qint64) Q_DECL_OVERRIDE
+    {
+        m_job->fail(QWebEngineUrlRequestJob::RequestFailed);
+        return -1;
+    }
+    void close() Q_DECL_OVERRIDE
+    {
+        QIODevice::close();
+        deleteLater();
+    }
+
+private:
+    QWebEngineUrlRequestJob *m_job;
+};
+
+class FailOnReadUrlSchemeHandler : public QWebEngineUrlSchemeHandler
+{
+public:
+    void requestStarted(QWebEngineUrlRequestJob *job) override
+    {
+        job->reply(QByteArrayLiteral("text/plain"), new FailingIODevice(job));
+    }
+};
+
+
 void tst_QWebEngineProfile::urlSchemeHandlerFailRequest()
 {
     FailingUrlSchemeHandler handler;
@@ -255,6 +293,20 @@ void tst_QWebEngineProfile::urlSchemeHandlerFailRequest()
     QCOMPARE(toPlainTextSync(view.page()), QString());
 }
 
+void tst_QWebEngineProfile::urlSchemeHandlerFailOnRead()
+{
+    FailOnReadUrlSchemeHandler handler;
+    QWebEngineProfile profile;
+    profile.installUrlSchemeHandler("foo", &handler);
+    QWebEngineView view;
+    QSignalSpy loadFinishedSpy(&view, SIGNAL(loadFinished(bool)));
+    view.setPage(new QWebEnginePage(&profile, &view));
+    view.settings()->setAttribute(QWebEngineSettings::ErrorPageEnabled, false);
+    view.load(QUrl(QStringLiteral("foo://bar")));
+    QVERIFY(loadFinishedSpy.wait());
+    QCOMPARE(toPlainTextSync(view.page()), QString());
+}
+
 void tst_QWebEngineProfile::customUserAgent()
 {
     QString defaultUserAgent = QWebEngineProfile::defaultProfile()->httpUserAgent();