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

Normalize all tab id values to integer #3428

Closed
gorhill opened this issue Jan 11, 2018 · 1 comment
Closed

Normalize all tab id values to integer #3428

gorhill opened this issue Jan 11, 2018 · 1 comment

Comments

@gorhill
Copy link
Owner

gorhill commented Jan 11, 2018

In light of of regression #3425.

Tab id values are natively integer values, but they are currently normalized to strings internally on the platform-independent code.

The reason for this is historical: tab id values were used as keys in Object used as dictionary (Map/Set were not available back then). When an integer is used as a key, it is converted to string internally by javascript, and this means when iterating through the items in the Object-based dictionary, the tab id key value was a string. Instead of converting the string back to integer, back then I chose to normalize all tab id values to string.

This issue is to normalize tab id values to their native integer representation, and convert Object-based dictionaries to Maps where needed so as to avoid the implicit javascript conversion of tab id values to string.

The behind-the-scene scope will be represented with the integer -1.

The no tab found value will be 0.

@jspenguin2017
Copy link
Contributor

jspenguin2017 commented Jan 15, 2018

Are you absolutely sure Chrome will never assign a tab to have ID 0?
When there is no tab, Chrome will set the tab ID to be the constant chrome.tabs.TAB_ID_NONE which happens to be -1. It is safe enough to assume that all tab IDs are larger than -1 although there is no documentation on that. I think -2 (or null, Infinity, etc) are safer than 0.

gorhill added a commit that referenced this issue Feb 26, 2018
gorhill added a commit that referenced this issue Feb 26, 2018
@gorhill gorhill closed this as completed Apr 16, 2018
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

No branches or pull requests

2 participants