Unify certificate error handling

Before we move the certificate error to core, unify
error handling between qml and widgets.

[ChangeLog]  WebPage::certificateError does not return bool
it is enough to call defer, ignoreCertificateError()
or rejectCertificate().

Change-Id: I14be40f1ab6b4cbded550260d66152f51cf47de2
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

6 files changed, 21 insertions, 23 deletions

diff --git a/examples/webenginewidgets/simplebrowser/webpage.cpp b/examples/webenginewidgets/simplebrowser/webpage.cpp
index 417b29b52..d8f954c90 100644
--- a/examples/webenginewidgets/simplebrowser/webpage.cpp
+++ b/examples/webenginewidgets/simplebrowser/webpage.cpp
@@ -70,7 +70,7 @@ WebPage::WebPage(QWebEngineProfile *profile, QObject *parent)
     connect(this, &QWebEnginePage::selectClientCertificate, this, &WebPage::handleSelectClientCertificate);
 }
 
-bool WebPage::certificateError(const QWebEngineCertificateError &error)
+void WebPage::certificateError(const QWebEngineCertificateError &error)
 {
     QWidget *mainWindow = view()->window();
 
@@ -99,7 +99,6 @@ bool WebPage::certificateError(const QWebEngineCertificateError &error)
                 error.rejectCertificate();
         }
     });
-    return true;
 }
 
 void WebPage::handleAuthenticationRequired(const QUrl &requestUrl, QAuthenticator *auth)
diff --git a/examples/webenginewidgets/simplebrowser/webpage.h b/examples/webenginewidgets/simplebrowser/webpage.h
index d3f50bd6e..fbb15fc0f 100644
--- a/examples/webenginewidgets/simplebrowser/webpage.h
+++ b/examples/webenginewidgets/simplebrowser/webpage.h
@@ -62,7 +62,7 @@ public:
     WebPage(QWebEngineProfile *profile, QObject *parent = nullptr);
 
 protected:
-    bool certificateError(const QWebEngineCertificateError &error) override;
+    void certificateError(const QWebEngineCertificateError &error) override;
 
 private slots:
     void handleAuthenticationRequired(const QUrl &requestUrl, QAuthenticator *auth);
diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index f5bec06fb..3c8f6e488 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -1723,14 +1723,12 @@ void QWebEnginePagePrivate::javascriptDialog(QSharedPointer<JavaScriptDialogCont
 void QWebEnginePagePrivate::allowCertificateError(const QSharedPointer<CertificateErrorController> &controller)
 {
     Q_Q(QWebEnginePage);
-    bool accepted = false;
-
     QWebEngineCertificateError error(controller);
-    accepted = q->certificateError(error);
-    if (error.deferred() && !error.answered())
+    q->certificateError(error);
+    if (!error.isOverridable() || (!error.deferred() && !error.answered()))
+        error.rejectCertificate();
+    else
         m_certificateErrorControllers.append(controller);
-    else if (!error.answered())
-        controller->accept(error.isOverridable() && accepted);
 }
 
 void QWebEnginePagePrivate::selectClientCert(const QSharedPointer<ClientCertSelectController> &controller)
@@ -2390,10 +2388,7 @@ void QWebEnginePage::javaScriptConsoleMessage(JavaScriptConsoleMessageLevel leve
     }
 }
 
-bool QWebEnginePage::certificateError(const QWebEngineCertificateError &)
-{
-    return false;
-}
+void QWebEnginePage::certificateError(const QWebEngineCertificateError &) { }
 
 bool QWebEnginePage::acceptNavigationRequest(const QUrl &url, NavigationType type, bool isMainFrame)
 {
diff --git a/src/webenginewidgets/api/qwebenginepage.h b/src/webenginewidgets/api/qwebenginepage.h
index 97f32e43f..c1fff2ac2 100644
--- a/src/webenginewidgets/api/qwebenginepage.h
+++ b/src/webenginewidgets/api/qwebenginepage.h
@@ -375,7 +375,7 @@ protected:
     virtual bool javaScriptConfirm(const QUrl &securityOrigin, const QString& msg);
     virtual bool javaScriptPrompt(const QUrl &securityOrigin, const QString& msg, const QString& defaultValue, QString* result);
     virtual void javaScriptConsoleMessage(JavaScriptConsoleMessageLevel level, const QString& message, int lineNumber, const QString& sourceID);
-    virtual bool certificateError(const QWebEngineCertificateError &certificateError);
+    virtual void certificateError(const QWebEngineCertificateError &certificateError);
     virtual bool acceptNavigationRequest(const QUrl &url, NavigationType type, bool isMainFrame);
 
 private:
diff --git a/src/webenginewidgets/doc/src/qwebenginepage_lgpl.qdoc b/src/webenginewidgets/doc/src/qwebenginepage_lgpl.qdoc
index 789d9794e..5def0d65c 100644
--- a/src/webenginewidgets/doc/src/qwebenginepage_lgpl.qdoc
+++ b/src/webenginewidgets/doc/src/qwebenginepage_lgpl.qdoc
@@ -513,16 +513,13 @@
 */
 
 /*!
-    \fn bool QWebEnginePage::certificateError(const QWebEngineCertificateError & certificateError)
+    \fn void QWebEnginePage::certificateError(const QWebEngineCertificateError & certificateError)
     This function is called when an invalid certificate error is raised while loading a given request.
 
-    The \a certificateError parameter contains information about the certificate and details of the error.
-
-    Return \c true to ignore the error and complete the request. Return \c false to stop loading
+    The \a certificateError parameter contains information about the certificate and details of the error,
+    it also provides the way to ignore the error and complete the request or stop loading
     the request.
 
-    \note If the error was successfully deferred then the returned value will be ignored.
-
     \sa QWebEngineCertificateError
 */
 
diff --git a/tests/auto/widgets/certificateerror/tst_certificateerror.cpp b/tests/auto/widgets/certificateerror/tst_certificateerror.cpp
index f11d9236c..34508be0c 100644
--- a/tests/auto/widgets/certificateerror/tst_certificateerror.cpp
+++ b/tests/auto/widgets/certificateerror/tst_certificateerror.cpp
@@ -57,11 +57,18 @@ struct PageWithCertificateErrorHandler : QWebEnginePage
     QSignalSpy loadSpy;
     QScopedPointer<QWebEngineCertificateError> error;
 
-    bool certificateError(const QWebEngineCertificateError &e) override {
+    void certificateError(const QWebEngineCertificateError &e) override
+    {
         error.reset(new QWebEngineCertificateError(e));
-        if (deferError)
+        if (deferError) {
             error->defer();
-        return acceptCertificate;
+            return;
+        }
+
+        if (acceptCertificate)
+            error->ignoreCertificateError();
+        else
+            error->rejectCertificate();
     }
 };