mirror of
				https://github.com/RGBCube/serenity
				synced 2025-10-31 10:12:45 +00:00 
			
		
		
		
	LibWeb: Make Frame point weakly to Page
This patch makes Page weakable and allows page-less frames to exist. Page is single-owner, and Frame is multiple-owner, so it's not sound for Frame to assume its containing Page will stick around for its own entire lifetime. Fixes #3976.
This commit is contained in:
		
							parent
							
								
									e445ff670d
								
							
						
					
					
						commit
						81add73955
					
				
					 12 changed files with 87 additions and 56 deletions
				
			
		|  | @ -172,7 +172,8 @@ Color IdentifierStyleValue::to_color(const DOM::Document& document) const | |||
|     if (id() == CSS::ValueID::VendorSpecificLink) | ||||
|         return document.link_color(); | ||||
| 
 | ||||
|     auto palette = document.frame()->page().palette(); | ||||
|     ASSERT(document.page()); | ||||
|     auto palette = document.page()->palette(); | ||||
|     switch (id()) { | ||||
|     case CSS::ValueID::VendorSpecificPaletteDesktopBackground: | ||||
|         return palette.color(ColorRole::DesktopBackground); | ||||
|  |  | |||
|  | @ -343,8 +343,10 @@ void Document::layout() | |||
|     m_layout_root->layout(); | ||||
|     m_layout_root->set_needs_display(); | ||||
| 
 | ||||
|     if (frame()->is_main_frame()) | ||||
|         frame()->page().client().page_did_layout(); | ||||
|     if (frame()->is_main_frame()) { | ||||
|         if (auto* page = this->page()) | ||||
|             page->client().page_did_layout(); | ||||
|     } | ||||
| } | ||||
| 
 | ||||
| void Document::update_style() | ||||
|  | @ -446,27 +448,27 @@ Color Document::link_color() const | |||
| { | ||||
|     if (m_link_color.has_value()) | ||||
|         return m_link_color.value(); | ||||
|     if (!frame()) | ||||
|     if (!page()) | ||||
|         return Color::Blue; | ||||
|     return frame()->page().palette().link(); | ||||
|     return page()->palette().link(); | ||||
| } | ||||
| 
 | ||||
| Color Document::active_link_color() const | ||||
| { | ||||
|     if (m_active_link_color.has_value()) | ||||
|         return m_active_link_color.value(); | ||||
|     if (!frame()) | ||||
|     if (!page()) | ||||
|         return Color::Red; | ||||
|     return frame()->page().palette().active_link(); | ||||
|     return page()->palette().active_link(); | ||||
| } | ||||
| 
 | ||||
| Color Document::visited_link_color() const | ||||
| { | ||||
|     if (m_visited_link_color.has_value()) | ||||
|         return m_visited_link_color.value(); | ||||
|     if (!frame()) | ||||
|     if (!page()) | ||||
|         return Color::Magenta; | ||||
|     return frame()->page().palette().visited_link(); | ||||
|     return page()->palette().visited_link(); | ||||
| } | ||||
| 
 | ||||
| static JS::VM& main_thread_vm() | ||||
|  | @ -596,4 +598,14 @@ void Document::set_ready_state(const String& ready_state) | |||
|     dispatch_event(Event::create("readystatechange")); | ||||
| } | ||||
| 
 | ||||
| Page* Document::page() | ||||
| { | ||||
|     return m_frame ? m_frame->page() : nullptr; | ||||
| } | ||||
| 
 | ||||
| const Page* Document::page() const | ||||
| { | ||||
|     return m_frame ? m_frame->page() : nullptr; | ||||
| } | ||||
| 
 | ||||
| } | ||||
|  |  | |||
|  | @ -99,6 +99,9 @@ public: | |||
|     Frame* frame() { return m_frame.ptr(); } | ||||
|     const Frame* frame() const { return m_frame.ptr(); } | ||||
| 
 | ||||
