Page MenuHomePhabricator
Differential D6665

[native] Fixed crash when logging out while thread opened
ClosedPublic
Actions

Authored by • kuba on Feb 8 2023, 6:07 AM.
Tags
None
Referenced Files
F3489851: D6665.id22291.diff
Wed, Dec 18, 1:55 PM
F3489149: D6665.diff
Wed, Dec 18, 11:44 AM
Unknown Object (File)
Thu, Nov 28, 5:07 PM
Unknown Object (File)
Nov 16 2024, 9:36 PM
Unknown Object (File)
Nov 15 2024, 6:54 PM
Unknown Object (File)
Nov 15 2024, 6:53 PM
Unknown Object (File)
Nov 14 2024, 8:57 PM
Unknown Object (File)
Nov 8 2024, 2:22 AM
View All 85 Files
Subscribers
ashoat
atul

Details

Reviewers
tomek
Commits
rCOMM40596931e8c8: [native] Fixed crash when logging out while thread opened
Summary

Removed invariant, which was causing the error, and modified measureMessages method so that it can handle the case where messages is null.

Test Plan

Checked logging out while there was opened the chat window, now it doesn't throw an error.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

• kuba created this revision.Feb 8 2023, 6:07 AM
Herald added subscribers: atul, ashoat. · View Herald TranscriptFeb 8 2023, 6:07 AM
• kuba edited the summary of this revision. (Show Details)Feb 8 2023, 6:09 AM
Harbormaster completed remote builds in B16279: Diff 22275.Feb 8 2023, 6:23 AM
• kuba requested review of this revision.Feb 8 2023, 6:23 AM
tomek accepted this revision.Feb 9 2023, 2:45 AM

The summary can be improved a bit

checked null values in measureMessages method.

This can be replaced by more correct description, e.g.

modified measureMessages method so that it can handle the case where messages is null

native/chat/message-list-container.react.js
93–117 ↗(On Diff #22275)

It seems like it should be possible to just avoid calling measureMessages when messageListData is null. We even have some code to handle !newListData && oldListData case.
On the other hand, measureMessages already supports null threadInfo, so doing the same for messages also makes sense.

So it seems like the current approach is ok.

This revision is now accepted and ready to land.Feb 9 2023, 2:45 AM
Closed by commit rCOMM40596931e8c8: [native] Fixed crash when logging out while thread opened (authored by • kuba). · Explain WhyFeb 9 2023, 2:58 AM
This revision was automatically updated to reflect the committed changes.
• kuba added a commit: rCOMM40596931e8c8: [native] Fixed crash when logging out while thread opened.
• kuba edited the summary of this revision. (Show Details)Feb 9 2023, 3:00 AM
• kuba edited the test plan for this revision. (Show Details)

Revision Contents
Changeset List

Diff 22291

View Options

native/chat/chat-context-provider.react.js

Loading...
View Options

native/chat/chat-context.js

Loading...
View Options

native/chat/message-list-container.react.js

Loading...