Skip to content

Commit 81138eb

Browse files
yakovmanshinyakovmanshin
authored
[#7309] Fixed messageDisplaySuppressed Ignored on Slow Networks (#7313)
* Added suppressMessageDisplay Check Before Message Display * Updated FIRIAMDisplayExecutorTests * Updated Format with style.sh * Added Logging for Early Return in FIRIAMDisplayExecutor
1 parent a6b1c3d commit 81138eb

File tree

2 files changed

+110
-12
lines changed

2 files changed

+110
-12
lines changed

‎FirebaseInAppMessaging/Sources/Flows/FIRIAMDisplayExecutor.m

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,14 @@ - (void)displayForMessage:(FIRIAMMessageDefinition *)message
638638
}
639639
}
640640

641+
// On slow networks, image loading may take significant time,
642+
// in which the value of `suppressMessageDisplay` could change.
643+
if (self.suppressMessageDisplay) {
644+
FIRLogDebug(kFIRLoggerInAppMessaging, @"I-IAM400042",
645+
@"Message display suppressed by developer at message display time.");
646+
return;
647+
}
648+
641649
self.impressionRecorded = NO;
642650
self.isMsgBeingDisplayed = YES;
643651

‎FirebaseInAppMessaging/Tests/Unit/FIRIAMDisplayExecutorTests.m

Lines changed: 102 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ @interface FIRIAMMessageContentDataForTesting : NSObject <FIRIAMMessageContentDa
3737
@property(nonatomic, nullable) NSURL *imageURL;
3838
@property(nonatomic, nullable) NSURL *landscapeImageURL;
3939
@property BOOL errorEncountered;
40+
@property BOOL loadImagesAsynchronously;
4041

4142
- (instancetype)initWithMessageTitle:(NSString *)title
4243
messageBody:(NSString *)body
@@ -46,7 +47,8 @@ - (instancetype)initWithMessageTitle:(NSString *)title
4647
secondaryActionURL:(nullable NSURL *)secondaryActionURL
4748
imageURL:(nullable NSURL *)imageURL
4849
landscapeImageURL:(nullable NSURL *)landscapeImageURL
49-
hasImageError:(BOOL)hasImageError;
50+
hasImageError:(BOOL)hasImageError
51+
loadImagesAsynchronously:(BOOL)loadImagesAsynchronously;
5052
@end
5153

5254
@implementation FIRIAMMessageContentDataForTesting
@@ -58,7 +60,8 @@ - (instancetype)initWithMessageTitle:(NSString *)title
5860
secondaryActionURL:(nullable NSURL *)secondaryActionURL
5961
imageURL:(nullable NSURL *)imageURL
6062
landscapeImageURL:(nullable NSURL *)landscapeImageURL
61-
hasImageError:(BOOL)hasImageError {
63+
hasImageError:(BOOL)hasImageError
64+
loadImagesAsynchronously:(BOOL)loadImagesAsynchronously {
6265
if (self = [super init]) {
6366
_titleText = title;
6467
_bodyText = body;
@@ -69,6 +72,7 @@ - (instancetype)initWithMessageTitle:(NSString *)title
6972
_actionURL = actionURL;
7073
_secondaryActionURL = secondaryActionURL;
7174
_errorEncountered = hasImageError;
75+
_loadImagesAsynchronously = loadImagesAsynchronously;
7276
}
7377
return self;
7478
}
@@ -77,14 +81,38 @@ - (void)loadImageDataWithBlock:(void (^)(NSData *_Nullable imageData,
7781
NSData *_Nullable landscapeImageData,
7882
NSError *_Nullable error))block {
7983
if (self.errorEncountered) {
80-
block(nil, nil, [NSError errorWithDomain:@"image error" code:0 userInfo:nil]);
84+
NSError *error = [NSError errorWithDomain:@"image error" code:0 userInfo:nil];
85+
86+
if (_loadImagesAsynchronously) {
87+
[self performOnMainQueueAfterDelay:0.01
88+
block:^{
89+
block(nil, nil, error);
90+
}];
91+
} else {
92+
block(nil, nil, error);
93+
}
8194
} else {
8295
NSData *imageData = [@"image data" dataUsingEncoding:NSUTF8StringEncoding];
8396
NSData *landscapeImageData = [@"landscape image data" dataUsingEncoding:NSUTF8StringEncoding];
8497

85-
block(imageData, landscapeImageData, nil);
98+
if (_loadImagesAsynchronously) {
99+
[self performOnMainQueueAfterDelay:0.01
100+
block:^{
101+
block(imageData, landscapeImageData, nil);
102+
}];
103+
} else {
104+
block(imageData, landscapeImageData, nil);
105+
}
86106
}
87107
}
108+
109+
- (void)performOnMainQueueAfterDelay:(NSTimeInterval)delay block:(void (^)(void))block {
110+
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, delay * NSEC_PER_SEC), dispatch_get_main_queue(),
111+
^{
112+
block();
113+
});
114+
}
115+
88116
@end
89117

90118
// Defines how the message display component triggers the delegate in unit testing.
@@ -207,8 +235,8 @@ @interface FIRIAMDisplayExecutorTests : XCTestCase
207235

208236
@property id<FIRInAppMessagingDisplay> mockMessageDisplayComponent;
209237

210-
// four pre-defined messages
211-
@property FIRIAMMessageDefinition *m1, *m2, *m3, *m4, *m5;
238+
// Pre-defined messages
239+
@property FIRIAMMessageDefinition *m1, *m2, *m3, *m4, *m5, *m6;
212240
@end
213241

214242
@implementation FIRIAMDisplayExecutorTests
@@ -222,7 +250,7 @@ - (void)setupMessageTexture {
222250
FIRIAMDisplayTriggerDefinition *contextualTriggerDefinition =
223251
[[FIRIAMDisplayTriggerDefinition alloc] initWithFirebaseAnalyticEvent:@"test_event"];
224252

225-
// m2, m4, and m5 will be of app open trigger
253+
// m2, m4, m5, and m6 will be of app open trigger
226254
FIRIAMDisplayTriggerDefinition *appOpentriggerDefinition =
227255
[[FIRIAMDisplayTriggerDefinition alloc] initForAppForegroundTrigger];
228256

@@ -235,7 +263,8 @@ - (void)setupMessageTexture {
235263
secondaryActionURL:nil
236264
imageURL:[NSURL URLWithString:@"https://google.com/image"]
237265
landscapeImageURL:nil
238-
hasImageError:NO];
266+
hasImageError:NO
267+
loadImagesAsynchronously:NO];
239268

240269
FIRIAMRenderingEffectSetting *renderSetting1 =
241270
[FIRIAMRenderingEffectSetting getDefaultRenderingEffectSetting];
@@ -261,7 +290,8 @@ - (void)setupMessageTexture {
261290
secondaryActionURL:nil
262291
imageURL:[NSURL URLWithString:@"https://unsplash.it/300/400"]
263292
landscapeImageURL:nil
264-
hasImageError:NO];
293+
hasImageError:NO
294+
loadImagesAsynchronously:NO];
265295

266296
FIRIAMRenderingEffectSetting *renderSetting2 =
267297
[FIRIAMRenderingEffectSetting getDefaultRenderingEffectSetting];
@@ -287,7 +317,8 @@ - (void)setupMessageTexture {
287317
secondaryActionURL:nil
288318
imageURL:[NSURL URLWithString:@"https://google.com/image"]
289319
landscapeImageURL:nil
290-
hasImageError:NO];
320+
hasImageError:NO
321+
loadImagesAsynchronously:NO];
291322

292323
FIRIAMRenderingEffectSetting *renderSetting3 =
293324
[FIRIAMRenderingEffectSetting getDefaultRenderingEffectSetting];
@@ -313,7 +344,8 @@ - (void)setupMessageTexture {
313344
secondaryActionURL:nil
314345
imageURL:[NSURL URLWithString:@"https://google.com/image"]
315346
landscapeImageURL:nil
316-
hasImageError:NO];
347+
hasImageError:NO
348+
loadImagesAsynchronously:NO];
317349

