From 3090505fd484c661a30a3526c8f5ccb53a48fba9 Mon Sep 17 00:00:00 2001
From: yvt <i@yvt.jp>
Date: Mon, 2 Aug 2021 00:27:50 +0900
Subject: [PATCH] refactor(script): `navigate_or_reload_child_browsing_context`
 should only handle cases involving navigation

The initial document creation does not involve navigation, and it would
cause a confusion if this was done by a function which has `navigation`
in its name. This commit renames `navigate_or_reload_child_browsing_
context` to `start_new_pipeline`, and introduces a new function which
has the original name and is dedicated to handle navigation.
---
 components/script/dom/htmliframeelement.rs | 34 +++++++++++++---------
 components/script/script_thread.rs         |  8 ++---
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/components/script/dom/htmliframeelement.rs b/components/script/dom/htmliframeelement.rs
index 6f5264999be..b6f9948874b 100644
--- a/components/script/dom/htmliframeelement.rs
+++ b/components/script/dom/htmliframeelement.rs
@@ -57,9 +57,9 @@ bitflags! {
 }
 
 #[derive(PartialEq)]
-pub enum NavigationType {
+enum PipelineType {
     InitialAboutBlank,
-    Regular,
+    Navigation,
 }
 
 #[derive(PartialEq)]
@@ -105,9 +105,17 @@ impl HTMLIFrameElement {
     }
 
     pub fn navigate_or_reload_child_browsing_context(
+        &self,
+        load_data: LoadData,
+        replace: HistoryEntryReplacement,
+    ) {
+        self.start_new_pipeline(load_data, PipelineType::Navigation, replace);
+    }
+
+    fn start_new_pipeline(
         &self,
         mut load_data: LoadData,
-        nav_type: NavigationType,
+        pipeline_type: PipelineType,
         replace: HistoryEntryReplacement,
     ) {
         let sandboxed = if self.is_sandboxed() {
@@ -117,12 +125,12 @@ impl HTMLIFrameElement {
         };
 
         let browsing_context_id = match self.browsing_context_id() {
-            None => return warn!("Navigating unattached iframe."),
+            None => return warn!("Attempted to start a new pipeline on an unattached iframe."),
             Some(id) => id,
         };
 
         let top_level_browsing_context_id = match self.top_level_browsing_context_id() {
-            None => return warn!("Navigating unattached iframe."),
+            None => return warn!("Attempted to start a new pipeline on an unattached iframe."),
             Some(id) => id,
         };
 
@@ -181,8 +189,8 @@ impl HTMLIFrameElement {
             device_pixel_ratio: window.device_pixel_ratio(),
         };
 
-        match nav_type {
-            NavigationType::InitialAboutBlank => {
+        match pipeline_type {
+            PipelineType::InitialAboutBlank => {
                 let (pipeline_sender, pipeline_receiver) = ipc::channel().unwrap();
 
                 self.about_blank_pipeline_id.set(Some(new_pipeline_id));
@@ -213,7 +221,7 @@ impl HTMLIFrameElement {
                 self.pipeline_id.set(Some(new_pipeline_id));
                 ScriptThread::process_attach_layout(new_layout_info, document.origin().clone());
             },
-            NavigationType::Regular => {
+            PipelineType::Navigation => {
                 let load_info = IFrameLoadInfoWithData {
                     info: load_info,
                     load_data: load_data,
@@ -251,7 +259,6 @@ impl HTMLIFrameElement {
             load_data.srcdoc = String::from(element.get_string_attribute(&local_name!("srcdoc")));
             self.navigate_or_reload_child_browsing_context(
                 load_data,
-                NavigationType::Regular,
                 HistoryEntryReplacement::Disabled,
             );
             return;
@@ -342,11 +349,12 @@ impl HTMLIFrameElement {
         } else {
             HistoryEntryReplacement::Disabled
         };
-        self.navigate_or_reload_child_browsing_context(load_data, NavigationType::Regular, replace);
+        self.navigate_or_reload_child_browsing_context(load_data, replace);
     }
 
     fn create_nested_browsing_context(&self) {
-        // Synchronously create a new context and navigate it to about:blank.
+        // Synchronously create a new browsing context, which will present
+        // `about:blank`. (This is not a navigation.)
         let url = ServoUrl::parse("about:blank").unwrap();
         let document = document_from_node(self);
         let window = window_from_node(self);
@@ -366,9 +374,9 @@ impl HTMLIFrameElement {
         self.top_level_browsing_context_id
             .set(Some(top_level_browsing_context_id));
         self.browsing_context_id.set(Some(browsing_context_id));
-        self.navigate_or_reload_child_browsing_context(
+        self.start_new_pipeline(
             load_data,
-            NavigationType::InitialAboutBlank,
+            PipelineType::InitialAboutBlank,
             HistoryEntryReplacement::Disabled,
         );
     }
diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs
index c1161674fc2..506cd5d6bf2 100644
--- a/components/script/script_thread.rs
+++ b/components/script/script_thread.rs
@@ -45,7 +45,7 @@ use crate::dom::element::Element;
 use crate::dom::event::{Event, EventBubbles, EventCancelable};
 use crate::dom::globalscope::GlobalScope;
 use crate::dom::htmlanchorelement::HTMLAnchorElement;
-use crate::dom::htmliframeelement::{HTMLIFrameElement, NavigationType};
+use crate::dom::htmliframeelement::HTMLIFrameElement;
 use crate::dom::identityhub::Identities;
 use crate::dom::mutationobserver::MutationObserver;
 use crate::dom::node::{window_from_node, Node, ShadowIncluding};
@@ -3705,11 +3705,7 @@ impl ScriptThread {
             .borrow()
             .find_iframe(parent_pipeline_id, browsing_context_id);
         if let Some(iframe) = iframe {
-            iframe.navigate_or_reload_child_browsing_context(
-                load_data,
-                NavigationType::Regular,
-                replace,
-            );
+            iframe.navigate_or_reload_child_browsing_context(load_data, replace);
         }
     }