Ladybird/Qt: Do not "share" ownership of Qt objects

When a QObject subclass (widgets, etc.) are provided a parent, then
ownership of that object is passed to the parent. Similarly, objects
added to a QLayout are owned by the layout.

Thus, do not store these objects in an OwnPtr.
This commit is contained in:
Timothy Flynn 2023-12-04 09:39:22 -05:00 committed by Andrew Kaster
parent a21998003c
commit 4653733a0d
9 changed files with 103 additions and 92 deletions

View file

@ -80,13 +80,13 @@ BrowserWindow::BrowserWindow(Vector<URL> const& initial_urls, WebView::CookieJar
auto* edit_menu = menuBar()->addMenu("&Edit");
m_copy_selection_action = make<QAction>("&Copy", this);
m_copy_selection_action = new QAction("&Copy", this);
m_copy_selection_action->setIcon(load_icon_from_uri("resource://icons/16x16/edit-copy.png"sv));
m_copy_selection_action->setShortcuts(QKeySequence::keyBindings(QKeySequence::StandardKey::Copy));
edit_menu->addAction(m_copy_selection_action);
QObject::connect(m_copy_selection_action, &QAction::triggered, this, &BrowserWindow::copy_selected_text);
m_select_all_action = make<QAction>("Select &All", this);
m_select_all_action = new QAction("Select &All", this);
m_select_all_action->setIcon(load_icon_from_uri("resource://icons/16x16/select-all.png"sv));
m_select_all_action->setShortcuts(QKeySequence::keyBindings(QKeySequence::StandardKey::SelectAll));
edit_menu->addAction(m_select_all_action);
@ -163,7 +163,7 @@ BrowserWindow::BrowserWindow(Vector<URL> const& initial_urls, WebView::CookieJar
auto* inspect_menu = menuBar()->addMenu("&Inspect");
m_view_source_action = make<QAction>("View &Source", this);
m_view_source_action = new QAction("View &Source", this);
m_view_source_action->setIcon(load_icon_from_uri("resource://icons/16x16/filetype-html.png"sv));
m_view_source_action->setShortcut(QKeySequence(Qt::CTRL | Qt::Key_U));
inspect_menu->addAction(m_view_source_action);
@ -358,7 +358,12 @@ BrowserWindow::BrowserWindow(Vector<URL> const& initial_urls, WebView::CookieJar
});
QObject::connect(open_file_action, &QAction::triggered, this, &BrowserWindow::open_file);
QObject::connect(settings_action, &QAction::triggered, this, [this] {
new SettingsDialog(this);
if (!m_settings_dialog) {
m_settings_dialog = new SettingsDialog(this);
}
m_settings_dialog->show();
m_settings_dialog->setFocus();
});
QObject::connect(quit_action, &QAction::triggered, this, &QMainWindow::close);
QObject::connect(m_tabs_container, &QTabWidget::currentChanged, [this](int index) {
@ -368,22 +373,22 @@ BrowserWindow::BrowserWindow(Vector<URL> const& initial_urls, WebView::CookieJar
QObject::connect(m_tabs_container, &QTabWidget::tabCloseRequested, this, &BrowserWindow::close_tab);
QObject::connect(close_current_tab_action, &QAction::triggered, this, &BrowserWindow::close_current_tab);
m_inspect_dom_node_action = make<QAction>("&Inspect Element", this);
m_inspect_dom_node_action = new QAction("&Inspect Element", this);
connect(m_inspect_dom_node_action, &QAction::triggered, this, [this] {
if (m_current_tab)
m_current_tab->show_inspector_window(Tab::InspectorTarget::HoveredElement);
});
m_go_back_action = make<QAction>("Go Back");
m_go_back_action = new QAction("Go Back", this);
connect(m_go_back_action, &QAction::triggered, this, [this] {
if (m_current_tab)
m_current_tab->back();
});
m_go_forward_action = make<QAction>("Go Forward");
m_go_forward_action = new QAction("Go Forward", this);
connect(m_go_forward_action, &QAction::triggered, this, [this] {
if (m_current_tab)
m_current_tab->forward();
});
m_reload_action = make<QAction>("&Reload");
m_reload_action = new QAction("&Reload", this);
connect(m_reload_action, &QAction::triggered, this, [this] {
if (m_current_tab)
m_current_tab->reload();
@ -451,22 +456,20 @@ Tab& BrowserWindow::new_tab(StringView html, Web::HTML::ActivateTab activate_tab
Tab& BrowserWindow::create_new_tab(Web::HTML::ActivateTab activate_tab)
{
auto tab = make<Tab>(this, m_web_content_options, m_webdriver_content_ipc_path);
auto tab_ptr = tab.ptr();
m_tabs.append(std::move(tab));
auto* tab = new Tab(this, m_web_content_options, m_webdriver_content_ipc_path);
if (m_current_tab == nullptr) {
set_current_tab(tab_ptr);
set_current_tab(tab);
}
m_tabs_container->addTab(tab_ptr, "New Tab");
m_tabs_container->addTab(tab, "New Tab");
if (activate_tab == Web::HTML::ActivateTab::Yes)
m_tabs_container->setCurrentWidget(tab_ptr);
m_tabs_container->setCurrentWidget(tab);
QObject::connect(tab_ptr, &Tab::title_changed, this, &BrowserWindow::tab_title_changed);
QObject::connect(tab_ptr, &Tab::favicon_changed, this, &BrowserWindow::tab_favicon_changed);
QObject::connect(tab, &Tab::title_changed, this, &BrowserWindow::tab_title_changed);
QObject::connect(tab, &Tab::favicon_changed, this, &BrowserWindow::tab_favicon_changed);
QObject::connect(&tab_ptr->view(), &WebContentView::urls_dropped, this, [this](auto& urls) {
QObject::connect(&tab->view(), &WebContentView::urls_dropped, this, [this](auto& urls) {
VERIFY(urls.size());
m_current_tab->navigate(urls[0].toString());
@ -474,17 +477,17 @@ Tab& BrowserWindow::create_new_tab(Web::HTML::ActivateTab activate_tab)
new_tab(urls[i].toString(), Web::HTML::ActivateTab::No);
});
tab_ptr->view().on_new_tab = [this](auto activate_tab) {
tab->view().on_new_tab = [this](auto activate_tab) {
auto& tab = new_tab("about:blank", activate_tab);
return tab.view().handle();
};
tab_ptr->view().on_tab_open_request = [this](auto url, auto activate_tab) {
tab->view().on_tab_open_request = [this](auto url, auto activate_tab) {
auto& tab = new_tab(qstring_from_ak_string(url.to_deprecated_string()), activate_tab);
return tab.view().handle();
};
tab_ptr->view().on_link_click = [this](auto url, auto target, unsigned modifiers) {
tab->view().on_link_click = [this](auto url, auto target, unsigned modifiers) {
// TODO: maybe activate tabs according to some configuration, this is just normal current browser behavior
if (modifiers == Mod_Ctrl) {
m_current_tab->view().on_tab_open_request(url, Web::HTML::ActivateTab::No);
@ -495,33 +498,33 @@ Tab& BrowserWindow::create_new_tab(Web::HTML::ActivateTab activate_tab)
}
};
tab_ptr->view().on_link_middle_click = [this](auto url, auto target, unsigned modifiers) {
tab->view().on_link_middle_click = [this](auto url, auto target, unsigned modifiers) {
m_current_tab->view().on_link_click(url, target, Mod_Ctrl);
(void)modifiers;
};
tab_ptr->view().on_get_all_cookies = [this](auto const& url) {
tab->view().on_get_all_cookies = [this](auto const& url) {
return m_cookie_jar.get_all_cookies(url);
};
tab_ptr->view().on_get_named_cookie = [this](auto const& url, auto const& name) {
tab->view().on_get_named_cookie = [this](auto const& url, auto const& name) {
return m_cookie_jar.get_named_cookie(url, name);
};
tab_ptr->view().on_get_cookie = [this](auto& url, auto source) -> DeprecatedString {
tab->view().on_get_cookie = [this](auto& url, auto source) -> DeprecatedString {
return m_cookie_jar.get_cookie(url, source);
};
tab_ptr->view().on_set_cookie = [this](auto& url, auto& cookie, auto source) {
tab->view().on_set_cookie = [this](auto& url, auto& cookie, auto source) {
m_cookie_jar.set_cookie(url, cookie, source);
};
tab_ptr->view().on_update_cookie = [this](auto const& cookie) {
tab->view().on_update_cookie = [this](auto const& cookie) {
m_cookie_jar.update_cookie(cookie);
};
tab_ptr->focus_location_editor();
return *tab_ptr;
tab->focus_location_editor();
return *tab;
}
void BrowserWindow::activate_tab(int index)
@ -533,9 +536,7 @@ void BrowserWindow::close_tab(int index)
{
auto* tab = m_tabs_container->widget(index);
m_tabs_container->removeTab(index);
m_tabs.remove_first_matching([&](auto& entry) {
return entry == tab;
});
tab->deleteLater();
}
void BrowserWindow::open_file()
@ -592,23 +593,23 @@ void BrowserWindow::open_previous_tab()
void BrowserWindow::enable_auto_color_scheme()
{
for (auto& tab : m_tabs) {
tab->view().set_preferred_color_scheme(Web::CSS::PreferredColorScheme::Auto);
}
for_each_tab([](auto& tab) {
tab.view().set_preferred_color_scheme(Web::CSS::PreferredColorScheme::Auto);
});
}
void BrowserWindow::enable_light_color_scheme()
{
for (auto& tab : m_tabs) {
tab->view().set_preferred_color_scheme(Web::CSS::PreferredColorScheme::Light);
}
for_each_tab([](auto& tab) {
tab.view().set_preferred_color_scheme(Web::CSS::PreferredColorScheme::Light);
});
}
void BrowserWindow::enable_dark_color_scheme()
{
for (auto& tab : m_tabs) {
tab->view().set_preferred_color_scheme(Web::CSS::PreferredColorScheme::Dark);
}
for_each_tab([](auto& tab) {
tab.view().set_preferred_color_scheme(Web::CSS::PreferredColorScheme::Dark);
});
}
void BrowserWindow::zoom_in()
@ -671,17 +672,19 @@ void BrowserWindow::copy_selected_text()
void BrowserWindow::resizeEvent(QResizeEvent* event)
{
QWidget::resizeEvent(event);
for (auto& tab : m_tabs) {
tab->view().set_window_size({ frameSize().width(), frameSize().height() });
}
for_each_tab([&](auto& tab) {
tab.view().set_window_size({ frameSize().width(), frameSize().height() });
});
}
void BrowserWindow::moveEvent(QMoveEvent* event)
{
QWidget::moveEvent(event);
for (auto& tab : m_tabs) {
tab->view().set_window_position({ event->pos().x(), event->pos().y() });
}
for_each_tab([&](auto& tab) {
tab.view().set_window_position({ event->pos().x(), event->pos().y() });
});
}
void BrowserWindow::wheelEvent(QWheelEvent* event)

View file

@ -21,6 +21,7 @@
namespace Ladybird {
class SettingsDialog;
class WebContentView;
class BrowserWindow : public QMainWindow {
@ -105,18 +106,28 @@ private:
void set_current_tab(Tab* tab);
void update_displayed_zoom_level();
template<typename Callback>
void for_each_tab(Callback&& callback)
{
for (int i = 0; i < m_tabs_container->count(); ++i) {
auto& tab = verify_cast<Tab>(*m_tabs_container->widget(i));
callback(tab);
}
}
QTabWidget* m_tabs_container { nullptr };
Vector<NonnullOwnPtr<Tab>> m_tabs;
Tab* m_current_tab { nullptr };
QMenu* m_zoom_menu { nullptr };
OwnPtr<QAction> m_go_back_action {};
OwnPtr<QAction> m_go_forward_action {};
OwnPtr<QAction> m_reload_action {};
OwnPtr<QAction> m_copy_selection_action {};
OwnPtr<QAction> m_select_all_action {};
OwnPtr<QAction> m_view_source_action {};
OwnPtr<QAction> m_inspect_dom_node_action {};
QAction* m_go_back_action { nullptr };
QAction* m_go_forward_action { nullptr };
QAction* m_reload_action { nullptr };
QAction* m_copy_selection_action { nullptr };
QAction* m_select_all_action { nullptr };
QAction* m_view_source_action { nullptr };
QAction* m_inspect_dom_node_action { nullptr };
SettingsDialog* m_settings_dialog { nullptr };
WebView::CookieJar& m_cookie_jar;

View file

@ -15,7 +15,7 @@ extern bool is_using_dark_system_theme(QWidget&);
InspectorWidget::InspectorWidget(WebContentView& content_view)
{
m_inspector_view = make<WebContentView>(WebContentOptions {}, StringView {});
m_inspector_view = new WebContentView({}, {});
if (is_using_dark_system_theme(*this))
m_inspector_view->update_palette(WebContentView::PaletteMode::Dark);
@ -23,7 +23,7 @@ InspectorWidget::InspectorWidget(WebContentView& content_view)
m_inspector_client = make<WebView::InspectorClient>(content_view, *m_inspector_view);
setLayout(new QVBoxLayout);
layout()->addWidget(m_inspector_view.ptr());
layout()->addWidget(m_inspector_view);
setWindowTitle("Inspector");
resize(875, 825);

View file

@ -30,7 +30,7 @@ public:
private:
void closeEvent(QCloseEvent*) override;
OwnPtr<WebContentView> m_inspector_view;
WebContentView* m_inspector_view;
OwnPtr<WebView::InspectorClient> m_inspector_client;
};

View file

@ -16,25 +16,26 @@
namespace Ladybird {
SettingsDialog::SettingsDialog(QMainWindow* window)
: m_window(window)
: QDialog(window)
, m_window(window)
{
m_layout = new QFormLayout(this);
m_enable_search = make<QCheckBox>(this);
m_enable_search = new QCheckBox(this);
m_enable_search->setChecked(Settings::the()->enable_search());
m_search_engine_dropdown = make<QPushButton>(this);
m_search_engine_dropdown = new QPushButton(this);
m_search_engine_dropdown->setText(qstring_from_ak_string(Settings::the()->search_engine().name));
m_search_engine_dropdown->setMaximumWidth(200);
m_enable_autocomplete = make<QCheckBox>(this);
m_enable_autocomplete = new QCheckBox(this);
m_enable_autocomplete->setChecked(Settings::the()->enable_autocomplete());
m_autocomplete_engine_dropdown = make<QPushButton>(this);
m_autocomplete_engine_dropdown = new QPushButton(this);
m_autocomplete_engine_dropdown->setText(Settings::the()->autocomplete_engine().name);
m_autocomplete_engine_dropdown->setMaximumWidth(200);
m_new_tab_page = make<QLineEdit>(this);
m_new_tab_page = new QLineEdit(this);
m_new_tab_page->setText(Settings::the()->new_tab_page());
QObject::connect(m_new_tab_page, &QLineEdit::textChanged, this, [this] {
auto url_string = ak_string_from_qstring(m_new_tab_page->text());
@ -60,9 +61,6 @@ SettingsDialog::SettingsDialog(QMainWindow* window)
setWindowTitle("Settings");
setLayout(m_layout);
resize(600, 250);
show();
setFocus();
}
void SettingsDialog::setup_search_engines()

View file

@ -5,7 +5,6 @@
* SPDX-License-Identifier: BSD-2-Clause
*/
#include <AK/OwnPtr.h>
#include <QCheckBox>
#include <QDialog>
#include <QFormLayout>
@ -19,6 +18,7 @@ namespace Ladybird {
class SettingsDialog : public QDialog {
Q_OBJECT
public:
explicit SettingsDialog(QMainWindow* window);
@ -27,11 +27,11 @@ private:
QFormLayout* m_layout;
QMainWindow* m_window { nullptr };
OwnPtr<QLineEdit> m_new_tab_page;
OwnPtr<QCheckBox> m_enable_search;
OwnPtr<QPushButton> m_search_engine_dropdown;
OwnPtr<QCheckBox> m_enable_autocomplete;
OwnPtr<QPushButton> m_autocomplete_engine_dropdown;
QLineEdit* m_new_tab_page { nullptr };
QCheckBox* m_enable_search { nullptr };
QPushButton* m_search_engine_dropdown { nullptr };
QCheckBox* m_enable_autocomplete { nullptr };
QPushButton* m_autocomplete_engine_dropdown { nullptr };
};
}

View file

@ -309,7 +309,7 @@ Tab::Tab(BrowserWindow* window, WebContentOptions const& web_content_options, St
}
});
m_page_context_menu = make<QMenu>("Context menu", this);
m_page_context_menu = new QMenu("Context menu", this);
m_page_context_menu->addAction(&m_window->go_back_action());
m_page_context_menu->addAction(&m_window->go_forward_action());
m_page_context_menu->addAction(&m_window->reload_action());
@ -361,7 +361,7 @@ Tab::Tab(BrowserWindow* window, WebContentOptions const& web_content_options, St
copy_link_url(m_link_context_menu_url);
});
m_link_context_menu = make<QMenu>("Link context menu", this);
m_link_context_menu = new QMenu("Link context menu", this);
m_link_context_menu->addAction(open_link_action);
m_link_context_menu->addAction(open_link_in_new_tab_action);
m_link_context_menu->addSeparator();
@ -413,7 +413,7 @@ Tab::Tab(BrowserWindow* window, WebContentOptions const& web_content_options, St
copy_link_url(m_image_context_menu_url);
});
m_image_context_menu = make<QMenu>("Image context menu", this);
m_image_context_menu = new QMenu("Image context menu", this);
m_image_context_menu->addAction(open_image_action);
m_image_context_menu->addAction(open_image_in_new_tab_action);
m_image_context_menu->addSeparator();
@ -435,25 +435,25 @@ Tab::Tab(BrowserWindow* window, WebContentOptions const& web_content_options, St
m_media_context_menu_mute_icon = load_icon_from_uri("resource://icons/16x16/audio-volume-muted.png"sv);
m_media_context_menu_unmute_icon = load_icon_from_uri("resource://icons/16x16/audio-volume-high.png"sv);
m_media_context_menu_play_pause_action = make<QAction>("&Play", this);
m_media_context_menu_play_pause_action = new QAction("&Play", this);
m_media_context_menu_play_pause_action->setIcon(m_media_context_menu_play_icon);
QObject::connect(m_media_context_menu_play_pause_action, &QAction::triggered, this, [this]() {
view().toggle_media_play_state();
});
m_media_context_menu_mute_unmute_action = make<QAction>("&Mute", this);
m_media_context_menu_mute_unmute_action = new QAction("&Mute", this);
m_media_context_menu_mute_unmute_action->setIcon(m_media_context_menu_mute_icon);
QObject::connect(m_media_context_menu_mute_unmute_action, &QAction::triggered, this, [this]() {
view().toggle_media_mute_state();
});
m_media_context_menu_controls_action = make<QAction>("Show &Controls", this);
m_media_context_menu_controls_action = new QAction("Show &Controls", this);
m_media_context_menu_controls_action->setCheckable(true);
QObject::connect(m_media_context_menu_controls_action, &QAction::triggered, this, [this]() {
view().toggle_media_controls_state();
});
m_media_context_menu_loop_action = make<QAction>("&Loop", this);
m_media_context_menu_loop_action = new QAction("&Loop", this);
m_media_context_menu_loop_action->setCheckable(true);
QObject::connect(m_media_context_menu_loop_action, &QAction::triggered, this, [this]() {
view().toggle_media_loop_state();
@ -477,7 +477,7 @@ Tab::Tab(BrowserWindow* window, WebContentOptions const& web_content_options, St
copy_link_url(m_media_context_menu_url);
});
m_audio_context_menu = make<QMenu>("Audio context menu", this);
m_audio_context_menu = new QMenu("Audio context menu", this);
m_audio_context_menu->addAction(m_media_context_menu_play_pause_action);
m_audio_context_menu->addAction(m_media_context_menu_mute_unmute_action);
m_audio_context_menu->addAction(m_media_context_menu_controls_action);
@ -508,7 +508,7 @@ Tab::Tab(BrowserWindow* window, WebContentOptions const& web_content_options, St
copy_link_url(m_media_context_menu_url);
});
m_video_context_menu = make<QMenu>("Video context menu", this);
m_video_context_menu = new QMenu("Video context menu", this);
m_video_context_menu->addAction(m_media_context_menu_play_pause_action);
m_video_context_menu->addAction(m_media_context_menu_mute_unmute_action);
m_video_context_menu->addAction(m_media_context_menu_controls_action);

View file

@ -72,7 +72,7 @@ private:
void close_sub_widgets();
QBoxLayout* m_layout;
QBoxLayout* m_layout { nullptr };
QToolBar* m_toolbar { nullptr };
QToolButton* m_reset_zoom_button { nullptr };
QAction* m_reset_zoom_button_action { nullptr };
@ -83,26 +83,26 @@ private:
QString m_title;
QLabel* m_hover_label { nullptr };
OwnPtr<QMenu> m_page_context_menu;
QMenu* m_page_context_menu { nullptr };
Optional<String> m_page_context_menu_search_text;
OwnPtr<QMenu> m_link_context_menu;
QMenu* m_link_context_menu { nullptr };
URL m_link_context_menu_url;
OwnPtr<QMenu> m_image_context_menu;
QMenu* m_image_context_menu { nullptr };
Gfx::ShareableBitmap m_image_context_menu_bitmap;
URL m_image_context_menu_url;
OwnPtr<QMenu> m_audio_context_menu;
OwnPtr<QMenu> m_video_context_menu;
QMenu* m_audio_context_menu { nullptr };
QMenu* m_video_context_menu { nullptr };
QIcon m_media_context_menu_play_icon;
QIcon m_media_context_menu_pause_icon;
QIcon m_media_context_menu_mute_icon;
QIcon m_media_context_menu_unmute_icon;
OwnPtr<QAction> m_media_context_menu_play_pause_action;
OwnPtr<QAction> m_media_context_menu_mute_unmute_action;
OwnPtr<QAction> m_media_context_menu_controls_action;
OwnPtr<QAction> m_media_context_menu_loop_action;
QAction* m_media_context_menu_play_pause_action { nullptr };
QAction* m_media_context_menu_mute_unmute_action { nullptr };
QAction* m_media_context_menu_controls_action { nullptr };
QAction* m_media_context_menu_loop_action { nullptr };
URL m_media_context_menu_url;
int tab_index();

View file

@ -8,7 +8,6 @@
#include "EventLoopImplementationQt.h"
#include "Settings.h"
#include "WebContentView.h"
#include <AK/OwnPtr.h>
#include <Ladybird/HelperProcess.h>
#include <Ladybird/Utilities.h>
#include <LibCore/ArgsParser.h>