layout: Remove TryFrom<Contents> for NonReplacedContents (#37815)

This implementation is quite confusing as it makes it harder to tell
that we are just looking for the case that `Contents` contains
`NonReplacedContents`.

Testing: This shouldn't have any functional change, so is covered by
existing tests.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>

components/layout/dom_traversal.rs

@@ -376,19 +376,6 @@ impl From<NonReplacedContents> for Contents {
     }
 }
 
-impl std::convert::TryFrom<Contents> for NonReplacedContents {
-    type Error = &'static str;
-
-    fn try_from(contents: Contents) -> Result<Self, Self::Error> {
-        match contents {
-            Contents::NonReplaced(non_replaced_contents) => Ok(non_replaced_contents),
-            Contents::Replaced(_) => {
-                Err("Tried to covnert a `Contents::Replaced` into `NonReplacedContent`")
-            },
-        }
-    }
-}
-
 impl NonReplacedContents {
     pub(crate) fn traverse<'dom>(
         self,

components/layout/flow/construct.rs

@@ -3,7 +3,6 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use std::borrow::Cow;
-use std::convert::TryFrom;
 
 use rayon::iter::{IntoParallelIterator, ParallelIterator};
 use servo_arc::Arc;
@@ -464,23 +463,27 @@ impl<'dom> BlockContainerBuilder<'dom, '_> {
         contents: Contents,
         box_slot: BoxSlot<'dom>,
     ) {
-        let (DisplayInside::Flow { is_list_item }, false) =
-            (display_inside, contents.is_replaced())
-        else {
-            // If this inline element is an atomic, handle it and return.
-            let context = self.context;
-            let propagated_data = self.propagated_data;
-            let atomic = self.ensure_inline_formatting_context_builder().push_atomic(
-                IndependentFormattingContext::construct(
-                    context,
-                    info,
-                    display_inside,
-                    contents,
-                    propagated_data,
-                ),
-            );
-            box_slot.set(LayoutBox::InlineLevel(vec![atomic]));
-            return;
+        let (is_list_item, non_replaced_contents) = match (display_inside, contents) {
+            (
+                DisplayInside::Flow { is_list_item },
+                Contents::NonReplaced(non_replaced_contents),
+            ) => (is_list_item, non_replaced_contents),
+            (_, contents) => {
+                // If this inline element is an atomic, handle it and return.
+                let context = self.context;
+                let propagated_data = self.propagated_data;
+                let atomic = self.ensure_inline_formatting_context_builder().push_atomic(
+                    IndependentFormattingContext::construct(
+                        context,
+                        info,
+                        display_inside,
+                        contents,
+                        propagated_data,
+                    ),
+                );
+                box_slot.set(LayoutBox::InlineLevel(vec![atomic]));
+                return;
+            },
         };
 
         // Otherwise, this is just a normal inline box. Whatever happened before, all we need to do
@@ -500,9 +503,7 @@ impl<'dom> BlockContainerBuilder<'dom, '_> {
         }
 
         // `unwrap` doesn’t panic here because `is_replaced` returned `false`.
-        NonReplacedContents::try_from(contents)
-            .unwrap()
-            .traverse(self.context, info, self);
+        non_replaced_contents.traverse(self.context, info, self);
 
         self.finish_anonymous_table_if_needed();
 

components/layout/table/construct.rs

@@ -3,7 +3,6 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use std::borrow::Cow;
-use std::convert::{TryFrom, TryInto};
 use std::iter::repeat;
 
 use atomic_refcell::AtomicRef;
@@ -769,11 +768,10 @@ impl<'dom> TraversalHandler<'dom> for TableBuilderTraversal<'_, 'dom> {
                     let new_row_group_index = self.builder.table.row_groups.len() - 1;
                     self.current_row_group_index = Some(new_row_group_index);
 
-                    NonReplacedContents::try_from(contents).unwrap().traverse(
-                        self.context,
-                        info,
-                        self,
-                    );
+                    let Contents::NonReplaced(non_replaced_contents) = contents else {
+                        unreachable!("Replaced should not have a LayoutInternal display type.");
+                    };
+                    non_replaced_contents.traverse(self.context, info, self);
                     self.finish_anonymous_row_if_needed();
 
                     self.current_row_group_index = None;
@@ -787,14 +785,13 @@ impl<'dom> TraversalHandler<'dom> for TableBuilderTraversal<'_, 'dom> {
                     self.finish_anonymous_row_if_needed();
 
                     let context = self.context;
-
                     let mut row_builder =
                         TableRowBuilder::new(self, info, self.current_propagated_data);
-                    NonReplacedContents::try_from(contents).unwrap().traverse(
-                        context,
-                        info,
-                        &mut row_builder,
-                    );
+
+                    let Contents::NonReplaced(non_replaced_contents) = contents else {
+                        unreachable!("Replaced should not have a LayoutInternal display type.");
+                    };
+                    non_replaced_contents.traverse(context, info, &mut row_builder);
                     row_builder.finish();
 
                     let row = ArcRefCell::new(TableTrack {
@@ -822,11 +819,10 @@ impl<'dom> TraversalHandler<'dom> for TableBuilderTraversal<'_, 'dom> {
                         columns: Vec::new(),
                     };
 
-                    NonReplacedContents::try_from(contents).unwrap().traverse(
-                        self.context,
-                        info,
-                        &mut column_group_builder,
-                    );
+                    let Contents::NonReplaced(non_replaced_contents) = contents else {
+                        unreachable!("Replaced should not have a LayoutInternal display type.");
+                    };
+                    non_replaced_contents.traverse(self.context, info, &mut column_group_builder);
 
                     let first_column = self.builder.table.columns.len();
                     if column_group_builder.columns.is_empty() {
@@ -855,20 +851,17 @@ impl<'dom> TraversalHandler<'dom> for TableBuilderTraversal<'_, 'dom> {
                     )));
                 },
                 DisplayLayoutInternal::TableCaption => {
-                    let contents = match contents.try_into() {
-                        Ok(non_replaced_contents) => {
-                            IndependentNonReplacedContents::Flow(BlockFormattingContext::construct(
-                                self.context,
-                                info,
-                                non_replaced_contents,
-                                self.current_propagated_data,
-                                false, /* is_list_item */
-                            ))
-                        },
-                        Err(_replaced) => {
-                            unreachable!("Replaced should not have a LayoutInternal display type.");
-                        },
+                    let Contents::NonReplaced(non_replaced_contents) = contents else {
+                        unreachable!("Replaced should not have a LayoutInternal display type.");
                     };
+                    let contents =
+                        IndependentNonReplacedContents::Flow(BlockFormattingContext::construct(
+                            self.context,
+                            info,
+                            non_replaced_contents,
+                            self.current_propagated_data,
+                            false, /* is_list_item */
+                        ));
 
                     let caption = ArcRefCell::new(TableCaption {
                         context: IndependentFormattingContext {
@@ -1016,21 +1009,18 @@ impl<'dom> TraversalHandler<'dom> for TableRowBuilder<'_, '_, 'dom, '_> {
 
                     let propagated_data =
                         self.propagated_data.disallowing_percentage_table_columns();
-                    let contents = match contents.try_into() {
-                        Ok(non_replaced_contents) => {
-                            BlockFormattingContext::construct(
-                                self.table_traversal.context,
-                                info,
-                                non_replaced_contents,
-                                propagated_data,
-                                false, /* is_list_item */
-                            )
-                        },
-                        Err(_replaced) => {
-                            unreachable!("Replaced should not have a LayoutInternal display type.");
-                        },
+                    let Contents::NonReplaced(non_replaced_contents) = contents else {
+                        unreachable!("Replaced should not have a LayoutInternal display type.");
                     };
 
+                    let contents = BlockFormattingContext::construct(
+                        self.table_traversal.context,
+                        info,
+                        non_replaced_contents,
+                        propagated_data,
+                        false, /* is_list_item */
+                    );
+
                     self.finish_current_anonymous_cell_if_needed();
 
                     let cell = ArcRefCell::new(TableSlotCell {