Skip to content
This repository was archived by the owner on Dec 11, 2019. It is now read-only.

fix "Cannot read property 'size' of undefined" error when getting persistent state #7467

Merged
merged 2 commits into from
Mar 3, 2017

Conversation

diracdeltas
Copy link
Member

@diracdeltas diracdeltas commented Mar 2, 2017

Test plan

run npm run unittest -- --grep='returns the state \(unchanged\) if tabs is falsey'

Description

When upgrading from 0.13.4 to 0.13.5, this was causing getting persistent state
to fail on startup, thereby losing session state. Fix #7466

Test Plan:

  1. copy your session-store-1 from Brave 0.13.4 into the brave-development profile folder
  2. npm start
  3. Brave should show bookmarks, settings, pinned tabs, etc from the 0.13.4 profile
  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

@diracdeltas diracdeltas added this to the 0.13.5 milestone Mar 2, 2017
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

Big ++ from me; I should have put that check in

…sistent state

When upgrading from 0.13.4 to 0.13.5, this was causing getting persistent state
to fail on startup, thereby losing session state. Fix #7466

Test Plan:
1. copy your session-store-1 from Brave 0.13.4 into the brave-development profile folder
2. npm start
3. Brave should show tabs, bookmarks, settings, etc from the 0.13.4 profile
@@ -483,6 +483,10 @@ describe('tabState unit tests', function () {
const expectedAppState = defaultAppState.set('tabs', tabsWithoutField)
assert.deepEqual(newAppState, expectedAppState)
})
it('returns the state (unchanged) if tabs is falsey', function () {
const newAppState = tabState.removeTabField(defaultAppState, 'loginRequiredDetail')
Copy link
Member Author

Choose a reason for hiding this comment

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

seems that you would want defaultAppState without the tabs field for this test?

Copy link
Member Author

Choose a reason for hiding this comment

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

otherwise lgtm

Copy link
Member

Choose a reason for hiding this comment

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

Fixing!

Copy link
Member

Choose a reason for hiding this comment

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

Fixed- that variable exists in other files (with other states- whoops!). Thanks for catching 😄

Auditors: @diracdeltas

Test Plan: `npm run unittest -- --grep='returns the state \(unchanged\) if tabs is falsey'`
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants