Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix issue on timeline bubbles not showing proper content after decrypt #7397

Merged
merged 4 commits into from
Mar 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 34 additions & 1 deletion Riot/Modules/MatrixKit/Models/Room/MXKRoomDataSourceManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,21 @@ @interface MXKRoomDataSourceManager()
*/
NSMutableDictionary *roomDataSources;

/**
The list of rooms with a "late decryption" event. Causing bubbles issues
Each element is a room ID.
*/
NSMutableSet *roomDataSourcesToDestroy;

/**
Observe UIApplicationDidReceiveMemoryWarningNotification to dispose of any resources that can be recreated.
*/
id UIApplicationDidReceiveMemoryWarningNotificationObserver;

/**
Observe kMXEventDidDecryptNotification to get late decrypted events.
*/
id mxEventDidDecryptNotificationObserver;
}

@end
Expand Down Expand Up @@ -119,6 +130,7 @@ - (instancetype)initWithMatrixSession:(MXSession *)matrixSession
{
mxSession = matrixSession;
roomDataSources = [NSMutableDictionary dictionary];
roomDataSourcesToDestroy = [NSMutableSet set];
_releasePolicy = MXKRoomDataSourceManagerReleasePolicyNeverRelease;

[[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(didMXSessionDidLeaveRoom:) name:kMXSessionDidLeaveRoomNotification object:nil];
Expand All @@ -138,6 +150,12 @@ - (instancetype)initWithMatrixSession:(MXSession *)matrixSession
}

}];

// Observe late decrypted events, and store rooms ids in memory
mxEventDidDecryptNotificationObserver = [[NSNotificationCenter defaultCenter] addObserverForName:kMXEventDidDecryptNotification object:nil queue:[NSOperationQueue mainQueue] usingBlock:^(NSNotification *notif) {
MXEvent *decryptedEvent = notif.object;
[self->roomDataSourcesToDestroy addObject:decryptedEvent.roomId];
}];
Comment on lines +155 to +158
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
mxEventDidDecryptNotificationObserver = [[NSNotificationCenter defaultCenter] addObserverForName:kMXEventDidDecryptNotification object:nil queue:[NSOperationQueue mainQueue] usingBlock:^(NSNotification *notif) {
MXEvent *decryptedEvent = notif.object;
[self->roomDataSourcesToDestroy addObject:decryptedEvent.roomId];
}];
mxEventDidDecryptNotificationObserver = [NSNotificationCenter.defaultCenter addObserverForName:kMXEventDidDecryptNotification object:nil queue:NSOperationQueue.mainQueue usingBlock:^(NSNotification *notif) {
MXEvent *decryptedEvent = notif.object;
[self->roomDataSourcesToDestroy addObject:decryptedEvent.roomId];
}];

Minor: dot notation should work on static vars.

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 was trying to remain consistent with similar denotation in the same file

Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't care much about that.
Writing more readable code is better to me.
I'll leave this up to you in the end. ;)

}
return self;
}
Expand All @@ -156,6 +174,11 @@ - (void)destroy
[[NSNotificationCenter defaultCenter] removeObserver:UIApplicationDidReceiveMemoryWarningNotificationObserver];
UIApplicationDidReceiveMemoryWarningNotificationObserver = nil;
}
if (mxEventDidDecryptNotificationObserver)
{
[[NSNotificationCenter defaultCenter] removeObserver:mxEventDidDecryptNotificationObserver];
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
[[NSNotificationCenter defaultCenter] removeObserver:mxEventDidDecryptNotificationObserver];
[NSNotificationCenter.defaultCenter removeObserver:mxEventDidDecryptNotificationObserver];

mxEventDidDecryptNotificationObserver = nil;
}
}

#pragma mark
Expand Down Expand Up @@ -202,9 +225,19 @@ - (void)roomDataSourceForRoom:(NSString *)roomId create:(BOOL)create onComplete:

// If not available yet, create the room data source
MXKRoomDataSource *roomDataSource = roomDataSources[roomId];


// check if the room's dataSource has events with late decryption issues and destroys it
BOOL roomDataSourceToBeDestroyed = [roomDataSourcesToDestroy containsObject:roomId];

if (roomDataSource && roomDataSourceToBeDestroyed && create) {
[roomDataSource destroy];
roomDataSources[roomId] = nil;
Comment on lines +233 to +234
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this should be reusing closeRoomDataSourceWithRoomId in case that ever gets tweaked and this is missed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So basically I found that the best spot to do it is before opening a room. Calling it on closing it won't cover a case when you receive the key once closed.
Other than that, I did not destroy everything when the user is inside the room.
Also I did found out the destroy is really necessary otherwise it won't work properly

roomDataSource = nil;
}

if (!roomDataSource && create && roomId)
{
[roomDataSourcesToDestroy removeObject:roomId];
[_roomDataSourceClass loadRoomDataSourceWithRoomId:roomId threadId:nil andMatrixSession:mxSession onComplete:^(id roomDataSource) {
[self addRoomDataSource:roomDataSource];
onComplete(roomDataSource);
Expand Down
1 change: 1 addition & 0 deletions changelog.d/pr-7397.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fix issue on timeline's bubbles not showing proper content after decrypt