|     Page* page(); | ||||
|     const Page* page() const; | ||||
| 
 | ||||
|     Color background_color(const Gfx::Palette&) const; | ||||
|     RefPtr<Gfx::Bitmap> background_image() const; | ||||
| 
 | ||||
|  |  | |||
|  | @ -61,9 +61,8 @@ void Window::set_wrapper(Badge<Bindings::WindowObject>, Bindings::WindowObject& | |||
| 
 | ||||
| void Window::alert(const String& message) | ||||
| { | ||||
|     if (!m_document.frame()) | ||||
|         return; | ||||
|     m_document.frame()->page().client().page_did_request_alert(message); | ||||
|     if (auto* page = m_document.page()) | ||||
|         page->client().page_did_request_alert(message); | ||||
| } | ||||
| 
 | ||||
| bool Window::confirm(const String& message) | ||||
|  |  | |||
|  | @ -105,7 +105,8 @@ void HTMLFormElement::submit(RefPtr<HTMLInputElement> submitter) | |||
|         request.set_body(body); | ||||
|     } | ||||
| 
 | ||||
|     document().frame()->page().load(request); | ||||
|     if (auto* page = document().page()) | ||||
|         page->load(request); | ||||
| } | ||||
| 
 | ||||
| } | ||||
|  |  | |||
|  | @ -62,9 +62,9 @@ void HTMLInputElement::did_click_button(Badge<LayoutButton>) | |||
| 
 | ||||
| RefPtr<LayoutNode> HTMLInputElement::create_layout_node(const CSS::StyleProperties* parent_style) | ||||
| { | ||||
|     ASSERT(document().frame()); | ||||
|     auto& frame = *document().frame(); | ||||
|     auto& page_view = const_cast<InProcessWebView&>(static_cast<const InProcessWebView&>(frame.page().client())); | ||||
|     ASSERT(document().page()); | ||||
|     auto& page = *document().page(); | ||||
|     auto& page_view = const_cast<InProcessWebView&>(static_cast<const InProcessWebView&>(page.client())); | ||||
| 
 | ||||
|     if (type() == "hidden") | ||||
|         return nullptr; | ||||
|  |  | |||
|  | @ -60,7 +60,7 @@ void LayoutWidget::did_set_rect() | |||
| void LayoutWidget::update_widget() | ||||
| { | ||||
|     auto adjusted_widget_position = absolute_rect().location().to_type<int>(); | ||||
|     auto& page_view = static_cast<const InProcessWebView&>(frame().page().client()); | ||||
|     auto& page_view = static_cast<const InProcessWebView&>(frame().page()->client()); | ||||
|     adjusted_widget_position.move_by(-page_view.horizontal_scrollbar().value(), -page_view.vertical_scrollbar().value()); | ||||
|     widget().move_to(adjusted_widget_position); | ||||
| } | ||||
|  |  | |||
|  | @ -160,8 +160,10 @@ bool FrameLoader::load(const LoadRequest& request, Type type) | |||
| 
 | ||||
|     set_resource(ResourceLoader::the().load_resource(Resource::Type::Generic, request)); | ||||
| 
 | ||||
|     if (type == Type::Navigation) | ||||
|         frame().page().client().page_did_start_loading(url); | ||||
|     if (type == Type::Navigation) { | ||||
|         if (auto* page = frame().page()) | ||||
|             page->client().page_did_start_loading(url); | ||||
|     } | ||||
| 
 | ||||
|     if (type == Type::IFrame) | ||||
|         return true; | ||||
|  | @ -184,7 +186,8 @@ bool FrameLoader::load(const LoadRequest& request, Type type) | |||
|                     return; | ||||
|                 } | ||||
|                 dbg() << "Decoded favicon, " << bitmap->size(); | ||||
|                 frame().page().client().page_did_change_favicon(*bitmap); | ||||
|                 if (auto* page = frame().page()) | ||||
|                     page->client().page_did_change_favicon(*bitmap); | ||||
|             }); | ||||
|     } | ||||
| 
 | ||||
|  |  | |||
|  | @ -125,7 +125,6 @@ bool EventHandler::handle_mousedown(const Gfx::IntPoint& position, unsigned butt | |||
|     } | ||||
| 
 | ||||
|     NonnullRefPtr document = *m_frame.document(); | ||||
|     auto& page_client = m_frame.page().client(); | ||||
| 
 | ||||
|     auto result = layout_root()->hit_test(position, HitTestType::Exact); | ||||
|     if (!result.layout_node) | ||||
|  | @ -148,7 +147,8 @@ bool EventHandler::handle_mousedown(const Gfx::IntPoint& position, unsigned butt | |||
|         return false; | ||||
|     } | ||||
| 
 | ||||
|     m_frame.page().set_focused_frame({}, m_frame); | ||||
|     if (auto* page = m_frame.page()) | ||||
|         page->set_focused_frame({}, m_frame); | ||||
| 
 | ||||
|     auto offset = compute_mouse_event_offset(position, *result.layout_node); | ||||
|     node->dispatch_event(UIEvents::MouseEvent::create("mousedown", offset.x(), offset.y())); | ||||
|  | @ -158,7 +158,8 @@ bool EventHandler::handle_mousedown(const Gfx::IntPoint& position, unsigned butt | |||
|     if (button == GUI::MouseButton::Right && is<HTML::HTMLImageElement>(*node)) { | ||||
|         auto& image_element = downcast<HTML::HTMLImageElement>(*node); | ||||
|         auto image_url = image_element.document().complete_url(image_element.src()); | ||||
|         page_client.page_did_request_image_context_menu(m_frame.to_main_frame_position(position), image_url, "", modifiers, image_element.bitmap()); | ||||
|         if (auto* page = m_frame.page()) | ||||
|             page->client().page_did_request_image_context_menu(m_frame.to_main_frame_position(position), image_url, "", modifiers, image_element.bitmap()); | ||||
|         return true; | ||||
|     } | ||||
| 
 | ||||
|  | @ -176,16 +177,19 @@ bool EventHandler::handle_mousedown(const Gfx::IntPoint& position, unsigned butt | |||
|                 m_frame.scroll_to_anchor(anchor); | ||||
|             } else { | ||||
|                 if (m_frame.is_main_frame()) { | ||||
|                     page_client.page_did_click_link(url, link->target(), modifiers); | ||||
|                     if (auto* page = m_frame.page()) | ||||
|                         page->client().page_did_click_link(url, link->target(), modifiers); | ||||
|                 } else { | ||||
|                     // FIXME: Handle different targets!
 | ||||
|                     m_frame.loader().load(url, FrameLoader::Type::Navigation); | ||||
|                 } | ||||
|             } | ||||
|         } else if (button == GUI::MouseButton::Right) { | ||||
|             page_client.page_did_request_link_context_menu(m_frame.to_main_frame_position(position), url, link->target(), modifiers); | ||||
|             if (auto* page = m_frame.page()) | ||||
|                 page->client().page_did_request_link_context_menu(m_frame.to_main_frame_position(position), url, link->target(), modifiers); | ||||
|         } else if (button == GUI::MouseButton::Middle) { | ||||
|             page_client.page_did_middle_click_link(url, link->target(), modifiers); | ||||
|             if (auto* page = m_frame.page()) | ||||
|                 page->client().page_did_middle_click_link(url, link->target(), modifiers); | ||||
|         } | ||||
|     } else { | ||||
|         if (button == GUI::MouseButton::Left) { | ||||
|  | @ -197,7 +201,8 @@ bool EventHandler::handle_mousedown(const Gfx::IntPoint& position, unsigned butt | |||
|                 m_in_mouse_selection = true; | ||||
|             } | ||||
|         } else if (button == GUI::MouseButton::Right) { | ||||
|             page_client.page_did_request_context_menu(m_frame.to_main_frame_position(position)); | ||||
|             if (auto* page = m_frame.page()) | ||||
|                 page->client().page_did_request_context_menu(m_frame.to_main_frame_position(position)); | ||||
|         } | ||||
|     } | ||||
|     return true; | ||||
|  | @ -214,7 +219,6 @@ bool EventHandler::handle_mousemove(const Gfx::IntPoint& position, unsigned butt | |||
|     } | ||||
| 
 | ||||
|     auto& document = *m_frame.document(); | ||||
|     auto& page_client = m_frame.page().client(); | ||||
| 
 | ||||
|     bool hovered_node_changed = false; | ||||
|     bool is_hovering_link = false; | ||||
|  | @ -227,7 +231,8 @@ bool EventHandler::handle_mousemove(const Gfx::IntPoint& position, unsigned butt | |||
|             document.set_hovered_node(result.layout_node->node()); | ||||
|             result.layout_node->handle_mousemove({}, position, buttons, modifiers); | ||||
|             // FIXME: It feels a bit aggressive to always update the cursor like this.
 | ||||
|             page_client.page_did_request_cursor_change(Gfx::StandardCursor::None); | ||||
|             if (auto* page = m_frame.page()) | ||||
|                 page->client().page_did_request_cursor_change(Gfx::StandardCursor::None); | ||||
|             return true; | ||||
|         } | ||||
| 
 | ||||
|  | @ -262,28 +267,31 @@ bool EventHandler::handle_mousemove(const Gfx::IntPoint& position, unsigned butt | |||
|                 layout_root()->set_selection_end({ hit.layout_node, hit.index_in_node }); | ||||
|             } | ||||
|             dump_selection("MouseMove"); | ||||
|             page_client.page_did_change_selection(); | ||||
|             if (auto* page = m_frame.page()) | ||||
|                 page->client().page_did_change_selection(); | ||||
|         } | ||||
|     } | ||||
| 
 | ||||
|     if (auto* page = m_frame.page()) { | ||||
|         if (is_hovering_link) | ||||
|         page_client.page_did_request_cursor_change(Gfx::StandardCursor::Hand); | ||||
|             page->client().page_did_request_cursor_change(Gfx::StandardCursor::Hand); | ||||
|         else if (is_hovering_text) | ||||
|         page_client.page_did_request_cursor_change(Gfx::StandardCursor::IBeam); | ||||
|             page->client().page_did_request_cursor_change(Gfx::StandardCursor::IBeam); | ||||
|         else | ||||
|         page_client.page_did_request_cursor_change(Gfx::StandardCursor::None); | ||||
|             page->client().page_did_request_cursor_change(Gfx::StandardCursor::None); | ||||
| 
 | ||||
|         if (hovered_node_changed) { | ||||
|             RefPtr<HTML::HTMLElement> hovered_html_element = document.hovered_node() ? document.hovered_node()->enclosing_html_element() : nullptr; | ||||
|             if (hovered_html_element && !hovered_html_element->title().is_null()) { | ||||
|             page_client.page_did_enter_tooltip_area(m_frame.to_main_frame_position(position), hovered_html_element->title()); | ||||
|                 page->client().page_did_enter_tooltip_area(m_frame.to_main_frame_position(position), hovered_html_element->title()); | ||||
|             } else { | ||||
|             page_client.page_did_leave_tooltip_area(); | ||||
|                 page->client().page_did_leave_tooltip_area(); | ||||
|             } | ||||
|             if (is_hovering_link) | ||||
|             page_client.page_did_hover_link(document.complete_url(hovered_link_element->href())); | ||||
|                 page->client().page_did_hover_link(document.complete_url(hovered_link_element->href())); | ||||
|             else | ||||
|             page_client.page_did_unhover_link(); | ||||
|                 page->client().page_did_unhover_link(); | ||||
|         } | ||||
|     } | ||||
|     return true; | ||||
| } | ||||
|  |  | |||
|  | @ -36,7 +36,7 @@ | |||
| namespace Web { | ||||
| 
 | ||||
| Frame::Frame(DOM::Element& host_element, Frame& main_frame) | ||||
|     : m_page(main_frame.page()) | ||||
|     : m_page(*main_frame.page()) | ||||
|     , m_main_frame(main_frame) | ||||
|     , m_loader(*this) | ||||
|     , m_event_handler({}, *this) | ||||
|  | @ -72,7 +72,7 @@ void Frame::setup() | |||
| 
 | ||||
| bool Frame::is_focused_frame() const | ||||
| { | ||||
|     return this == &page().focused_frame(); | ||||
|     return m_page && &m_page->focused_frame() == this; | ||||
| } | ||||
| 
 | ||||
| void Frame::set_document(DOM::Document* document) | ||||
|  | @ -89,10 +89,12 @@ void Frame::set_document(DOM::Document* document) | |||
| 
 | ||||
|     if (m_document) { | ||||
|         m_document->attach_to_frame({}, *this); | ||||
|         page().client().page_did_change_title(m_document->title()); | ||||
|         if (m_page) | ||||
|             m_page->client().page_did_change_title(m_document->title()); | ||||
|     } | ||||
| 
 | ||||
|     page().client().page_did_set_document_in_main_frame(m_document); | ||||
|     if (m_page) | ||||
|         m_page->client().page_did_set_document_in_main_frame(m_document); | ||||
| } | ||||
| 
 | ||||
