UI/Qt: Request closure instead of always force closing tabs

Author: Lubrsi

UI/Qt/BrowserWindow.cpp

@@ -237,8 +237,8 @@ BrowserWindow::BrowserWindow(Vector<URL::URL> const& initial_urls, IsPopupWindow
 
         set_current_tab(tab);
     });
-    QObject::connect(m_tabs_container, &QTabWidget::tabCloseRequested, this, &BrowserWindow::close_tab);
-    QObject::connect(close_current_tab_action, &QAction::triggered, this, &BrowserWindow::close_current_tab);
+    QObject::connect(m_tabs_container, &QTabWidget::tabCloseRequested, this, &BrowserWindow::request_to_close_tab);
+    QObject::connect(close_current_tab_action, &QAction::triggered, this, &BrowserWindow::request_to_close_current_tab);
 
     for (int i = 0; i <= 7; ++i) {
         new QShortcut(QKeySequence(Qt::CTRL | static_cast<Qt::Key>(Qt::Key_1 + i)), this, [this, i] {
@@ -378,7 +378,7 @@ void BrowserWindow::activate_tab(int index)
     m_tabs_container->setCurrentIndex(index);
 }
 
-void BrowserWindow::close_tab(int index)
+void BrowserWindow::definitely_close_tab(int index)
 {
     auto* tab = m_tabs_container->widget(index);
     m_tabs_container->removeTab(index);
@@ -398,9 +398,15 @@ void BrowserWindow::open_file()
     m_current_tab->open_file();
 }
 
-void BrowserWindow::close_current_tab()
+void BrowserWindow::request_to_close_tab(int index)
 {
-    close_tab(m_tabs_container->currentIndex());
+    auto* tab = as<Tab>(m_tabs_container->widget(index));
+    tab->request_close();
+}
+
+void BrowserWindow::request_to_close_current_tab()
+{
+    request_to_close_tab(m_tabs_container->currentIndex());
 }
 
 int BrowserWindow::tab_index(Tab* tab)
@@ -452,7 +458,7 @@ void BrowserWindow::create_close_button_for_tab(Tab* tab)
 
     connect(button, &QPushButton::clicked, this, [this, tab]() {
         auto index = m_tabs_container->indexOf(tab);
-        close_tab(index);
+        request_to_close_tab(index);
     });
 
     m_tabs_container->tabBar()->setTabButton(index, position, button);
@@ -625,7 +631,7 @@ bool BrowserWindow::eventFilter(QObject* obj, QEvent* event)
             if (obj == m_tabs_container) {
                 auto const tab_index = m_tabs_container->tabBar()->tabAt(mouse_event->pos());
                 if (tab_index != -1) {
-                    close_tab(tab_index);
+                    request_to_close_tab(tab_index);
                     return true;
                 }
             }

UI/Qt/BrowserWindow.h

@@ -63,9 +63,10 @@ public slots:
     Tab& new_tab_from_url(URL::URL const&, Web::HTML::ActivateTab);
     Tab& new_child_tab(Web::HTML::ActivateTab, Tab& parent, Optional<u64> page_index);
     void activate_tab(int index);
-    void close_tab(int index);
+    void definitely_close_tab(int index);
     void move_tab(int old_index, int new_index);
-    void close_current_tab();
+    void request_to_close_tab(int index);
+    void request_to_close_current_tab();
     void open_next_tab();
     void open_previous_tab();
     void open_file();

UI/Qt/Tab.cpp

@@ -115,7 +115,7 @@ Tab::Tab(BrowserWindow* window, RefPtr<WebView::WebContentClient> parent_client,
     };
 
     view().on_close = [this] {
-        m_window->close_tab(tab_index());
+        m_window->definitely_close_tab(tab_index());
     };
 
     view().on_link_hover = [this](auto const& url) {
@@ -374,20 +374,20 @@ Tab::Tab(BrowserWindow* window, RefPtr<WebView::WebContentClient> parent_client,
 
     auto* close_tab_action = new QAction("&Close Tab", this);
     QObject::connect(close_tab_action, &QAction::triggered, this, [this]() {
-        view().on_close();
+        request_close();
     });
 
     auto* close_tabs_to_left_action = new QAction("C&lose Tabs to Left", this);
     QObject::connect(close_tabs_to_left_action, &QAction::triggered, this, [this]() {
         for (auto i = tab_index() - 1; i >= 0; i--) {
-            m_window->close_tab(i);
+            m_window->request_to_close_tab(i);
         }
     });
 
     auto* close_tabs_to_right_action = new QAction("Close Tabs to R&ight", this);
     QObject::connect(close_tabs_to_right_action, &QAction::triggered, this, [this]() {
         for (auto i = m_window->tab_count() - 1; i > tab_index(); i--) {
-            m_window->close_tab(i);
+            m_window->request_to_close_tab(i);
         }
     });
 
@@ -397,7 +397,7 @@ Tab::Tab(BrowserWindow* window, RefPtr<WebView::WebContentClient> parent_client,
             if (i == tab_index())
                 continue;
 
-            m_window->close_tab(i);
+            m_window->request_to_close_tab(i);
         }
     });
 
@@ -513,4 +513,19 @@ void Tab::find_next()
     m_find_in_page->find_next();
 }
 
+void Tab::request_close()
+{
+    // Prevent closing on first request so WebContent can cleanly shutdown (e.g. asking if the user is sure they want
+    // to leave, closing WebSocket connections, etc.)
+    if (!m_already_requested_close) {
+        m_already_requested_close = true;
+        view().request_close();
+        return;
+    }
+
+    // If the user has already requested a close, then respect the user's request and just close the tab.
+    // For example, the WebContent process may not be responding.
+    m_window->definitely_close_tab(tab_index());
+}
+
 }

UI/Qt/Tab.h

@@ -61,6 +61,8 @@ class Tab final : public QWidget {
     void find_previous();
     void find_next();
 
+    void request_close();
+
     QIcon const& favicon() const { return m_favicon; }
     QString const& title() const { return m_title; }
 
@@ -113,6 +115,8 @@ public slots:
     QAction* m_reload_action { nullptr };
 
     QPointer<QDialog> m_dialog;
+
+    bool m_already_requested_close { false };
 };
 
 }