Closed Bug 786484 Opened 12 years ago Closed 4 years ago

presence of a collapsed browser element in browser.xul (e.g. new tab preloading <browser>) causes excessive invalidation

Categories

(Core :: Layout, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: taras.mozilla, Unassigned)

References

Details

(Whiteboard: [Snappy])

Attachments

(1 file)

This is obvious with paint flashing. Causes jankyness.
I think we should probably back out bug 753448, since the problem it tries to fix (bug 752839) is already fixed independently by bug 716108.
Blocks: 753448
Component: Toolbars → General
Summary: Moving mouse over toolbar buttons redraws whole window → Moving mouse over toolbar buttons redraws whole window with browser.newtab.preload=true
Whiteboard: [Snappy:P1] → [Snappy]
Summary: Moving mouse over toolbar buttons redraws whole window with browser.newtab.preload=true → browser.newtab.preload=true causes excessive invalidation (e.g. moving mouse over toolbar buttons redraws whole window)
(In reply to Dão Gottwald [:dao] from comment #2)
> I think we should probably back out bug 753448, since the problem it tries
> to fix (bug 752839) is already fixed independently by bug 716108.

I think they are independent improvements.
The difference between preloading enabled and disabled is pretty noticeable to me. With browser.newtab.preload set to true, the new tab page is shown instantaneously.
(In reply to Marco Castelluccio [:marco] from comment #4)
> The difference between preloading enabled and disabled is pretty noticeable
> to me. With browser.newtab.preload set to true, the new tab page is shown
> instantaneously.

Yeah, there's still some value in preloading besides the new tab animation. I was mislead by bug 753448 comment 0 referring to bug 752839.
Are we unnecessarily invalidating because of the hidden iframe?
Dao, by the way. I can no longer reproduce this on my laptop. I did have the new newtab behavior prefed on.
I still see this with browser.newtab.preload set to true.
I think this will be "worked around" by bug 780123. It doesn't fix the underlying issue which is probably some erroneous invalidation but we need bug 780123 anyway.
This also causes longer tab opening and closing animations (on my PC ~40ms slower).
I just checked with paint flashing enabled and this problem will indeed be fixed/ worked around by bug 780123. The underlying issue here is probably some bug in the layout invalidation code.
So what's the underlying issue?  What _exactly_ does this pref do?  How is the behavior with DLBI builds?
Toggling the pref inserts a collapsed <browser> element into all active browser windows. These browsers load "about:newtab" and don't do anything special besides swapping docShells when opening new tabs.

What do you mean with DLBI builds? Is there a pref I can toggle or are there any try builds somewhere?
Does making the iframe hidden instead of collapsed fix the problem?
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #16)
> Does making the iframe hidden instead of collapsed fix the problem?

That doesn't help because then the preloading browser doesn't load the document at all i.e. doesn't create docShell. (browser.docShell == null)
Yeah, I know it's not a suitable fix, but I was curious about whether it worked around the layout issue.
In any case, this looks like a layout bug.

Tim, can you attach a chrome-context scratchpad script that simulates what the new tab preloader does, so that someone more familiar with layout can easily investigate this?
Product: Firefox → Core
Component: General → Layout
Summary: browser.newtab.preload=true causes excessive invalidation (e.g. moving mouse over toolbar buttons redraws whole window) → presence of a collapsed browser element in browser.xul (e.g. new tab preloading <browser>) causes excessive invalidation
> That doesn't help because then the preloading browser doesn't load the document at all
> i.e. doesn't create docShell. (browser.docShell == null)

That's pretty odd.  This was just for a <browser style="display: none">?
(In reply to Boris Zbarsky (:bz) from comment #20)
> > That doesn't help because then the preloading browser doesn't load the document at all
> > i.e. doesn't create docShell. (browser.docShell == null)
> 
> That's pretty odd.  This was just for a <browser style="display: none">?

Sorry, I was wrong. The error thrown is this one:

Error: NS_ERROR_NOT_IMPLEMENTED: Component returned failure code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsIFrameLoaderOwner.swapFrameLoaders]
Source File: chrome://global/content/bindings/browser.xml
Line: 1155

So it looks like the document got loaded but swapFrameLoaders doesn't work for some reason?
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #18)
> Yeah, I know it's not a suitable fix, but I was curious about whether it
> worked around the layout issue.

Sorry, I could have figured that out :) It does indeed fix the issue.
I just tested again with preloading "about:blank" and we still have invalidation problems. So that's not tied to "about:newtab" itself.
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #19)
> Tim, can you attach a chrome-context scratchpad script that simulates what
> the new tab preloader does, so that someone more familiar with layout can
> easily investigate this?

Good idea, here it is. Once that script got executed, the invalidation bug is present until shutdown.
No longer blocks: 791670
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: