Skip to content

Bug 1194664 - Add checks when adding/removing KVOURL observers on BVC#933

Merged
sleroux merged 1 commit intomasterfrom
sleroux/Bug1194664-CrashRestoreSessionTests
Aug 14, 2015
Merged

Bug 1194664 - Add checks when adding/removing KVOURL observers on BVC#933
sleroux merged 1 commit intomasterfrom
sleroux/Bug1194664-CrashRestoreSessionTests

Conversation

@sleroux
Copy link

@sleroux sleroux commented Aug 14, 2015

Added a flag to keep track of when we've attached the KVOURL observer since it can be called in two different places. The crash was being caused by the web page restoring after the page was loaded. This scenario wouldn't happen for real since a restoration always conincides with a web view creation but added the checks regardless. Also wrapped the removal of the observer with a Try/catch block just in case.

@thebnich
Copy link
Contributor

Ugh, KVO management is a pain. Rather than worrying about all this dynamic KVO registration based on restoring, what if KVOURL is always registered like the others, then we just did something like this?

case KVOURL:
    if restoring { break }
    ...

@sleroux
Copy link
Author

sleroux commented Aug 14, 2015

Ya that would probably work better. Guess it's similar to the originally approach I had where I just set the boolean flag on Browser

@sleroux sleroux force-pushed the sleroux/Bug1194664-CrashRestoreSessionTests branch from 446b6ac to 8891c27 Compare August 14, 2015 17:53
sleroux pushed a commit that referenced this pull request Aug 14, 2015
…essionTests

Bug 1194664 - Add checks when adding/removing KVOURL observers on BVC
@sleroux sleroux merged commit 92493c1 into master Aug 14, 2015
@sleroux sleroux deleted the sleroux/Bug1194664-CrashRestoreSessionTests branch August 14, 2015 17:54
isabelrios pushed a commit to isabelrios/firefox-ios that referenced this pull request Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants