LibWeb: Make "top-level browsing context" concept more spec-compliant

Any browsing context that doesn't have a parent browsing context is now considered a top-level browsing context. This matches the HTML spec. This means we no longer keep a pointer to the top-level context, since we can simply walk the parent chain until we find the topmost ancestor.
cubiclove · Sep 10, 2021 · 8fabaaa · 8fabaaa
1 parent 4e5becf
commit 8fabaaa
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 25 deletions.
diff --git a/Userland/Libraries/LibWeb/HTML/BrowsingContextContainer.cpp b/Userland/Libraries/LibWeb/HTML/BrowsingContextContainer.cpp
@@ -27,7 +27,8 @@ void BrowsingContextContainer::inserted()
  if (!is_connected())
  return;
  if (auto* frame = document().browsing_context()) {
- m_nested_browsing_context = BrowsingContext::create_nested(*this, frame->top_level_browsing_context());
+ VERIFY(frame->page());
+ m_nested_browsing_context = BrowsingContext::create_nested(*frame->page(), *this);
  m_nested_browsing_context->set_frame_nesting_levels(frame->frame_nesting_levels());
  m_nested_browsing_context->register_frame_nesting(document().url());
  }

diff --git a/Userland/Libraries/LibWeb/Page/BrowsingContext.cpp b/Userland/Libraries/LibWeb/Page/BrowsingContext.cpp
@@ -18,9 +18,8 @@
 
 namespace Web {
 
-BrowsingContext::BrowsingContext(Page& page, HTML::BrowsingContextContainer* container, BrowsingContext& top_level_browsing_context)
+BrowsingContext::BrowsingContext(Page& page, HTML::BrowsingContextContainer* container)
  : m_page(page)
- , m_top_level_browsing_context(top_level_browsing_context)
  , m_loader(*this)
  , m_event_handler({}, *this)
  , m_container(container)
@@ -35,16 +34,6 @@ BrowsingContext::BrowsingContext(Page& page, HTML::BrowsingContextContainer* con
  });
 }
 
-BrowsingContext::BrowsingContext(HTML::BrowsingContextContainer& container, BrowsingContext& top_level_browsing_context)
- : BrowsingContext(*top_level_browsing_context.page(), &container, top_level_browsing_context)
-{
-}
-
-BrowsingContext::BrowsingContext(Page& page)
- : BrowsingContext(page, nullptr, *this)
-{
-}
-
 BrowsingContext::~BrowsingContext()
 {
 }

diff --git a/Userland/Libraries/LibWeb/Page/BrowsingContext.h b/Userland/Libraries/LibWeb/Page/BrowsingContext.h
@@ -24,8 +24,8 @@ namespace Web {
 
 class BrowsingContext : public TreeNode<BrowsingContext> {
 public:
- static NonnullRefPtr<BrowsingContext> create_nested(HTML::BrowsingContextContainer& container, BrowsingContext& top_level_browsing_context) { return adopt_ref(*new BrowsingContext(container, top_level_browsing_context)); }
- static NonnullRefPtr<BrowsingContext> create(Page& page) { return adopt_ref(*new BrowsingContext(page)); }
+ static NonnullRefPtr<BrowsingContext> create_nested(Page& page, HTML::BrowsingContextContainer& container) { return adopt_ref(*new BrowsingContext(page, &container)); }
+ static NonnullRefPtr<BrowsingContext> create(Page& page) { return adopt_ref(*new BrowsingContext(page, nullptr)); }
  ~BrowsingContext();
 
  class ViewportClient {
@@ -36,7 +36,7 @@ class BrowsingContext : public TreeNode<BrowsingContext> {
  void register_viewport_client(ViewportClient&);
  void unregister_viewport_client(ViewportClient&);
 
- bool is_top_level() const { return this == &top_level_browsing_context(); }
+ bool is_top_level() const { return !container(); }
  bool is_focused_context() const;
 
  DOM::Document const* active_document() const { return m_active_document; }
@@ -64,8 +64,15 @@ class BrowsingContext : public TreeNode<BrowsingContext> {
 
  void scroll_to_anchor(String const&);
 
- BrowsingContext& top_level_browsing_context() { return *m_top_level_browsing_context; }
- BrowsingContext const& top_level_browsing_context() const { return *m_top_level_browsing_context; }
+ BrowsingContext& top_level_browsing_context()
+ {
+ BrowsingContext* context = this;
+ while (context->parent())
+ context = context->parent();
+ return *context;
+ }
+
+ BrowsingContext const& top_level_browsing_context() const { return const_cast<BrowsingContext*>(this)->top_level_browsing_context(); }
 
  HTML::BrowsingContextContainer* container() { return m_container; }
  HTML::BrowsingContextContainer const* container() const { return m_container; }
@@ -95,18 +102,12 @@ class BrowsingContext : public TreeNode<BrowsingContext> {
  DOM::Document const* container_document() const;
 
 private:
- explicit BrowsingContext(Page&, HTML::BrowsingContextContainer*, BrowsingContext& top_level_browsing_context);
- explicit BrowsingContext(HTML::BrowsingContextContainer&, BrowsingContext& top_level_browsing_context);
- explicit BrowsingContext(Page&);
+ explicit BrowsingContext(Page&, HTML::BrowsingContextContainer*);
 
  void reset_cursor_blink_cycle();
 
  WeakPtr<Page> m_page;
 
- // NOTE: We expect there to always be a top-level browsing context as long as we exist.
- // The use of WeakPtr is for safety in case we get something wrong.
- WeakPtr<BrowsingContext> m_top_level_browsing_context;
-
  FrameLoader m_loader;
  EventHandler m_event_handler;