577607 - Make subframes use their parent frame's refresh driver

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

Subframes (and resource documents) in content should just use the refresh driver of the root content document (which is the same thing as the root document in e10s-land).

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Part 1. Tear down external resource presshells when the main document's presshell goes away

We need this so that the external resource docs won't keep holding on to the main doc refresh driver after the main doc presshell goes away.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Part 2. Use the display doc or parent doc display drivers as needed

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Part 2 with a tweak to Thaw/Freeze

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 456538 [details] [diff] [review]
Part 2 with a tweak to Thaw/Freeze

No, this is wrong.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Part 2 for real

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 456542 [details] [diff] [review]
Part 2 for real

This is still failing some tests.  Need to sort that out.

Comment 7

[Boris Zbarsky [:bzbarsky]]

And the reason it's wrong is that the refresh driver only flushes the prescontext it's initialized with.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Part 2. Make it possible for refresh driver to flush more than one presshell

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Part 3. Make subframes use the parent's refresh driver

Comment 10

[Boris Zbarsky [:bzbarsky]]

The patches in bug 569520 depend on this bug at the moment.  It could be detangled, with some pain...

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 456666 [details] [diff] [review]
Part 2.  Make it possible for refresh driver to flush more than one presshell

+  PRPackedBool      mReflowScheduled; // If true, we have a reflow scheduled.
+                                      // Guaranteed to be false if
+                                      // mReflowContinueTimer is non-null.
+
+  PRPackedBool      mSuppressInterruptibleReflows;

Fix indent

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 456667 [details] [diff] [review]
Part 3.  Make subframes use the parent's refresh driver

+        ourItem->GetSameTypeRootTreeItem(getter_AddRefs(root));
+        if (root != ourItem) {
+          mRefreshDriver = parent->GetShell()->GetPresContext()->RefreshDriver();

Couldn't you just use GetSameTypeParent here and check for null?

I think we really need an HasSameTypeParent method on nsIDocument for situations like this...

Comment 13

[Boris Zbarsky [:bzbarsky]]

> Fix indent

Done.

> Couldn't you just use GetSameTypeParent here and check for null?

Yes.  Done.

> I think we really need an HasSameTypeParent method on nsIDocument for
> situations like this...

Or perhaps IsRootOfType or something... Bug 586178 filed.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: Roll-up patch for approval. DO NOT CHECK IN

Risk:  Moderate.  I'm pretty sure things should work fine, and I've done a bunch of testing on sites with subframes, and our tests pass, but there's a bunch of rejiggering of how we trigger reflow and restyling in here.  Should be high-quality rejiggering, of course!

Benefit: lays the groundwork for some performance-related improvements we want to make.  Should somewhat improve performance on pages with subframes, and should definitely reduce the number of wakeups on such pages (which is good for mobile).

Comment 15

[Joe Drew (not getting mail)]

Comment on attachment 464706 [details] [diff] [review]
Roll-up patch for approval.  DO NOT CHECK IN

Approved with a short leash.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Part 1 got pushed as http://hg.mozilla.org/mozilla-central/rev/d07f35b7a38f

The others got pushed as well, but then backed out for now.

Comment 17

[Chris Pearce [:cpearce (Not reading bugmail)]]

Part 2: http://hg.mozilla.org/mozilla-central/rev/7d133a2a7d20
Part 3: http://hg.mozilla.org/mozilla-central/rev/4e326cd7bac6