Closed Bug 979034 Opened 6 years ago Closed 6 years ago

Make CustomizationTabPreloader kick-off when the user opens the menu panel.

Categories

(Firefox :: Tabbed Browser, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 30
Tracking Status
firefox29 --- fixed
firefox30 --- fixed

People

(Reporter: mconley, Assigned: mconley)

References

(Depends on 1 open bug)

Details

(Whiteboard: [Australis:P2])

Attachments

(1 file, 2 obsolete files)

CustomizationTabPreloader currently initializes 7s after the first paint.

This adds unnecessary overhead to all users for what is likely an uncommon activity (entering customization mode).

The idea is to kick off the preloader after the menu panel has been first shown (or once we've finished entering customization mode). In the panel case, we hope that the preloading will have taken effect by the time the user hits the "Customize" button. This will, of course, not benefit users who enter customize mode from the OS X global menu bar or the toolbar context menu.
Assignee: nobody → mconley
Attached patch Patch v1.1 (obsolete) — Splinter Review
Attachment #8385496 - Attachment is obsolete: true
Comment on attachment 8385498 [details] [diff] [review]
Patch v1.1

What do you think of this, Tim?
Attachment #8385498 - Flags: review?(ttaubert)
Comment on attachment 8385498 [details] [diff] [review]
Patch v1.1

Review of attachment 8385498 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/modules/CustomizationTabPreloader.jsm
@@ +19,5 @@
>  const CUSTOMIZATION_URL = "about:customizing";
>  
>  // The interval between swapping in a preload docShell and kicking off the
>  // next preload in the background.
>  const PRELOADER_INTERVAL_MS = 600;

It just occurred to me that this interval should be changed according to how long the customization transition takes. If 600ms works just fine we can also leave it as is.

@@ +24,2 @@
>  
>  const TOPIC_TIMER_CALLBACK = "timer-callback";

This can be removed as well.
Attachment #8385498 - Flags: review?(ttaubert) → review+
(In reply to Tim Taubert [:ttaubert] from comment #4)
> Comment on attachment 8385498 [details] [diff] [review]
> Patch v1.1
> 
> Review of attachment 8385498 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/modules/CustomizationTabPreloader.jsm
> @@ +19,5 @@
> >  const CUSTOMIZATION_URL = "about:customizing";
> >  
> >  // The interval between swapping in a preload docShell and kicking off the
> >  // next preload in the background.
> >  const PRELOADER_INTERVAL_MS = 600;
> 
> It just occurred to me that this interval should be changed according to how
> long the customization transition takes. If 600ms works just fine we can
> also leave it as is.
> 

The transition is supposed to take approximately 200ms now, plus time to wrap toolbar items, etc. I think 600ms is a fine interval for now.

> @@ +24,2 @@
> >  
> >  const TOPIC_TIMER_CALLBACK = "timer-callback";
> 
> This can be removed as well.

Ah, good call. Thanks.
Thanks!
Attachment #8385498 - Attachment is obsolete: true
Attachment #8385535 - Flags: review+
remote:   https://hg.mozilla.org/integration/fx-team/rev/3eb813b8fed8
Whiteboard: [Australis:P2] → [Australis:P2][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/3eb813b8fed8
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P2][fixed-in-fx-team] → [Australis:P2]
Target Milestone: --- → Firefox 30
Comment on attachment 8385535 [details] [diff] [review]
Patch v1.2 (r+'d by ttaubert)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 

Bug 975552.


User impact if declined: 

Probably none - but this will reduce the start-up overhead for all users. It instead moves the preloading to immediately after the menu panel is opened.


Testing completed (on m-c, etc.): 

Plenty of local testing, and some nights on m-c.


Risk to taking this patch (and alternatives if risky): 

Very low.


String or IDL/UUID changes made by this patch:

None.
Attachment #8385535 - Flags: approval-mozilla-aurora?
Attachment #8385535 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
QA Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.