jamonholmgren / ProMotion

ProMotion is a RubyMotion gem that makes iPhone development less like Objective-C and more like Ruby.
MIT License
1.26k stars 147 forks source link

modal WebScreen with NavBar leaks #493

Closed forrestgrant closed 10 years ago

forrestgrant commented 10 years ago

WebScreen (Same code for both scenarios)

This uses SugarCube::Timer for demo simplicity, but has been tested with buttons.

class WebScreen < PM::WebScreen
  def on_load
    1.second.later { close }
  end
end

Modal without nav bar

class FirstScreen < PM::Screen
  def show_web_screen
    open_modal WebScreen.new
  end
end

pm webscreen without nav

Modal with a nav bar

class FirstScreen < PM::Screen
  def show_web_screen
    open_modal WebScreen.new(nav_bar: true)
  end
end

pm webscreen with nav

markrickert commented 10 years ago

What version of PM are you using? Any way you can throw up an example app for us into a repos so we can test this out for ourselves?

Thanks for reporting!

forrestgrant commented 10 years ago

Sure thing, thanks for looking into it, @markrickert! https://github.com/forrestgrant/pm_web_leak

The app referenced in my original comment is using 1.1.1 The sample app repo is using 1.2.1

I tried the most recent from github (2.0.0.rc3) but kept receving the following error upon opening a WebScreen:

Objective-C stub for message `setFrame:' type `v@:{CGRect={CGPoint=ff}{CGSize=ff}}' not precompiled. Make sure you properly link with the framework or library that defines this message.

Perhaps this should be another issue

jamonholmgren commented 10 years ago

For the stub message, try running rake clean --all before filing the issue just in case.

markrickert commented 10 years ago

rake clean:all ?

jamonholmgren commented 10 years ago

^^ what he said.

forrestgrant commented 10 years ago

Same result. With: ProMotion 2.0.0.rc3 RubyMotion 2.3.0

We can ignore this for now, and focus on the WebView leak.

jamonholmgren commented 10 years ago

Probably related for stub issue: https://github.com/motion-kit/motion-kit/issues/50

Watson1978 commented 10 years ago

@jamonholmgren Maybe, I think you might require to release UINavigationController when close modal view. (I tried with ProMotion v1.2.1 code)

diff --git a/lib/ProMotion/screen/screen_navigation.rb b/lib/ProMotion/screen/screen_navigation.rb
index 3fb6ea4..59c27f0 100644
--- a/lib/ProMotion/screen/screen_navigation.rb
+++ b/lib/ProMotion/screen/screen_navigation.rb
@@ -53,6 +53,7 @@ def close_screen(args = {})
       args[:animated] = true unless args.has_key?(:animated)

       if self.modal?
+        close_nav_screen args if self.navigationController
         close_modal_screen args

       elsif self.navigationController
@@ -155,6 +156,7 @@ def close_nav_screen(args={})
       else
         self.navigationController.popViewControllerAnimated(args[:animated])
       end
+      self.navigationController = nil
     end

   end

Maybe my code might not be correct :p

jamonholmgren commented 10 years ago

:+1: Nice!

jamonholmgren commented 10 years ago

@forrestgrant Would you be willing to apply @Watson1978 's patch and test it and submit a PR if it fixes the memory leak?

forrestgrant commented 10 years ago

Done! #494

I'm still getting the stub error (in 2.0.0), but I'll create another issue for that.

jamonholmgren commented 10 years ago

Closed with https://github.com/clearsightstudio/ProMotion/releases/tag/v2.0.0.rc4. Pushed to RubyGems too.

markrickert commented 10 years ago

:thumbsup:

forrestgrant commented 10 years ago

:beers: