Bug 1674053 Comment 8 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to :Gijs (he/him) from comment #6)
> So as always, it pays to check your assumptions... yesterday I looked into this more and figured the slow part would be checking the count of bookmarks in the toolbar - the test flicks between a blank page on which we show the toolbar, to a webpage where we do not.
> 
> So I tried commenting out the checks for the number of bookmarks, and it [made sod all difference](https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=DtL2NHU9Tq6joAy36sWMNg.0&revision=cfff259fbb08bbe1b46afe4f3c62a42f2ffb10af).
> 
> From a profile, it would seem that actually, the bigger cost is creating and destroying the places view when we show/hide the toolbar, and calling `inferFromText` every time we show/hide the toolbar (tripped from the `toolbarvisibilitychange` event). For the bookmarks toolbar, we could probably just adopt the data from the navbar, which doesn't show/hide.

Unfortunately, despite what the profile says, this didn't seem to help much, so it's back to bisecting the change to figure out what is broken. :-\
(In reply to :Gijs (he/him) from comment #6)
> So as always, it pays to check your assumptions... yesterday I looked into this more and figured the slow part would be checking the count of bookmarks in the toolbar - the test flicks between a blank page on which we show the toolbar, to a webpage where we do not.
> 
> So I tried commenting out the checks for the number of bookmarks, and it [made sod all difference](https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=DtL2NHU9Tq6joAy36sWMNg.0&revision=cfff259fbb08bbe1b46afe4f3c62a42f2ffb10af).
> 
> From a profile, it would seem that actually, the bigger cost is creating and destroying the places view when we show/hide the toolbar, and calling `inferFromText` every time we show/hide the toolbar (tripped from the `toolbarvisibilitychange` event). For the bookmarks toolbar, we could probably just adopt the data from the navbar, which doesn't show/hide.

Unfortunately, despite what the profile says, this didn't seem to help much, so it's back to bisecting the change to figure out what is broken. :-\

( https://treeherder.mozilla.org/jobs?repo=try&revision=71c64fc49f79850164e733cef7fd9e54e83e8dcd for reference; I might still end up submitting that because it's a net reduction of work, and well, I've written it now...)

Back to Bug 1674053 Comment 8