1444694 - Set up the initial browser and tab in a dedicated method

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, enhancement)

Description

[Dão Gottwald [:dao]]

This might prove useful for bug 1443849. It doesn't buy us much right now except for further breaking up init(), which is nice.

Comment 1

[Dão Gottwald [:dao]]

Attachment: Bug 1444694 - Set up the initial browser and tab in a dedicated method.

Review commit: https://reviewboard.mozilla.org/r/226844/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/226844/

Comment 2

[Dão Gottwald [:dao]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a046a1a4b8a128935070b4522d3c071149456837

Comment 3

[:Gijs (he/him)]

Comment on attachment 8957876 [details]
Bug 1444694 - Set up the initial browser and tab in a dedicated method.

https://reviewboard.mozilla.org/r/226844/#review232610

Tentative r=me. I mean, right now, nothing much changes (if nothing else touches the properties any earlier) because the lazy getters both get invoked on the line immediately after the one where their code was removed ( ie the `new TabProgressListener` call). But this is a bit odd, at least, because only-loosely-related things now depend on being accessed through this getter (e.g. whether the initial tab is `fullyOpen` and has a `_tPos` property).

Still r=me because nothing changes and I can see that we will want to simplify this further, and doing that incrementally is helpful in the sense that patches are easier to review and bisect if there are issues. Just saying that, if it were, I wouldn't be super-thrilled with this as an end-state.

Comment 4

[Dão Gottwald [:dao]]

(In reply to :Gijs from comment #3)
> Comment on attachment 8957876 [details]
> Bug 1444694 - Lazify mCurrentTab and mCurrentBrowser.
> 
> https://reviewboard.mozilla.org/r/226844/#review232610
> 
> Tentative r=me. I mean, right now, nothing much changes (if nothing else
> touches the properties any earlier) because the lazy getters both get
> invoked on the line immediately after the one where their code was removed (
> ie the `new TabProgressListener` call). But this is a bit odd, at least,
> because only-loosely-related things now depend on being accessed through
> this getter (e.g. whether the initial tab is `fullyOpen` and has a `_tPos`
> property).
> 
> Still r=me because nothing changes and I can see that we will want to
> simplify this further, and doing that incrementally is helpful in the sense
> that patches are easier to review and bisect if there are issues. Just
> saying that, if it were, I wouldn't be super-thrilled with this as an
> end-state.

Okay, I can set up the tab and browser more explicitly.

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 8957876 [details]
Bug 1444694 - Set up the initial browser and tab in a dedicated method.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226844/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dce4c11dc650
Set up the initial browser and tab in a dedicated method. r=Gijs

Comment 7

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/dce4c11dc650