avm1: Run entire stack frames at once

core/src/avm1.rs

@@ -6,7 +6,6 @@ use crate::context::UpdateContext;
 use crate::prelude::*;
 use gc_arena::{GcCell, MutationContext};
 use std::collections::HashMap;
-use std::convert::TryInto;
 use url::form_urlencoded;
 
 use swf::avm1::read::Reader;
@@ -441,64 +440,6 @@ impl<'gc> Avm1<'gc> {
         }
     }
 
-    /// Perform some action with the current stack frame's reader.
-    ///
-    /// This function constructs a reader based off the current stack frame's
-    /// reader. You are permitted to mutate the stack frame as you wish. If the
-    /// stack frame we started with still exists in the same location on the
-    /// stack, it's PC will be updated to the Reader's current PC.
-    ///
-    /// Stack frame identity (for the purpose of the above paragraph) is
-    /// determined by the data pointed to by the `SwfSlice` of a given frame.
-    ///
-    /// # Warnings
-    ///
-    /// It is incorrect to call this function multiple times in the same stack.
-    /// Doing so will result in any changes in duplicate readers being ignored.
-    /// Always pass the borrowed reader into functions that need it.
-    fn with_current_reader_mut<F, R>(
-        &mut self,
-        context: &mut UpdateContext<'_, 'gc, '_>,
-        func: F,
-    ) -> Result<R, Error<'gc>>
-    where
-        F: FnOnce(
-            &mut Self,
-            &mut Reader<'_>,
-            &mut UpdateContext<'_, 'gc, '_>,
-        ) -> Result<R, Error<'gc>>,
-    {
-        let (frame_cell, swf_version, data, pc) = {
-            let frame = self.stack_frames.last().ok_or(Error::NoStackFrame)?;
-            let mut frame_ref = frame.write(context.gc_context);
-            frame_ref.lock()?;
-
-            (
-                *frame,
-                frame_ref.swf_version(),
-                frame_ref.data(),
-                frame_ref.pc(),
-            )
-        };
-
-        let mut read = Reader::new(data.as_ref(), swf_version);
-        read.seek(pc.try_into().unwrap());
-
-        let r = func(self, &mut read, context);
-
-        let mut frame_ref = frame_cell.write(context.gc_context);
-        frame_ref.unlock_execution();
-        frame_ref.set_pc(read.pos());
-
-        if let Err(error) = &r {
-            if error.is_halting() {
-                self.halt();
-            }
-        }
-
-        r
-    }
-
     /// Destroy the current stack frame (if there is one).
     ///
     /// The given return value will be pushed on the stack if there is a
@@ -537,14 +478,8 @@ impl<'gc> Avm1<'gc> {
             return Ok(());
         }
         while !self.stack_frames.is_empty() {
-            if let Err(e) = self.with_current_reader_mut(context, |this, r, context| {
+            let activation = self.current_stack_frame().ok_or(Error::FrameNotOnStack)?;
-                if !this.halted {
+            if let Err(e) = StackFrame::new(self, activation).run(context) {
-                    let activation = this.current_stack_frame().unwrap();
-                    StackFrame::new(this, activation).do_next_action(context, r)
-                } else {
-                    Ok(())
-                }
-            }) {
                 if let Error::ThrownValue(error) = &e {
                     let string = error
                         .coerce_to_string(self, context)
@@ -591,14 +526,8 @@ impl<'gc> Avm1<'gc> {
                 .map(|fr| GcCell::ptr_eq(stop_frame, *fr))
                 .unwrap_or(false)
             {
-                self.with_current_reader_mut(context, |this, r, context| {
+                let activation = self.current_stack_frame().ok_or(Error::FrameNotOnStack)?;
-                    if !this.halted {
+                StackFrame::new(self, activation).run(context)?
-                        let activation = this.current_stack_frame().unwrap();
-                        StackFrame::new(this, activation).do_next_action(context, r)
-                    } else {
-                        Ok(())
-                    }
-                })?;
             }
 
             Ok(())

core/src/avm1/stack_frame.rs

@@ -15,6 +15,7 @@ use enumset::EnumSet;
 use gc_arena::{Collect, GcCell};
 use rand::Rng;
 use std::borrow::Cow;
+use std::convert::TryInto;
 use swf::avm1::read::Reader;
 use swf::avm1::types::{Action, Function};
 
@@ -25,6 +26,11 @@ macro_rules! avm_debug {
     )
 }
 
+enum FrameControl {
+    Continue,
+    Return,
+}
+
 #[derive(Collect)]
 #[collect(no_drop)]
 pub struct StackFrame<'a, 'gc: 'a> {
@@ -37,17 +43,49 @@ impl<'a, 'gc: 'a> StackFrame<'a, 'gc> {
         Self { avm, activation }
     }
 
+    pub fn run(&mut self, context: &mut UpdateContext<'_, 'gc, '_>) -> Result<(), Error<'gc>> {
+        let mut activation = self.activation.write(context.gc_context);
+        activation.lock()?;
+        let data = activation.data();
+        let mut read = Reader::new(data.as_ref(), activation.swf_version());
+        read.seek(activation.pc().try_into().unwrap());
+        drop(activation);
+
+        let result = loop {
+            let result = self.do_action(context, &mut read);
+            match result {
+                Ok(FrameControl::Return) => break Ok(()),
+                Ok(FrameControl::Continue) => {}
+                Err(e) => break Err(e),
+            }
+        };
+
+        let mut activation = self.activation.write(context.gc_context);
+        activation.unlock_execution();
+        activation.set_pc(read.pos());
+        drop(activation);
+
+        if let Err(error) = &result {
+            if error.is_halting() {
+                self.avm.halt();
+            }
+        }
+
+        result
+    }
+
     /// Run a single action from a given action reader.
-    pub fn do_next_action(
+    fn do_action(
         &mut self,
         context: &mut UpdateContext<'_, 'gc, '_>,
         reader: &mut Reader<'_>,
-    ) -> Result<(), Error<'gc>> {
+    ) -> Result<FrameControl, Error<'gc>> {
         let data = self.activation.read().data();
 
         if reader.pos() >= (data.end - data.start) {
             //Executing beyond the end of a function constitutes an implicit return.
             self.avm.retire_stack_frame(context, Value::Undefined);
+            return Ok(FrameControl::Return);
         } else if let Some(action) = reader.read_action()? {
             avm_debug!("Action: {:?}", action);
 
@@ -135,7 +173,10 @@ impl<'a, 'gc: 'a> StackFrame<'a, 'gc> {
                 Action::PushDuplicate => self.action_push_duplicate(context),
                 Action::RandomNumber => self.action_random_number(context),
                 Action::RemoveSprite => self.action_remove_sprite(context),
-                Action::Return => self.action_return(context),
+                Action::Return => {
+                    self.action_return(context)?;
+                    return Ok(FrameControl::Return);
+                }
                 Action::SetMember => self.action_set_member(context),
                 Action::SetProperty => self.action_set_property(context),
                 Action::SetTarget(target) => self.action_set_target(context, &target),
@@ -185,9 +226,10 @@ impl<'a, 'gc: 'a> StackFrame<'a, 'gc> {
         } else {
             //The explicit end opcode was encountered so return here
             self.avm.retire_stack_frame(context, Value::Undefined);
+            return Ok(FrameControl::Return);
         }
 
-        Ok(())
+        Ok(FrameControl::Continue)
     }
 
     fn unknown_op(
@@ -372,15 +414,15 @@ impl<'a, 'gc: 'a> StackFrame<'a, 'gc> {
             };
 
             if let Some(frame) = frame {
-                // We must run the actions in the order that the tags appear,
+                for action in clip.actions_on_frame(context, frame) {
-                // so we want to push the stack frames in reverse order.
-                for action in clip.actions_on_frame(context, frame).rev() {
                     self.avm.insert_stack_frame_for_action(
                         self.avm.target_clip_or_root(),
                         self.avm.current_swf_version(),
                         action,
                         context,
                     );
+                    let frame = self.avm.current_stack_frame().unwrap();
+                    self.avm.run_current_frame(context, frame)?;
                 }
             } else {
                 log::warn!("Call: Invalid frame {:?}", frame);
@@ -1975,10 +2017,12 @@ impl<'a, 'gc: 'a> StackFrame<'a, 'gc> {
             object,
             context.gc_context,
         );
-        let new_activation = self.activation.read().to_rescope(block, with_scope);
+        let new_activation = GcCell::allocate(
-        self.avm
+            context.gc_context,
-            .stack_frames
+            self.activation.read().to_rescope(block, with_scope),
-            .push(GcCell::allocate(context.gc_context, new_activation));
+        );
+        self.avm.stack_frames.push(new_activation);
+        self.avm.run_current_frame(context, new_activation)?;
         Ok(())
     }
 }