From 2694ec6605891271ed99f69a03b3929178655963 Mon Sep 17 00:00:00 2001
From: Jocelyn Turcotte <jocelyn.turcotte@digia.com>
Date: Thu, 14 Aug 2014 17:45:07 +0200
Subject: [PATCH] Fix a crash in WebEngineSettings when opening a new window
A new window means that the QWebEnginePage gets its WebContentsAdapter
swapped and that the pointer in WebEngineSettings must be updated.
Do the WebContentsAdapter-to-WebEngineSettings binding in
WebContentsAdapter::initialize to cover both cases.
This also refactors the way that QWebEngineSettings is created by
removing the need to pass a QWebEngineSettingsPrivate instance to be
adopted, and also move the global settings construction logic in
the singleton accessor instead of relying on the fact that it uses
a different contructor.
Change-Id: I6f8a2ed1407a4b25f9898526db9432721c354ddf
Reviewed-by: Andras Becsi <andras.becsi@digia.com>
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@digia.com>
---
src/core/web_contents_adapter.cpp | 3 +++
src/core/web_engine_settings.cpp | 12 ++---------
src/core/web_engine_settings.h | 3 ++-
src/webenginewidgets/api/qwebenginepage.cpp | 2 +-
.../api/qwebenginesettings.cpp | 20 ++++++-------------
src/webenginewidgets/api/qwebenginesettings.h | 1 -
.../api/qwebenginesettings_p.h | 1 -
7 files changed, 14 insertions(+), 28 deletions(-)
diff --git a/src/core/web_contents_adapter.cpp b/src/core/web_contents_adapter.cpp
index d2eb076d6..a4a8bbe23 100644
--- a/src/core/web_contents_adapter.cpp
+++ b/src/core/web_contents_adapter.cpp
@@ -357,6 +357,9 @@ void WebContentsAdapter::initialize(WebContentsAdapterClient *adapterClient)
if (!d->webContents)
d->webContents.reset(createBlankWebContents(adapterClient));
+ // This might replace any adapter that has been initialized with this WebEngineSettings.
+ adapterClient->webEngineSettings()->setWebContentsAdapter(this);
+
content::RendererPreferences* rendererPrefs = d->webContents->GetMutableRendererPrefs();
rendererPrefs->use_custom_colors = true;
// Qt returns a flash time (the whole cycle) in ms, chromium expects just the interval in seconds
diff --git a/src/core/web_engine_settings.cpp b/src/core/web_engine_settings.cpp
index 4d0e727e3..507866e56 100644
--- a/src/core/web_engine_settings.cpp
+++ b/src/core/web_engine_settings.cpp
@@ -82,14 +82,6 @@ WebEngineSettings::WebEngineSettings(WebEngineSettingsDelegate *delegate)
Q_ASSERT(delegate);
}
-WebEngineSettings::WebEngineSettings(WebEngineSettingsDelegate *delegate, WebContentsAdapter *adapter)
- : m_adapter(adapter)
- , m_delegate(delegate)
- , m_batchTimer(new BatchTimer(this))
-{
- Q_ASSERT(delegate);
-}
-
WebEngineSettings::~WebEngineSettings()
{
}
@@ -252,8 +244,8 @@ void WebEngineSettings::doApply()
// FIXME: batch sequential calls to apply?
applySettingsToWebPreferences(webPreferences.data());
- if (m_adapter)
- m_adapter->updateWebPreferences(*webPreferences.data());
+ Q_ASSERT(m_adapter);
+ m_adapter->updateWebPreferences(*webPreferences.data());
}
void WebEngineSettings::applySettingsToWebPreferences(WebPreferences *prefs)
diff --git a/src/core/web_engine_settings.h b/src/core/web_engine_settings.h
index 5e19c242d..a1c41c187 100644
--- a/src/core/web_engine_settings.h
+++ b/src/core/web_engine_settings.h
@@ -99,7 +99,6 @@ public:
};
WebEngineSettings(WebEngineSettingsDelegate*);
- WebEngineSettings(WebEngineSettingsDelegate *,WebContentsAdapter *);
virtual ~WebEngineSettings();
void overrideWebPreferences(WebPreferences *prefs);
@@ -125,6 +124,7 @@ public:
private:
void doApply();
void applySettingsToWebPreferences(WebPreferences *);
+ void setWebContentsAdapter(WebContentsAdapter *adapter) { m_adapter = adapter; }
WebContentsAdapter* m_adapter;
WebEngineSettingsDelegate* m_delegate;
@@ -136,6 +136,7 @@ private:
QScopedPointer<BatchTimer> m_batchTimer;
friend class BatchTimer;
+ friend class WebContentsAdapter;
};
#endif // WEB_ENGINE_SETTINGS_H
diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp
index 71099e547..4cf35f8c4 100644
--- a/src/webenginewidgets/api/qwebenginepage.cpp
+++ b/src/webenginewidgets/api/qwebenginepage.cpp
@@ -168,7 +168,7 @@ void CallbackDirectory::CallbackSharedDataPointer::doDeref()
QWebEnginePagePrivate::QWebEnginePagePrivate()
: adapter(new WebContentsAdapter)
, history(new QWebEngineHistory(new QWebEngineHistoryPrivate(this)))
- , settings(new QWebEngineSettings(new QWebEngineSettingsPrivate(adapter.data())))
+ , settings(new QWebEngineSettings)
, view(0)
{
memset(actions, 0, sizeof(actions));
diff --git a/src/webenginewidgets/api/qwebenginesettings.cpp b/src/webenginewidgets/api/qwebenginesettings.cpp
index 52cc59a80..b2a57952f 100644
--- a/src/webenginewidgets/api/qwebenginesettings.cpp
+++ b/src/webenginewidgets/api/qwebenginesettings.cpp
@@ -84,15 +84,11 @@ QWebEngineSettingsPrivate::QWebEngineSettingsPrivate()
{
}
-QWebEngineSettingsPrivate::QWebEngineSettingsPrivate(WebContentsAdapter *adapter)
- : coreSettings(new WebEngineSettings(this, adapter))
-{
-}
-
void QWebEngineSettingsPrivate::apply()
{
coreSettings->scheduleApply();
QWebEngineSettingsPrivate *globals = QWebEngineSettings::globalSettings()->d;
+ Q_ASSERT((this == globals) != (allSettings->contains(this)));
if (this == globals) {
Q_FOREACH (QWebEngineSettingsPrivate *settings, *allSettings)
settings->coreSettings->scheduleApply();
@@ -111,8 +107,12 @@ WebEngineSettings *QWebEngineSettingsPrivate::fallbackSettings() const {
QWebEngineSettings *QWebEngineSettings::globalSettings()
{
static QWebEngineSettings* globalSettings = 0;
- if (!globalSettings)
+ if (!globalSettings) {
globalSettings = new QWebEngineSettings;
+ // globalSettings shouldn't be in that list.
+ allSettings->removeAll(globalSettings->d);
+ globalSettings->d->initDefaults();
+ }
return globalSettings;
}
@@ -162,14 +162,6 @@ void QWebEngineSettings::resetFontSize(QWebEngineSettings::FontSize type)
QWebEngineSettings::QWebEngineSettings()
: d(new QWebEngineSettingsPrivate)
{
- d->initDefaults();
-}
-
-
-QWebEngineSettings::QWebEngineSettings(QWebEngineSettingsPrivate *p)
- : d(p)
-{
- Q_ASSERT(d);
allSettings->append(d);
d->coreSettings->scheduleApply();
}
diff --git a/src/webenginewidgets/api/qwebenginesettings.h b/src/webenginewidgets/api/qwebenginesettings.h
index bbb0cf899..c8af3a1b1 100644
--- a/src/webenginewidgets/api/qwebenginesettings.h
+++ b/src/webenginewidgets/api/qwebenginesettings.h
@@ -87,7 +87,6 @@ private:
friend class QWebEngineSettingsPrivate;
QWebEngineSettings();
- QWebEngineSettings(QWebEngineSettingsPrivate *);
~QWebEngineSettings();
QWebEngineSettingsPrivate *d;
diff --git a/src/webenginewidgets/api/qwebenginesettings_p.h b/src/webenginewidgets/api/qwebenginesettings_p.h
index 96fa9e27b..dda39a37d 100644
--- a/src/webenginewidgets/api/qwebenginesettings_p.h
+++ b/src/webenginewidgets/api/qwebenginesettings_p.h
@@ -52,7 +52,6 @@ class QWebEngineSettingsPrivate : public WebEngineSettingsDelegate {
public:
QWebEngineSettingsPrivate();
- QWebEngineSettingsPrivate(WebContentsAdapter *adapter);
void initDefaults();
void apply() Q_DECL_OVERRIDE;
--
GitLab