[iOS] Fixes SplashScreenView crash when remove from view hierarchy

[zhongwuzw]

Fixes flutter/flutter#37818 , flutter/flutter#105423. There are some cases that we remove the splashScreenView from superview before add it to view hierarchy.

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
  writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[jmagman]

cc @cyanglaz

[cyanglaz]

LGTM % nits.

@gaaclarke Do you mind taking a secondary review?

[cyanglaz]

Suggested change

	UIView* nilView = nil;
	[flutterViewController setSplashScreenView:nilView];
	[flutterViewController setSplashScreenView:nil];

[zhongwuzw]

@cyanglaz Emm, setSplashScreenView:nil would build error because splashScreenView declare nonnull and we treat build warning as error. Actually we have the case that let user pass the nil to remove the splash screen view, see

engine/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm

Lines 630 to 637 in 257b05f

	- (void)setSplashScreenView:(UIView*)view {
	if (!view) {
	// Special case: user wants to remove the splash screen view.
	if (_splashScreenView) {
	[self removeSplashScreenView:nil];
	}
	return;
	}

.

[cyanglaz]

Isn't this wrong that we make the view paramter nonnull and still checking nullability in the implementation? Maybe we should update the method syntax to make the parameter nonnull.

[jmagman]

It looks like setSplashScreenView should allow nil #6834, probably an oversight from adding NS_ASSUME_NONNULL_BEGIN. Can you instead make the API nullable?

- @property(strong, nonatomic) UIView* splashScreenView;
+ @property(strong, nonatomic, nullable) UIView* splashScreenView;

and add a comment to the header like:

* Set to nil to remove the splash screen view.

[zhongwuzw]

@cyanglaz We support view nullable to remove the view from view hierarchy, but we use NS_ASSUME_NONNULL_BEGIN to declare view nonnull.

@jmagman We can make the API nullable, but it may lead to an API-breaking change for Swift users, if Swift users use the getter of splashScreenView, the return type changed from UIView to UIView?.
But I agree that we should make it nullable, should we need to make a new PR as a breaking change to add nullable keyword?

[cyanglaz]

If we don't make this API nullable, it means this feature is not supported on swift, right?
So I think we should introduce the swift breaking change and make the API nullable.

[zhongwuzw]

If we don't make this API nullable, it means this feature is not supported on swift, right? So I think we should introduce the swift breaking change and make the API nullable.

@cyanglaz I create another PR to introduce API breaking change. Please review it #34743.

[jmagman]

#34743 has merged, apologies for that taking so long. Can you rebase on your change and make this nil?

[zhongwuzw]

@jmagman Done. :)

[cyanglaz]

LGTM

[jmagman]

LGTM