Closed Bug 1444694 Opened 5 years ago Closed 5 years ago

Set up the initial browser and tab in a dedicated method

Categories

(Firefox :: Tabbed Browser, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 60
Tracking Status
firefox60 --- fixed

People

(Reporter: dao, Assigned: dao)

References

Details

Attachments

(1 file)

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 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.
Attachment #8957876 - Flags: review?(gijskruitbosch+bugs) → review+
(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.
Summary: Lazify mCurrentTab and mCurrentBrowser → Set up the initial browser and tab in a dedicated method
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dce4c11dc650
Set up the initial browser and tab in a dedicated method. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/dce4c11dc650
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
You need to log in before you can comment on or make changes to this bug.