Changes between Version 3 and Version 4 of Ticket #5015, comment 24


Ignore:
Timestamp:
03/21/2017 10:38:15 AM (3 years ago)
Author:
sebastian
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #5015, comment 24

    v3 v4  
    1 Well, the reason we inject CSS selectors in batches, was a limitation in Blink that caused it to struggle when inserting a long list of selectors at once, but this limit has been raised significantly a long time ago. Then there was Safari which still has a similar limit, but is no longer supported by this code. It seems inserting selectors in batches is no longer necessary. So once we implement #242 and use `chrome.tabs.insertCSS()` we might just insert all selectors at once, neither does `insertCSS()` report any parsing errors. Furthermore, Chrome is used to deoptimize the whole function when using `try..catch`, and this code path is critical as it blocks page load. Recently, the deoptimization was removed from V8, but I'm not sure which Chrome versions are still effected. 
     1Well, the reason we inject CSS selectors in batches, was a limitation in Blink that caused it to struggle when inserting a long list of selectors at once, but this limit has been raised significantly a long time ago. Then there was Safari which still has a similar limit, but is no longer supported by this code. It seems inserting selectors in batches is no longer necessary. So once we implement #242 and use `chrome.tabs.insertCSS()` we might just insert all selectors at once, neither does `insertCSS()` give us an error when there are any invalid selectors. Furthermore, Chrome is used to deoptimize the whole function when using `try..catch`, and this code path is critical as it blocks page load. Recently, the deoptimization was removed from V8, but I'm not sure which Chrome versions are still effected. 
    22 
    33However, the issue ''trev'' originally reproduced still seems to be another one. As he told me on IRC, he reproduced it ~3 times out of ~100. So the theory that there was an invalid selector in the subscriptions, which then automatically got updated before reloading the page, doesn't seem plausible.