318350
FIRIAMRenderingEffectSetting *renderSetting4 =
319351
[FIRIAMRenderingEffectSetting getDefaultRenderingEffectSetting];
@@ -353,7 +385,8 @@ - (void)setupMessageTexture {
353385
secondaryActionURL:nil
354386
imageURL:[NSURL URLWithString:@"https://google.com/image"]
355387
landscapeImageURL:nil
356-
hasImageError:NO];
388+
hasImageError:NO
389+
loadImagesAsynchronously:NO];
357390

358391
FIRIAMRenderingEffectSetting *renderSetting5 =
359392
[FIRIAMRenderingEffectSetting getDefaultRenderingEffectSetting];
@@ -372,6 +405,33 @@ - (void)setupMessageTexture {
372405
appData:nil
373406
experimentPayload:nil
374407
isTestMessage:NO];
408+
409+
FIRIAMMessageContentDataForTesting *m6ContentData = [[FIRIAMMessageContentDataForTesting alloc]
410+
initWithMessageTitle:@"m6 title"
411+
messageBody:@"message body"
412+
actionButtonText:nil
413+
secondaryActionButtonText:nil
414+
actionURL:[NSURL URLWithString:@"http://google.com"]
415+
secondaryActionURL:nil
416+
imageURL:[NSURL URLWithString:@"https://google.com/image"]
417+
landscapeImageURL:nil
418+
hasImageError:NO
419+
loadImagesAsynchronously:YES];
420+
421+
FIRIAMRenderingEffectSetting *renderSetting6 =
422+
[FIRIAMRenderingEffectSetting getDefaultRenderingEffectSetting];
423+
renderSetting6.viewMode = FIRIAMRenderAsBannerView;
424+
425+
FIRIAMMessageRenderData *renderData6 =
426+
[[FIRIAMMessageRenderData alloc] initWithMessageID:@"m6"
427+
messageName:@"name"
428+
contentData:m6ContentData
429+
renderingEffect:renderSetting6];
430+
431+
self.m6 = [[FIRIAMMessageDefinition alloc] initWithRenderData:renderData6
432+
startTime:activeStartTime
433+
endTime:activeEndTime
434+
triggerDefinition:@[ appOpentriggerDefinition ]];
375435
}
376436

377437
NSTimeInterval DISPLAY_MIN_INTERVALS = 1;
@@ -888,6 +948,36 @@ - (void)testNoRenderingIfMessageDisplayIsSuppressed {
888948
XCTAssertEqual(1, remainingMsgCount2);
889949
}
890950

951+
- (void)testNoRenderingIfMessageDisplayIsSuppressedDuringImageLoading {
952+
// This setup allows next message to be displayed from display interval perspective.
953+
OCMStub([self.mockTimeFetcher currentTimestampInSeconds])
954+
.andReturn(DISPLAY_MIN_INTERVALS * 60 + 100);
955+
956+
[self.clientMessageCache setMessageData:@[ self.m6 ]];
957+
958+
FIRIAMMessageDisplayForTesting *display = [[FIRIAMMessageDisplayForTesting alloc]
959+
initWithDelegateInteraction:FIRInAppMessagingDelegateInteractionClick];
960+
self.displayExecutor.messageDisplayComponent = display;
961+
[self.displayExecutor checkAndDisplayNextAppForegroundMessage];
962+
self.displayExecutor.suppressMessageDisplay = YES;
963+
964+
XCTestExpectation *expectation = [[XCTestExpectation alloc] init];
965+
966+
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 0.05 * NSEC_PER_SEC), dispatch_get_main_queue(),
967+
^{
968+
// no message display has happened
969+
XCTAssertNil(display.message);
970+
971+
NSInteger remainingMsgCount = [self.clientMessageCache allRegularMessages].count;
972+
// no message is removed from the cache
973+
XCTAssertEqual(1, remainingMsgCount);
974+
975+
[expectation fulfill];
976+
});
977+
978+
[self waitForExpectations:@[ expectation ] timeout:0.1];
979+
}
980+
891981
// No contextual message rendering if suppress message display flag is turned on
892982
- (void)testNoContextualMsgRenderingIfMessageDisplayIsSuppressed {
893983
// This setup allows next message to be displayed from display interval perspective.

0 commit comments

Comments
 (0)