Avoid using a Rc<RefCell<_>> in `HtmlRewriteController::handlers_di…

…spatcher()`.
cloudflare · Aug 16, 2024 · 9a62d7b · 9a62d7b
1 parent 53469c5
commit 9a62d7b
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 31 deletions.
diff --git a/src/rewriter/rewrite_controller.rs b/src/rewriter/rewrite_controller.rs
@@ -5,8 +5,6 @@ use crate::rewritable_units::{DocumentEnd, Token, TokenCaptureFlags};
 use crate::selectors_vm::{AuxStartTagInfoRequest, ElementData, SelectorMatchingVm, VmError};
 use crate::transform_stream::*;
 use hashbrown::HashSet;
-use std::cell::RefCell;
-use std::rc::Rc;
 
 #[derive(Default)]
 pub struct ElementDescriptor {
@@ -25,7 +23,7 @@ impl ElementData for ElementDescriptor {
 }
 
 pub struct HtmlRewriteController<'h> {
-    handlers_dispatcher: Rc<RefCell<ContentHandlersDispatcher<'h>>>,
+    handlers_dispatcher: ContentHandlersDispatcher<'h>,
     selector_matching_vm: Option<SelectorMatchingVm<ElementDescriptor>>,
 }
 
@@ -36,32 +34,22 @@ impl<'h> HtmlRewriteController<'h> {
         selector_matching_vm: Option<SelectorMatchingVm<ElementDescriptor>>,
     ) -> Self {
         HtmlRewriteController {
-            handlers_dispatcher: Rc::new(RefCell::new(handlers_dispatcher)),
+            handlers_dispatcher,
             selector_matching_vm,
         }
     }
 }
 
-// NOTE: it's a macro instead of an instance method, so it can be executed
-// when we hold a mutable reference for the selector matching VM.
-macro_rules! create_match_handler {
-    ($self:tt) => {{
-        let handlers_dispatcher = Rc::clone(&$self.handlers_dispatcher);
-
-        move |m| handlers_dispatcher.borrow_mut().start_matching(m)
-    }};
-}
-
 impl<'h> HtmlRewriteController<'h> {
     #[inline]
     fn respond_to_aux_info_request(
         aux_info_req: AuxStartTagInfoRequest<ElementDescriptor, SelectorHandlersLocator>,
     ) -> StartTagHandlingResult<Self> {
         Err(DispatcherError::InfoRequest(Box::new(
             move |this, aux_info| {
-                let mut match_handler = create_match_handler!(this);
-
                 if let Some(ref mut vm) = this.selector_matching_vm {
+                    let mut match_handler = |m| this.handlers_dispatcher.start_matching(m);
+
                     aux_info_req(vm, aux_info, &mut match_handler)
                         .map_err(RewritingError::MemoryLimitExceeded)?;
                 }
@@ -73,7 +61,7 @@ impl<'h> HtmlRewriteController<'h> {
 
     #[inline]
     fn get_capture_flags(&self) -> TokenCaptureFlags {
-        self.handlers_dispatcher.borrow().get_token_capture_flags()
+        self.handlers_dispatcher.get_token_capture_flags()
     }
 }
 
@@ -90,7 +78,7 @@ impl TransformController for HtmlRewriteController<'_> {
     ) -> StartTagHandlingResult<Self> {
         match self.selector_matching_vm {
             Some(ref mut vm) => {
-                let mut match_handler = create_match_handler!(self);
+                let mut match_handler = |m| self.handlers_dispatcher.start_matching(m);
 
                 match vm.exec_for_start_tag(local_name, ns, &mut match_handler) {
                     Ok(_) => Ok(self.get_capture_flags()),
@@ -108,10 +96,8 @@ impl TransformController for HtmlRewriteController<'_> {
 
     fn handle_end_tag(&mut self, local_name: LocalName) -> TokenCaptureFlags {
         if let Some(ref mut vm) = self.selector_matching_vm {
-            let handlers_dispatcher = Rc::clone(&self.handlers_dispatcher);
-
-            vm.exec_for_end_tag(local_name, move |elem_desc| {
-                handlers_dispatcher.borrow_mut().stop_matching(elem_desc);
+            vm.exec_for_end_tag(local_name, |elem_desc| {
+                self.handlers_dispatcher.stop_matching(elem_desc);
             });
         }
 
@@ -126,14 +112,12 @@ impl TransformController for HtmlRewriteController<'_> {
             .and_then(SelectorMatchingVm::current_element_data_mut);
 
         self.handlers_dispatcher
-            .borrow_mut()
             .handle_token(token, current_element_data)
             .map_err(RewritingError::ContentHandlerError)
     }
 
     fn handle_end(&mut self, document_end: &mut DocumentEnd) -> Result<(), RewritingError> {
         self.handlers_dispatcher
-            .borrow_mut()
             .handle_end(document_end)
             .map_err(RewritingError::ContentHandlerError)
     }
@@ -142,7 +126,6 @@ impl TransformController for HtmlRewriteController<'_> {
     fn should_emit_content(&self) -> bool {
         !self
             .handlers_dispatcher
-            .borrow()
             .has_matched_elements_with_removed_content()
     }
 }
diff --git a/src/rewriter/settings.rs b/src/rewriter/settings.rs
@@ -418,7 +418,7 @@ impl Default for MemorySettings {
     fn default() -> Self {
         MemorySettings {
             preallocated_parsing_buffer_size: 1024,
-            max_allowed_memory_usage: std::usize::MAX,
+            max_allowed_memory_usage: usize::MAX,
         }
     }
 }

diff --git a/tests/harness/suites/html5lib_tests/decoder.rs b/tests/harness/suites/html5lib_tests/decoder.rs
@@ -92,11 +92,8 @@ impl<'a> Decoder<'a> {
                 if m.0 != 0 {
                     if c != ';' && self.entities == Entities::Attribute {
                         if let Some(&c) = self.chars.peek() {
-                            match c {
-                                'A'..='Z' | 'a'..='z' | '0'..='9' | '=' => {
-                                    continue;
-                                }
-                                _ => {}
+                            if matches!(c, 'A'..='Z' | 'a'..='z' | '0'..='9' | '=') {
+                                continue;
                             }
                         }
                     }