Crash preventation #4661

GUserx · 2020-01-11T16:26:27Z

googlebot · 2020-01-11T16:26:30Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot · 2020-01-11T16:34:13Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

GUserx · 2020-01-12T10:04:08Z

Crashlytics/Crashlytics/Models/FIRCLSFileManager.m

+  BOOL contentsOfProcessingPathRemoved = [self removeContentsOfProcessingPath];
+  BOOL contentsOfPendingPathRemoved = [self removeContentsOfPendingPath];
+  BOOL contentsOfDirectoryAtPreparedPathRemoved =
+      [self removeContentsOfDirectoryAtPath:self.preparedPath];
+  BOOL contentsOfDirectoryAtActivePathRemoved =
+      [self removeContentsOfDirectoryAtPath:self.activePath];
+  success = contentsOfProcessingPathRemoved && contentsOfPendingPathRemoved &&
+            contentsOfDirectoryAtPreparedPathRemoved && contentsOfDirectoryAtActivePathRemoved;


Here might be this type of form if you wish:
return [self removeContentsOfProcessingPath] & [self removeContentsOfPendingPath] & ... etc
But the above case is more friendly for debug imo =)

We can declare success at the time we check for all the criteria:
BOOL success = contentsOfProcessingPathRemoved && contentsOfPendingPathRemoved &&
contentsOfDirectoryAtPreparedPathRemoved && contentsOfDirectoryAtActivePathRemoved;

We can declare success at the time we check for all the criteria:
BOOL success = contentsOfProcessingPathRemoved && contentsOfPendingPathRemoved &&
contentsOfDirectoryAtPreparedPathRemoved && contentsOfDirectoryAtActivePathRemoved;

You are definitely right :)
Thanks!

paulb777 · 2020-01-12T16:03:54Z

@GUserx Thanks for the suggestions.

We'd appreciate if you'd send separate PRs for different pods. In this case, a separate PR for the Installations versus Crashylytics would help with the review process.

For the Installations change, please investigate the subsequent CI failure in https://travis-ci.org/firebase/firebase-ios-sdk/jobs/635952938

- NOTE  | [iOS] xcodebuild:  /Users/travis/build/firebase/firebase-ios-sdk/Example/Messaging/Tests/FIRMessagingContextManagerServiceTest.m:85: error: -[FIRMessagingContextManagerServiceTest testMessageWithFutureStartTime] : ((self.scheduledLocalNotifications.count) equal to (1)) failed: ("0") is not equal to ("1")
- NOTE  | [iOS] xcodebuild:  /Users/travis/build/firebase/firebase-ios-sdk/Example/Messaging/Tests/FIRMessagingContextManagerServiceTest.m:170: error: -[FIRMessagingContextManagerServiceTest testTimedNotificationsUserInfo] : ((self.scheduledLocalNotifications.count) equal to (1)) failed: ("0") is not equal to ("1")
- NOTE  | [iOS] xcodebuild:  /Users/travis/build/firebase/firebase-ios-sdk/Example/Messaging/Tests/FIRMessagingContextManagerServiceTest.m:175: error: -[FIRMessagingContextManagerServiceTest testTimedNotificationsUserInfo] : ((notification.userInfo[messageIdentifierKey]) equal to (messageIdentifier)) failed: ("(null)") is not equal to ("fcm-cm-test1")
- NOTE  | [iOS] xcodebuild:  /Users/travis/build/firebase/firebase-ios-sdk/Example/Messaging/Tests/FIRMessagingContextManagerServiceTest.m:176: error: -[FIRMessagingContextManagerServiceTest testTimedNotificationsUserInfo] : ((notification.userInfo[customDataKey]) equal to (customData)) failed: ("(null)") is not equal to ("world")

GUserx · 2020-01-12T16:28:02Z

@GUserx Thanks for the suggestions.

We'd appreciate if you'd send separate PRs for different pods. In this case, a separate PR for the Installations versus Crashylytics would help with the review process.

For the Installations change, please investigate the subsequent CI failure in https://travis-ci.org/firebase/firebase-ios-sdk/jobs/635952938