| void Frame::set_size(const Gfx::IntSize& size) | ||||
|  | @ -120,7 +122,8 @@ void Frame::set_needs_display(const Gfx::IntRect& rect) | |||
|         return; | ||||
| 
 | ||||
|     if (is_main_frame()) { | ||||
|         page().client().page_did_invalidate(to_main_frame_rect(rect)); | ||||
|         if (m_page) | ||||
|             m_page->client().page_did_invalidate(to_main_frame_rect(rect)); | ||||
|         return; | ||||
|     } | ||||
| 
 | ||||
|  | @ -168,7 +171,8 @@ void Frame::scroll_to_anchor(const String& fragment) | |||
|         float_rect.move_by(-padding_box.left, -padding_box.top); | ||||
|     } | ||||
| 
 | ||||
|     page().client().page_did_request_scroll_into_view(enclosing_int_rect(float_rect)); | ||||
|     if (m_page) | ||||
|         m_page->client().page_did_request_scroll_into_view(enclosing_int_rect(float_rect)); | ||||
| } | ||||
| 
 | ||||
| Gfx::IntRect Frame::to_main_frame_rect(const Gfx::IntRect& a_rect) | ||||
|  |  | |||
|  | @ -55,8 +55,8 @@ public: | |||
| 
 | ||||
|     void set_document(DOM::Document*); | ||||
| 
 | ||||
|     Page& page() { return m_page; } | ||||
|     const Page& page() const { return m_page; } | ||||
|     Page* page() { return m_page; } | ||||
|     const Page* page() const { return m_page; } | ||||
| 
 | ||||
|     const Gfx::IntSize& size() const { return m_size; } | ||||
|     void set_size(const Gfx::IntSize&); | ||||
|  | @ -100,7 +100,7 @@ private: | |||
| 
 | ||||
|     void setup(); | ||||
| 
 | ||||
|     Page& m_page; | ||||
|     WeakPtr<Page> m_page; | ||||
|     Frame& m_main_frame; | ||||
| 
 | ||||
|     FrameLoader m_loader; | ||||
|  |  | |||
|  | @ -40,7 +40,7 @@ namespace Web { | |||
| 
 | ||||
| class PageClient; | ||||
| 
 | ||||
| class Page { | ||||
| class Page : public Weakable<Page> { | ||||
|     AK_MAKE_NONCOPYABLE(Page); | ||||
|     AK_MAKE_NONMOVABLE(Page); | ||||
| 
 | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Andreas Kling
						Andreas Kling