Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
05ad20c
Forwards iOS dark mode trait to the Flutter framework (#34441).
matthew-carroll Jul 9, 2019
1bf2e6a
Formatting update.
matthew-carroll Jul 9, 2019
6126ae2
Removed unnecessary calls to onUserSettingsChanged for Dark Mode.
matthew-carroll Jul 9, 2019
27af2f8
Formatting update.
matthew-carroll Jul 9, 2019
74148fc
Added tests.
matthew-carroll Aug 8, 2019
e0b9656
Format update.
matthew-carroll Aug 8, 2019
7bd9ac8
Format update.
matthew-carroll Aug 8, 2019
201f691
Add #ifdef to avoid CI falures due to iOS 13.
matthew-carroll Aug 9, 2019
e1f0a20
Changed #elif to #else
matthew-carroll Aug 9, 2019
55af42e
Switch to local VC traitCollection instead of singleton.
matthew-carroll Aug 9, 2019
b709f95
Changed test simulator back to OS 12.2
matthew-carroll Aug 9, 2019
ed60b1c
Removed all new user settings messages except when trait collection c…
matthew-carroll Aug 9, 2019
8fdb300
Added user settings call to viewDidLoad in VC.
matthew-carroll Aug 9, 2019
ee5e8b8
Enabled ARC for test.
matthew-carroll Aug 9, 2019
5c83334
Formatting.
matthew-carroll Aug 9, 2019
5ac2ada
Formatting.
matthew-carroll Aug 9, 2019
435a134
Fixed test mocking mistake, added platform contrast to this PR.
matthew-carroll Aug 13, 2019
13cfeaf
Fixed formatting, returned early from contrast tests if iOS 13 is not…
matthew-carroll Aug 13, 2019
e53c791
Formatting.
matthew-carroll Aug 13, 2019
c0de8e5
Surrounded accessibility contrast with conditional preprocessors and …
matthew-carroll Aug 14, 2019
c76d20a
Adjusted to be compatible with API 12.2
matthew-carroll Aug 14, 2019
557fb06
Formatting
matthew-carroll Aug 15, 2019
7bfdd78
PR Updates.
matthew-carroll Aug 15, 2019
a4053eb
Formatting
matthew-carroll Aug 15, 2019
1921433
Formatting
matthew-carroll Aug 15, 2019
0e2a1b5
Reverted run_tests.h device type.
matthew-carroll Aug 15, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,20 @@ @interface FlutterViewController () <FlutterBinaryMessenger>
@property(nonatomic, readwrite, getter=isDisplayingFlutterUI) BOOL displayingFlutterUI;
@end

// The following conditional compilation defines an API 13 concept on earlier API targets so that
// a compiler compiling against API 12 or below does not blow up due to non-existent members.
#if __IPHONE_OS_VERSION_MAX_ALLOWED < 130000
typedef enum UIAccessibilityContrast : NSInteger {
UIAccessibilityContrastUnspecified = 0,
UIAccessibilityContrastNormal = 1,
UIAccessibilityContrastHigh = 2
} UIAccessibilityContrast;

@interface UITraitCollection (MethodsFromNewerSDK)
- (UIAccessibilityContrast)accessibilityContrast;
@end
#endif

@implementation FlutterViewController {
std::unique_ptr<fml::WeakPtrFactory<FlutterViewController>> _weakFactory;
fml::scoped_nsobject<FlutterEngine> _engine;
Expand Down Expand Up @@ -434,6 +448,9 @@ - (void)viewWillAppear:(BOOL)animated {
_engineNeedsLaunch = NO;
}

// Send platform settings to Flutter, e.g., platform brightness.
[self onUserSettingsChanged:nil];

// Only recreate surface on subsequent appearances when viewport metrics are known.
// First time surface creation is done on viewDidLayoutSubviews.
if (_viewportMetrics.physical_width) {
Expand Down Expand Up @@ -885,10 +902,17 @@ - (void)onLocaleUpdated:(NSNotification*)notification {

#pragma mark - Set user settings

- (void)traitCollectionDidChange:(UITraitCollection*)previousTraitCollection {
[super traitCollectionDidChange:previousTraitCollection];
[self onUserSettingsChanged:nil];
}

- (void)onUserSettingsChanged:(NSNotification*)notification {
[[_engine.get() settingsChannel] sendMessage:@{
@"textScaleFactor" : @([self textScaleFactor]),
@"alwaysUse24HourFormat" : @([self isAlwaysUse24HourFormat]),
@"platformBrightness" : [self brightnessMode],
@"platformContrast" : [self contrastMode]
}];
}

Expand Down Expand Up @@ -962,6 +986,40 @@ - (BOOL)isAlwaysUse24HourFormat {
return [dateFormat rangeOfString:@"a"].location == NSNotFound;
}

// The brightness mode of the platform, e.g., light or dark, expressed as a string that
// is understood by the Flutter framework. See the settings system channel for more
// information.
- (NSString*)brightnessMode {
if (@available(iOS 13, *)) {
UIUserInterfaceStyle style = self.traitCollection.userInterfaceStyle;

if (style == UIUserInterfaceStyleDark) {
return @"dark";
} else {
return @"light";
}
} else {
return @"light";
}
}

// The contrast mode of the platform, e.g., normal or high, expressed as a string that is
// understood by the Flutter framework. See the settings system channel for more
// information.
- (NSString*)contrastMode {
if (@available(iOS 13, *)) {
UIAccessibilityContrast contrast = self.traitCollection.accessibilityContrast;

if (contrast == UIAccessibilityContrastHigh) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constant only exists in the iOS 13 SDK, unlike UIUserInterfaceStyleDark which is in 12. This isn't going to compile on the bots.

If you want to add this we need to go back to the early discussion of declaring the constant yourself, conditionally, behind an ifdef based on the compiling SDK, so that you can use the constant regardless of which SDK you're compiling against. Same with adding a declaration of accessibilityContrast, which is also 13+

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example of defining such a constant that I can reference and replicate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a stab at only including the accessibility logic when the max API is 13 or above. Please let me know if that's sufficient.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As previously discussed, putting the actual implementation behind ifdefs doesn't do what you want, because that would mean it would only work if someone compiles their own engine locally with the Xcode beta; none of this code would actually be shipped in our library.

If this code is important to have now, then per the previous discussion what you need are declarations of the constant and the method that are behind reversed SDK ifdefs (i.e., the local declarations are used when compiling with a stable SDK, and not there to cause duplicate definition errors when using the beta SDK), allowing all of the logic here to compile regardless of SDK.

I don't have an example offhand, and would have to dig one up tomorrow. But the declarations that you need behind the ifdefs are exactly the ones that are in the beta SDK: the same name and value for the constant, and the identical signature for the method (declared via a class extension).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An example to replicate would be great. I wasn't clear on the distinction last time and I haven't been able to locate an example of it in practice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return @"high";
} else {
return @"normal";
}
} else {
return @"normal";
}
}

#pragma mark - Status Bar touch event handling

// Standard iOS status bar height in pixels.
Expand Down
221 changes: 221 additions & 0 deletions shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,29 @@
#import <XCTest/XCTest.h>
#import "flutter/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h"

#include "FlutterBinaryMessenger.h"

#if !__has_feature(objc_arc)
#error ARC must be enabled!
#endif

@interface FlutterViewControllerTest : XCTestCase
@end

// The following conditional compilation defines an API 13 concept on earlier API targets so that
// a compiler compiling against API 12 or below does not blow up due to non-existent members.
#if __IPHONE_OS_VERSION_MAX_ALLOWED < 130000
typedef enum UIAccessibilityContrast : NSInteger {
UIAccessibilityContrastUnspecified = 0,
UIAccessibilityContrastNormal = 1,
UIAccessibilityContrastHigh = 2
} UIAccessibilityContrast;

@interface UITraitCollection (MethodsFromNewerSDK)
- (UIAccessibilityContrast)accessibilityContrast;
@end
#endif

@implementation FlutterViewControllerTest

- (void)testBinaryMessenger {
Expand All @@ -23,4 +43,205 @@ - (void)testBinaryMessenger {
OCMVerify([engine binaryMessenger]);
}

#pragma mark - Platform Brightness

- (void)testItReportsLightPlatformBrightnessByDefault {
// Setup test.
id engine = OCMClassMock([FlutterEngine class]);

id settingsChannel = OCMClassMock([FlutterBasicMessageChannel class]);
OCMStub([engine settingsChannel]).andReturn(settingsChannel);

FlutterViewController* vc = [[FlutterViewController alloc] initWithEngine:engine
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these tests running with ARC? Because these look like leaks. The rest of the engine does not use ARC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea. I was just continuing with what I already found in this file. If you have any specific recommendations for changes, I can make them, but I'm working from a lack of iOS knowledge here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chinmaygarde @stuartmorgan do either of you have any specific instructions for what should be done here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dnfield @gspencergoog do either of you know what changes should be made here to avoid leaks? I don't know what changes to make to bring this test code into compliance...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would turn on ARC for the test file, since that would be easy (it looks like the test project is doing it file-by-file, which seems like an odd approach; I would just enable it for the whole project).

Failing that, add an autorelease here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LongCatIsLooong helped me get ARC enabled

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to have ARC off for the project so that we can explicitly test deallocating things...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I only enabled it for this file. Do you want me to change something?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that's fine - my reply was more directed towards @stuartmorgan's comment :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well since the only test in that file is leaking the view controller, I'm pretty sure it's not deliberately off to test dealloc...

nibName:nil
bundle:nil];

// Exercise behavior under test.
[vc traitCollectionDidChange:nil];

// Verify behavior.
OCMVerify([settingsChannel sendMessage:[OCMArg checkWithBlock:^BOOL(id message) {
return [message[@"platformBrightness"] isEqualToString:@"light"];
}]]);

// Clean up mocks
[engine stopMocking];
[settingsChannel stopMocking];
}

- (void)testItReportsPlatformBrightnessWhenViewWillAppear {
// Setup test.
id engine = OCMClassMock([FlutterEngine class]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you verified that engine won't outlive this test? Per offline discussion, if a class mock outlives the test, Bad Things happen. I think the recent refactors to method channel handling means that it won't leak the engine like it used to, but I'd like to be sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How might I verify this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just went ahead and explicitly cleaned up every mock in each test.


id settingsChannel = OCMClassMock([FlutterBasicMessageChannel class]);
OCMStub([engine settingsChannel]).andReturn(settingsChannel);

FlutterViewController* vc = [[FlutterViewController alloc] initWithEngine:engine
nibName:nil
bundle:nil];

// Exercise behavior under test.
[vc viewWillAppear:false];

// Verify behavior.
OCMVerify([settingsChannel sendMessage:[OCMArg checkWithBlock:^BOOL(id message) {
return [message[@"platformBrightness"] isEqualToString:@"light"];
}]]);

// Clean up mocks
[engine stopMocking];
[settingsChannel stopMocking];
}

- (void)testItReportsDarkPlatformBrightnessWhenTraitCollectionRequestsIt {
if (!@available(iOS 13, *)) {
return;
}

// Setup test.
id engine = OCMClassMock([FlutterEngine class]);

id settingsChannel = OCMClassMock([FlutterBasicMessageChannel class]);
OCMStub([engine settingsChannel]).andReturn(settingsChannel);

FlutterViewController* realVC = [[FlutterViewController alloc] initWithEngine:engine
nibName:nil
bundle:nil];
id mockTraitCollection =
[self fakeTraitCollectionWithUserInterfaceStyle:UIUserInterfaceStyleDark];

// We partially mock the real FlutterViewController to act as the OS and report
// the UITraitCollection of our choice. Mocking the object under test is not
// desirable, but given that the OS does not offer a DI approach to providing
// our own UITraitCollection, this seems to be the least bad option.
id partialMockVC = OCMPartialMock(realVC);
OCMStub([partialMockVC traitCollection]).andReturn(mockTraitCollection);

// Exercise behavior under test.
[partialMockVC traitCollectionDidChange:nil];

// Verify behavior.
OCMVerify([settingsChannel sendMessage:[OCMArg checkWithBlock:^BOOL(id message) {
return [message[@"platformBrightness"] isEqualToString:@"dark"];
}]]);

// Clean up mocks
[partialMockVC stopMocking];
[engine stopMocking];
[settingsChannel stopMocking];
[mockTraitCollection stopMocking];
}

// Creates a mocked UITraitCollection with nil values for everything except userInterfaceStyle,
// which is set to the given "style".
- (UITraitCollection*)fakeTraitCollectionWithUserInterfaceStyle:(UIUserInterfaceStyle)style {
id mockTraitCollection = OCMClassMock([UITraitCollection class]);
OCMStub([mockTraitCollection userInterfaceStyle]).andReturn(style);
return mockTraitCollection;
}

#pragma mark - Platform Contrast

- (void)testItReportsNormalPlatformContrastByDefault {
if (!@available(iOS 13, *)) {
return;
}

// Setup test.
id engine = OCMClassMock([FlutterEngine class]);

id settingsChannel = OCMClassMock([FlutterBasicMessageChannel class]);
OCMStub([engine settingsChannel]).andReturn(settingsChannel);

FlutterViewController* vc = [[FlutterViewController alloc] initWithEngine:engine
nibName:nil
bundle:nil];

// Exercise behavior under test.
[vc traitCollectionDidChange:nil];

// Verify behavior.
OCMVerify([settingsChannel sendMessage:[OCMArg checkWithBlock:^BOOL(id message) {
return [message[@"platformContrast"] isEqualToString:@"normal"];
}]]);

// Clean up mocks
[engine stopMocking];
[settingsChannel stopMocking];
}

- (void)testItReportsPlatformContrastWhenViewWillAppear {
if (!@available(iOS 13, *)) {
return;
}

// Setup test.
id engine = OCMClassMock([FlutterEngine class]);

id settingsChannel = OCMClassMock([FlutterBasicMessageChannel class]);
OCMStub([engine settingsChannel]).andReturn(settingsChannel);

FlutterViewController* vc = [[FlutterViewController alloc] initWithEngine:engine
nibName:nil
bundle:nil];

// Exercise behavior under test.
[vc viewWillAppear:false];

// Verify behavior.
OCMVerify([settingsChannel sendMessage:[OCMArg checkWithBlock:^BOOL(id message) {
return [message[@"platformContrast"] isEqualToString:@"normal"];
}]]);

// Clean up mocks
[engine stopMocking];
[settingsChannel stopMocking];
}

- (void)testItReportsHighContrastWhenTraitCollectionRequestsIt {
if (!@available(iOS 13, *)) {
return;
}

// Setup test.
id engine = OCMClassMock([FlutterEngine class]);

id settingsChannel = OCMClassMock([FlutterBasicMessageChannel class]);
OCMStub([engine settingsChannel]).andReturn(settingsChannel);

FlutterViewController* realVC = [[FlutterViewController alloc] initWithEngine:engine
nibName:nil
bundle:nil];
id mockTraitCollection = [self fakeTraitCollectionWithContrast:UIAccessibilityContrastHigh];

// We partially mock the real FlutterViewController to act as the OS and report
// the UITraitCollection of our choice. Mocking the object under test is not
// desirable, but given that the OS does not offer a DI approach to providing
// our own UITraitCollection, this seems to be the least bad option.
id partialMockVC = OCMPartialMock(realVC);
OCMStub([partialMockVC traitCollection]).andReturn(mockTraitCollection);

// Exercise behavior under test.
[partialMockVC traitCollectionDidChange:mockTraitCollection];

// Verify behavior.
OCMVerify([settingsChannel sendMessage:[OCMArg checkWithBlock:^BOOL(id message) {
return [message[@"platformContrast"] isEqualToString:@"high"];
}]]);

// Clean up mocks
[partialMockVC stopMocking];
[engine stopMocking];
[settingsChannel stopMocking];
[mockTraitCollection stopMocking];
}

// Creates a mocked UITraitCollection with nil values for everything except accessibilityContrast,
// which is set to the given "contrast".
- (UITraitCollection*)fakeTraitCollectionWithContrast:(UIAccessibilityContrast)contrast {
id mockTraitCollection = OCMClassMock([UITraitCollection class]);
OCMStub([mockTraitCollection accessibilityContrast]).andReturn(contrast);
return mockTraitCollection;
}

@end
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
objects = {

/* Begin PBXBuildFile section */
0D17A5C022D78FCD0057279F /* FlutterViewControllerTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 0D17A5BF22D78FCD0057279F /* FlutterViewControllerTest.m */; };
0D17A5C022D78FCD0057279F /* FlutterViewControllerTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 0D17A5BF22D78FCD0057279F /* FlutterViewControllerTest.m */; settings = {COMPILER_FLAGS = "-fobjc-arc"; }; };
0D4C3FB022DF9F5300A67C70 /* FlutterPluginAppLifeCycleDelegateTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 0D4C3FAF22DF9F5300A67C70 /* FlutterPluginAppLifeCycleDelegateTest.m */; settings = {COMPILER_FLAGS = "-fobjc-arc"; }; };
0D52D3BD22C566D50011DEBD /* FlutterBinaryMessengerRelayTest.mm in Sources */ = {isa = PBXBuildFile; fileRef = 0D52D3B622C566D50011DEBD /* FlutterBinaryMessengerRelayTest.mm */; settings = {COMPILER_FLAGS = "-fobjc-arc"; }; };
0D6AB6B622BB05E100EEE540 /* AppDelegate.m in Sources */ = {isa = PBXBuildFile; fileRef = 0D6AB6B522BB05E100EEE540 /* AppDelegate.m */; };
Expand Down