- NOTE  | [iOS] xcodebuild:  /Users/travis/build/firebase/firebase-ios-sdk/Example/Messaging/Tests/FIRMessagingContextManagerServiceTest.m:85: error: -[FIRMessagingContextManagerServiceTest testMessageWithFutureStartTime] : ((self.scheduledLocalNotifications.count) equal to (1)) failed: ("0") is not equal to ("1")
- NOTE  | [iOS] xcodebuild:  /Users/travis/build/firebase/firebase-ios-sdk/Example/Messaging/Tests/FIRMessagingContextManagerServiceTest.m:170: error: -[FIRMessagingContextManagerServiceTest testTimedNotificationsUserInfo] : ((self.scheduledLocalNotifications.count) equal to (1)) failed: ("0") is not equal to ("1")
- NOTE  | [iOS] xcodebuild:  /Users/travis/build/firebase/firebase-ios-sdk/Example/Messaging/Tests/FIRMessagingContextManagerServiceTest.m:175: error: -[FIRMessagingContextManagerServiceTest testTimedNotificationsUserInfo] : ((notification.userInfo[messageIdentifierKey]) equal to (messageIdentifier)) failed: ("(null)") is not equal to ("fcm-cm-test1")
- NOTE  | [iOS] xcodebuild:  /Users/travis/build/firebase/firebase-ios-sdk/Example/Messaging/Tests/FIRMessagingContextManagerServiceTest.m:176: error: -[FIRMessagingContextManagerServiceTest testTimedNotificationsUserInfo] : ((notification.userInfo[customDataKey]) equal to (customData)) failed: ("(null)") is not equal to ("world")

@paulb777 Thanks for your comment.
I did revert to previous version of this commit.

paulb777 · 2020-01-12T19:32:34Z

Thanks. Note that CI did not run for Crashlytics because of the issue fixed by #4663

samedson · 2020-01-13T15:29:20Z

@GUserx thanks for the PR! We're going to run through some validation and review this PR later in the week after we've gone through with the first release

GUserx · 2020-01-13T17:11:26Z

@GUserx thanks for the PR! We're going to run through some validation and review this PR later in the week after we've gone through with the first release

Got it. Thank you for info.

googlebot added the cla: no label Jan 11, 2020

GUserx force-pushed the code-refactor branch from 96daff5 to 08e48de Compare January 11, 2020 16:34

googlebot added cla: yes and removed cla: no labels Jan 11, 2020

GUserx mentioned this pull request Jan 11, 2020

Crash prevention in FIRCLSNetworkClient #4659

Closed

paulb777 added the api: crashlytics label Jan 11, 2020

GUserx force-pushed the code-refactor branch from 08e48de to 0bc6a5c Compare January 12, 2020 09:54

GUserx commented Jan 12, 2020

View reviewed changes

GUserx closed this Jan 12, 2020

GUserx reopened this Jan 12, 2020

GUserx force-pushed the code-refactor branch from 0bc6a5c to 53497f2 Compare January 12, 2020 16:20

morganchen12 requested review from samedson and removed request for samedson January 21, 2020 20:53

Crash preventation

bf38689

GUserx force-pushed the code-refactor branch from 53497f2 to bf38689 Compare January 25, 2020 09:10

jasonhu-g approved these changes Jan 27, 2020

View reviewed changes

jasonhu-g merged commit 34f720c into firebase:master Jan 27, 2020

samedson mentioned this pull request Jan 31, 2020

Update Crashlytics CHANGELOG for beta.3 out of band release #4788

Merged

firebase locked and limited conversation to collaborators Feb 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Crash preventation #4661

Crash preventation #4661

GUserx commented Jan 11, 2020 •

edited by paulb777

Loading

googlebot commented Jan 11, 2020

googlebot commented Jan 11, 2020

GUserx Jan 12, 2020 •

edited

Loading

jasonhu-g Jan 24, 2020

GUserx Jan 25, 2020

paulb777 commented Jan 12, 2020

GUserx commented Jan 12, 2020

paulb777 commented Jan 12, 2020 •

edited

Loading

samedson commented Jan 13, 2020

GUserx commented Jan 13, 2020

Crash preventation #4661

Crash preventation #4661

Conversation

GUserx commented Jan 11, 2020 • edited by paulb777 Loading

googlebot commented Jan 11, 2020

googlebot commented Jan 11, 2020

GUserx Jan 12, 2020 • edited Loading

Choose a reason for hiding this comment

jasonhu-g Jan 24, 2020

Choose a reason for hiding this comment

GUserx Jan 25, 2020

Choose a reason for hiding this comment

paulb777 commented Jan 12, 2020

GUserx commented Jan 12, 2020

paulb777 commented Jan 12, 2020 • edited Loading

samedson commented Jan 13, 2020

GUserx commented Jan 13, 2020

GUserx commented Jan 11, 2020 •

edited by paulb777

Loading

GUserx Jan 12, 2020 •

edited

Loading

paulb777 commented Jan 12, 2020 •

edited

Loading