Uploaded image for project: 'Apache Cordova'
  1. Apache Cordova
  2. CB-8863

Fix potential problems with blocks usage in core plugins

VotersWatch issueWatchersLinkUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • None
    • cordova-ios
    • None

    Description

      Audit the core plugins to check for proper blocks usage.

      Problems that I see can be summed up in this (fixed) code:
      https://github.com/apache/cordova-plugin-splashscreen/blob/fa60f01adcba2d21583de7972a0facc4da1eb75e/src/ios/CDVSplashScreen.m#L292-L313

      1. You need a weak reference to "self" in blocks to prevent retain cycle problems.
      Fix: https://github.com/apache/cordova-plugin-splashscreen/blob/fa60f01adcba2d21583de7972a0facc4da1eb75e/src/ios/CDVSplashScreen.m#L292

      2. Anything that calls UIKit methods, which includes plugins that write JavaScript back to the UIWebView (commandDelegate functions), must be called in the main thread.
      Fix: https://github.com/apache/cordova-plugin-splashscreen/blob/fa60f01adcba2d21583de7972a0facc4da1eb75e/src/ios/CDVSplashScreen.m#L308-L310 (UPDATE: we actually already handle this in evalJS)

      3. Avoid creating private variables like this in the first place – I would create a class extension which will effectively have "private" properties (nothing is truly private in objc). This is to avoid this situation in the block where you have to cast the weak self into a strong self (and check it's still around), just to access a private variable:
      https://github.com/apache/cordova-plugin-splashscreen/blob/fa60f01adcba2d21583de7972a0facc4da1eb75e/src/ios/CDVSplashScreen.m#L298-L304
      Of course since we are modifying UIKit items here, rule 2 above also applies.

      Attachments

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            purplecabbage Jesse MacFadyen
            shazron Shazron Abdullah
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment