836565 - Reflow-on-zoom has lag after zooming-in, then zooming-out

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Readability, defect)

Description

[Scott Johnson (:jwir3)]

When reflow-on-zoom is enabled, and the user pinch-zooms to zoom into a piece of text (see testcase url), a reflow happens as expected (and, arguably, within the performance guidelines expected by the user), but when the user then pinches to zoom out, the reflow/repaint combination requires significantly more time to complete. The zoom-out occurs within the timeframe expected (less than a second), but the text is still shown with the smaller line box width, as well as only in the smaller region that we just zoomed out from. The rest of the page is white.

There are a couple of possibilities as to what is happening here. One could be that we're seeing a problem where the tiles in the tile cache are stale, but they are being displayed nonetheless. If this is the issue, we might be able to force a reconstruction of the tile cache.

On the other hand, it could be the case that the event sequence from reflow-on-zoom is different than that of reflow-on-zoom-out, causing a different number of reflows to be triggered.

Comment 2

[Scott Johnson (:jwir3)]

Hm, so I just noticed that this issue isn't particularly confined to just reflow-on-zoom. Even with reflow-on-zoom disabled, this issue is appearing for me. That is to say, the exact same steps to reproduce, but with "Pinch to reflow text" disabled under settings.

Chris, is this a known issue? Perhaps I'm just experiencing a situation where it's more obvious that it takes a long time to perform the repaint, since I'm explicitly waiting for it to show up with text wrapped to the screen width.

Comment 3

[Chris Lord [:cwiiis]]

(In reply to Scott Johnson (:jwir3) from comment #2)
> Hm, so I just noticed that this issue isn't particularly confined to just
> reflow-on-zoom. Even with reflow-on-zoom disabled, this issue is appearing
> for me. That is to say, the exact same steps to reproduce, but with "Pinch
> to reflow text" disabled under settings.
> 
> Chris, is this a known issue? Perhaps I'm just experiencing a situation
> where it's more obvious that it takes a long time to perform the repaint,
> since I'm explicitly waiting for it to show up with text wrapped to the
> screen width.

Well, it's a known issue that redrawing the page takes an amount of time :)

An issue here is that when the zoom level changes, we redraw the entire display-port in a single, un-cancellable transaction - this may cause a bigger delay than is necessary.

Instead, we could redraw the visible area and progressively update the area outside of that afterwards, which ought to provide a faster initial update (assuming that update time is almost entirely drawing and the page is reasonably uniform, you're looking at a time reduction of ~75%). Note the comment 'Only draw progressively when the resolution is unchanged.' in BasicTiledThebesLayer.

This ought to be quite a simple patch for me to write, so if you don't mind hanging on, I'll see if I can get to this by the end of the day. If it isn't as simple as I suspect, I'll let you know. The thing I worry about is introducing coherency errors when zooming, but we'll need to test to see if that's really an issue or not.

I'd suggest that you also try to get the reflow to happen earlier, so that it can be coupled with the zoomed out redraw. This would also massively reduce the perceived delay. This will be a benefit regardless of the above suggested change.

Comment 4

[Chris Lord [:cwiiis]]

Attachment: Progressive zoom update

I'm adding the review flag, as I think this is probably something we want to do regardless of this bug and a quick test shows it working (will update with talos numbers later).

Scott, I realise this alone isn't going to help with your bug though - if the reflow is happening separately, you need to augment progressiveUpdateCallback to cancel draws when there's a pending reflow.

You also probably want to set the danger flag after a zoom change to make sure that the low precision buffer is updated, or zooming out may yield momentarily visible, incorrect-looking content.

Comment 5

[Benoit Girard (:BenWa)]

Wont the old res buffer disappear after we've painted the first tile progressively? I though this was the reason with this check?

Comment 6

[Benoit Girard (:BenWa)]

Comment on attachment 711762 [details] [diff] [review]
Progressive zoom update

Taking of my review queue until I have clarification needed to review this.

Comment 7

[Jet Villegas (inactive)]

I recommend a new bug be filed to capture what patch 711762 is actually doing, which is to obliterate any cached painting when the zoom factor changes. As Chris says, it's something we want independent of reflow-on-zoom.

There are still a number of questions in this bug that are unanswered. I'd like to see data about why reflow-on-zoom is so slow (ie. a real perf profile) to test Scott's hypotheses:

(In reply to Scott Johnson (:jwir3) from comment #0)

> There are a couple of possibilities as to what is happening here. One could
> be that we're seeing a problem where the tiles in the tile cache are stale,
> but they are being displayed nonetheless. If this is the issue, we might be
> able to force a reconstruction of the tile cache.
> 
> On the other hand, it could be the case that the event sequence from
> reflow-on-zoom is different than that of reflow-on-zoom-out, causing a
> different number of reflows to be triggered.

Comment 8

[Scott Johnson (:jwir3)]

(In reply to Jet Villegas (:jet) from comment #7)
> I recommend a new bug be filed to capture what patch 711762 is actually
> doing, which is to obliterate any cached painting when the zoom factor
> changes. As Chris says, it's something we want independent of reflow-on-zoom.

I've filed bug 850227 to track this, added the patch to this bug, and re-requested review.

> There are still a number of questions in this bug that are unanswered. I'd
> like to see data about why reflow-on-zoom is so slow (ie. a real perf
> profile) to test Scott's hypotheses:

I'm working on this right now.

Comment 9

[Scott Johnson (:jwir3)]

I now have some profiles to discuss. I used the following pages as test cases:

1) http://people.mozilla.org/~sjohnson/junkyard/lesmis.html - This page tests a large amount of text in a single element (this is more of an edge test case)
2) http://www.nytimes.com/2013/03/14/world/europe/cardinals-elect-new-pope.html?_r=1& (This is probably a more realistic test case).

Profile links:

"Le Mis" Page (1):
- Reflow on zoom enabled, single zoom in, followed by single zoom out: http://people.mozilla.com/~bgirard/cleopatra/#report=f2d0603b0ad01ac42ffe509a4600457f7f140748
- Reflow on zoom enabled, multiple zoom in/zoom out operations:
http://people.mozilla.com/~bgirard/cleopatra/#report=51aa2af9ffd9af10d9e43a08364d1f6188e2d8b4

"New York Times" Page (2):
- Reflow on zoom enabled, single zoom in, followed by a single zoom out:
http://people.mozilla.com/~bgirard/cleopatra/#report=663082fd2151077df2f14cef9dd3dfcfb4ea2b24
- Reflow on zoom enabled, multiple zoom in/out operations:
http://people.mozilla.com/~bgirard/cleopatra/#report=3031b38c6f184cf477a316981d2148433ecb321d

Observations:
- In case 1) of the NyTimes site, we spend about 33% of out time in DoReflow(), which amounts to about 2.5s. In the same run, we spend about 10% of our time in PresShell::Paint() (~0.7s).
- In case 2) of the NyTimes site, we spend about 34% of our time in DoReflow(), which amounts to about 6.7s, while we spend around 15% of our time in PresShell::Paint() (~2.9s).
- In case 1) of the Le Mis site, we spend about 46% of our time (~4s) in layout::DoReflow(), whereas we only spend about 2.1% of our time in PresShell::Paint().
- In case 2) of the Le Mis site, we spend about 49% of our time (~7.8s) in DoReflow(), and about 5.5% of our time (~0.8s) in PresShell::Paint.

One thing that's strange is that I have notes indicating that one of the runs of the LeMis site showed ~27.7% of time spent in nsDisplayText::Paint() (through PresShell::Paint()), but that doesn't seem to be reflected in the profiles I posted. It could be the case that this was in one of my preliminary profiles, which were taken prior to a couple of changes that made sure that pinch-zoom operations and reflow-zoom requests were 1:1, so it could be that we were over-painting/over-reflowing because we might perform the operation more than once per pinch-zoom gesture (which is bad). It didn't seem unreasonable, though, that, given the discrepancy between the amount of text on the page in the LeMis case, that we would be spending a much larger amount of time painting that text (especially in a case where we're zooming out, and thus have to repaint the entire screen, or at least most of it).

In the NyTimes cases, because the percentage of time spent between PresShell::Paint() and DoReflow() is fairly consistent between the two test cases, I feel confident that these are the areas that we should focus on improving in order to improve performance of this feature.

I'm going to collect a bit more data on one other site, just to be sure that the results aren't being skewed by something on the site I chose, and then I will post a strategy for possibly fixing this. (Or, at least things that we could try).

Comment 10

[Scott Johnson (:jwir3)]

I profiled the behavior on an additional page: http://www.wired.co.uk/news/archive/2013-03/14/bill-gates-capitalism#.UUGe-5TS1fE.reddit

- Profile of page with reflow-on-zoom enabled, when we zoom in and out, once:
http://people.mozilla.com/~bgirard/cleopatra/#report=c9e037b25518174069c28170c9d11645e456d73b
- Profile of page with reflow-on-zoom enabled, when we zoom in and out repeatedly: http://people.mozilla.com/~bgirard/cleopatra/#report=c9e037b25518174069c28170c9d11645e456d73b

Observations:
In the case where we zoom in and out once, we're spending about 26.4% (3s) of our time in DoReflow(), and about 22.3% (~2.5s) in PresShell::Paint().

In the case where we zoom in and out repeatedly, with reflow-on-zoom enabled, we spend about 26.8% (~3.1s) of our time in DoReflow(), and about 22.3% (~2s) in PresShell::Paint(). 

In both cases, the painting seems to be spread out among all the frames (i.e. we're not spending a large amount of time just painting text).

Given these results, any profiles I take in the future are going to be focused on the zoom in/out repeatedly case, since the profiles seem to be fairly consistent, and I feel like this case gives more reliable results (it minimizes the case where we might zoom in/out once and get an outlier condition, since the sample size is greater in this case).

Comment 11

[Scott Johnson (:jwir3)]

Attachment: b836565-part1 (WIP)

Part one of the patch series to improve performance of reflow-on-zoom. This patch makes the number of calls to the reflow-on-zoom backend (which actually triggers the reflow operation) match the number of calls to onPinchFinish().

Comment 12

[Scott Johnson (:jwir3)]

Attachment: b836565-part2 (WIP)

This is the second part of the patch, but it's not working the way I want it to right now. The idea is that I want to just enable the horizontal resize bits on text frames within the frame tree. I added a new frame state bit, NS_FRAME_TEXT_IS_DIRTY (probably it should be renamed) that is added to every frame in the tree when a reflow-on-zoom operation is performed. My intent was to then add the appropriate reflow state bits (HResize) to the frames, but I haven't yet been able to determine a way to get the appropriate text frames to enter the reflow process without marking everything dirty (and thus reflowing everything).

This is definitely still a work in progress, but I thought I would post an idea of what I have so far, in case anyone has ideas that jump out at them that might be useful.

Comment 13

[Scott Johnson (:jwir3)]

Attachment: b836565-part1

Performance improvement for reflow-on-zoom. Part 1/2. This patch does two things - first, it doesn't trigger a reflow on max line box width change, unless the max line box width is actually changing. Second, it forces reflow-on-zoom to trigger a max line box width change only ONCE for each pinch gesture.

Comment 14

[Scott Johnson (:jwir3)]

Attachment: b836565-part2

Patch to improve performance of reflow-on-zoom. Part 2/2. This patch disables reflow-on-zoom crawling through child document viewers when changing the max line box width. 

Performance improvements of this patch:

Without patch: 5.4s-7s spent in layout::DoReflow(), 10s-10.5s spent in layout::DoReflow() and PresShell::Paint().

With patch: 0.8s-1.0s spent in layout::DoReflow(), and 6.5-7s spent in PresShell::Paint().

Subjectively, though, the improvement is much more significant. It cuts the time required to perform reflow-on-zoom down by about 2/3. Where it took ~3s to perform a reflow-on-zoom operation previously, it now takes (typically) less than a second.

Comment 15

[Scott Johnson (:jwir3)]

Profiles for performance improvements:

With both patches applied:
http://people.mozilla.com/~bgirard/cleopatra/#report=ea0298328b60e972a513d7735efb39ee1f8c8027

Without either patch applied (but with patches for bug 803719 applied):
http://people.mozilla.com/~bgirard/cleopatra/#report=59b8595b22f96bbef26c817ae4d6c4bf124c178a

Comment 16

[Scott Johnson (:jwir3)]

Both of the above profiles were created by zooming in to the same page (at generally the same location) and zooming back out a total of 3 times (6 reflows in all).

Comment 17

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 729147 [details] [diff] [review]
b836565-part1

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

LGTM

Comment 18

[Scott Johnson (:jwir3)]

Attachment: log file for output from patch part 2

For part2, it looks like many news sites (cnn.com, nytimes.com) have a LOT Of subdocuments. Reflowing all of these causes the reflow time to more than triple. Unfortunately, though, just reflowing the primary document probably doesn't help us that much, since it's not correct. 

I'm wondering if perhaps we could do the reflow in stages. Since the primary document is probably the most important, we could reflow this first, as quickly as possible. Once the reflow and paint for the primary document is complete, we could then schedule a reflow for the remaining sub-documents. This might get us better response times, while at the same time verifying that we don't cause the sub documents to not recognize the max line box width change.

I've attached a log of output from my debugging sessions, both with and without the patch.

Comment 19

[Scott Johnson (:jwir3)]

Status update:

dbaron suggested that I experiment with setting the HResize Reflow flag, and then not marking frames dirty immediately, using the following process:
1) Change the call in SetMaxLineBoxWidth to set frames to NS_FRAME_HAS_DIRTY_CHILDREN, instead of NS_FRAME_IS_DIRTY.
2) (For experimental purposes only) Set mFlags.mHResize to true unconditionally in the nsReflowState constructors.

Unfortunately, this didn't improve reflow times for sites. I'm preparing a table of data to show how much time is taken during reflow for given situations, so that I can better show why this didn't work. 

I did find that instead of calling FrameNeedsReflow in SetMaxLineBoxWidth, if we call RecreateFramesFor(), this drastically improves the time required for the Le Mis webpage. (It doesn't seem to help in the general case, though, with sites like CNN.com or NyTimes)

This leads me to a working hypothesis - I think that, for sites that have a frame tree that is shallow, but large in breadth, we spend a lot of time re-joining inline frames when we zoom out. Reconstructing the frames from the content tree alleviates this problem because we don't have to spend the same amount of time doing this re-joining. 

Unfortunately, I doubt that many sites are this shallow, so I'm not sure it would help us in the general case.

I am preparing a chart of current solutions, and what times we have in reflow for each, so that I can consider what a hybrid solution might look like that would help optimize for a number of different cases simultaneously.

Comment 20

[Scott Johnson (:jwir3)]

Part 1 inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3374dda8138b

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3374dda8138b

Comment 22

[Scott Johnson (:jwir3)]

Re-opening, because there are likely other fixes we want to apply within the context of this bug.

Comment 23

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 729147 [details] [diff] [review]
b836565-part1

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

::: mobile/android/chrome/content/browser.js
@@ +2786,5 @@
>      this.setScrollClampingSize(aViewport.zoom);
>  
>      // Adjust the max line box width to be no more than the viewport width, but
>      // only if the reflow-on-zoom preference is enabled.
> +    let isZooming = Math.abs(aViewport.zoom - this._zoom) >= 1e-6;

If you're going to be touching this code again, note that bug 861205 added a fuzzyEquals method that you can use instead of doing the Math.abs >= 1e-6 yourself.

Comment 24

[Scott Johnson (:jwir3)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #23)
> If you're going to be touching this code again, note that bug 861205 added a
> fuzzyEquals method that you can use instead of doing the Math.abs >= 1e-6
> yourself.

Ah, cool. Thanks.

Comment 25

[Scott Johnson (:jwir3)]

I've taken two additional profiles, to try and determine what is happening differently between when we call ReconstructFrames vs. FrameNeedsReflow after a reflow-on-zoom event. In the most obvious case where the former is an improvement, the Le Miserables test, the profile for the ReconstructFrames() version is here:

http://people.mozilla.com/~bgirard/cleopatra/#report=7ae2876f54cffe1d9fe38e0fb365dbf25620f02e

And the profile for the FrameNeedsReflow() version is here:
http://people.mozilla.com/~bgirard/cleopatra/#report=74f342f7e06ad459a53f652da41e2ff259c50863

From these data, it looks like we spend much more time in layout::DoReflow() (47% vs. 0.6%), and, it also looks like we're spending a large portion of that time in FindFontForChar(), as well as in methods that seem to be splitting or joining text (e.g. nsLineBreaker::AppendText and gfxFont::SplitAndInitTextRun)

One thing I noticed that's odd, though, is that in the case where we're spending significantly more time to reflow-on-zoom the page, we're spending a lot of time in an area where the profiler can't seem to see (there's a line indicating a large amount of time, around 39.6% that's spent in libc.so), or in layout::DoReflow(), but not in any of the function calls inside of DoReflow(). 

This seems to indicate to me that we're a) spending a lot of time doing some kind of system call when we set the max line box width and b) doing a set of arduous computations inside of the DoReflow() function itself that aren't absolutely necessary (or, alternatively, calling DoReflow() an exorbitant amount of times, perhaps).

Comment 26

[Scott Johnson (:jwir3)]

Attachment: time spent report

Here's the final report I have for the different patches, and how they affect performance. Each test consisted of testing either zoom-out, or zoom-in, and measured how long it took from a single reflow-on-zoom event to the subsequent paint. Each test was measured 11 times (the tables below the first one). I was careful to stop as many other running programs on my Samsung Galaxy S2 as possible before running the tests, so as to get a better read of what Fennec was actually doing.

Based on these data, I think the HResize solution might actually be a viable solution. Originally, I thought this was not the case, as it didn't exhibit a change, but after getting a more solid timing estimation of how long the actual reflow-on-zoom was taking, I believe that the more accurate results indicate this is a good direction in which to progress.

Further, the reframe solution shows almost no change (I consider anything less than ~3% in either the plus or minus to be no significant change) EXCEPT in the case of the "Les Mis" page. This page has a very shallow, but very broad, frame tree. It might be worth seeing if we can do this same type of solution for pages that exhibit this type of frame tree. We'd have to get some type of measurement on the breadth:depth ratio of a tree, but this could work to the benefit of, for example, text files that are viewed on mobile.

The idea of disabling the reflow of child document viewers does also show performance wins across the board, BUT this probably should be taken with a grain of salt. The test shows a perf win when we disable the reflow of child document viewers, but in reality, what we would want to do is POSTPONE this reflow for some number of ms. That is, the test doesn't show that we're actually winning initially, but then we'll likely have to take a perf hit when we perform the reflow (which actually will likely be slightly worse, because we've already painted once, and now we're going to force Fennec to paint again). Additionally, the solution adds a lot of complexity to the browser, so I suggest we hold off on this one until we are sure it's what we want to do.

Finally, the chart shows a performance loss on the "Reuters" site for the solution to bug 836568, however, the time it takes for the Reuters site to reflow is so small already, that I think this performance hit is worthwhile to take in order to get the performance benefits on the NYTimes site. This might be debatable, though.

So, here's what I propose:

1) Land the patch for bug 836568 as soon as possible, getting these changes into effect,
2) Implement a better solution for the "Hresize" case - i.e. don't just set HResize to true in the nsHTMLReflowState constructor, but actually try to be intelligent about it).
3) Re-measure performance with these two cases to determine improvements.
4) If necessary, add the child document viewer timeout (second patch attached to this bug) and see if performance is improved.
5) Look into developing an heuristic to see if we can perform a reframe operation instead of a reflow operation on frame trees that have a high breadth/depth ratio.

Comment 27

[Scott Johnson (:jwir3)]

Attachment: b836565-hresize

Here's the patch to implement the hresize component of this, as we discussed on Tuesday, David. I have verified that there is a performance increase, but I'm just looking for feedback if this is the correct approach, or whether I need to put the code to trigger hresize in other structures besides just nsHTMLReflowState.

Comment 28

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 741996 [details] [diff] [review]
b836565-hresize

>diff --git a/layout/base/nsIPresShell.h b/layout/base/nsIPresShell.h
>--- a/layout/base/nsIPresShell.h
>+++ b/layout/base/nsIPresShell.h
>@@ -1397,16 +1397,36 @@ public:
>   virtual void BackingScaleFactorChanged() = 0;
> 
>   nscoord MaxLineBoxWidth() {
>     return mMaxLineBoxWidth;
>   }
> 
>   void SetMaxLineBoxWidth(nscoord aMaxLineBoxWidth);
> 
>+  /**
>+   * Returns whether or not there is a reflow on zoom event pending. A reflow
>+   * on zoom event is a change to the max line box width, followed by a reflow.
>+   * This subsequent reflow event should trigger a horizontal resize reflow,

trigger a horizontal resize reflow
 -> treat all frames as though they resized horizontally (and thus reflow all their descendants)

>+   * rather than marking all frames dirty from the root. This is the way the
>+   * pres shell indicates that an hresize reflow should take place during reflow
>+   * state construction.
>+   */


>diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp
>--- a/layout/base/nsPresShell.cpp
>+++ b/layout/base/nsPresShell.cpp
>@@ -7871,16 +7871,18 @@ PresShell::DoReflow(nsIFrame* target, bo
> 
>     // Any FlushPendingNotifications with interruptible reflows
>     // should be suppressed now. We don't want to do extra reflow work
>     // before our reflow event happens.
>     mSuppressInterruptibleReflows = true;
>     MaybeScheduleReflow();
>   }
> 
>+  ClearReflowOnZoomPending();
>+
>   return !interrupted;
> }

This is in the wrong place, since |target| might not be the root frame.  I think the best fix is to put it is DidDoReflow, and pass |interrupted| from ProcessReflowCommands to DidDoReflow (ResizeReflowIgnoreOverride would just pass false for interrupted), and then only call ClearReflowOnZoomPending if interrupted (as distinguished from aInterruptible, which says whether we're allowed to interrupt rather than whether we actually did!) is true.


r=dbaron with that fixed

Comment 29

[Scott Johnson (:jwir3)]

Hi David:

I've made the changes you requested, but I have a question on the latter one that I just want to clarify before I land the change:

(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #28)
> This is in the wrong place, since |target| might not be the root frame.  I
> think the best fix is to put it is DidDoReflow, and pass |interrupted| from
> ProcessReflowCommands to DidDoReflow (ResizeReflowIgnoreOverride would just
> pass false for interrupted), and then only call ClearReflowOnZoomPending if
> interrupted (as distinguished from aInterruptible, which says whether we're
> allowed to interrupt rather than whether we actually did!) is true.

So, it seems to me that inside of ProcessReflowCommands, |interrupted| will be true iff DoReflow returns false. This can happen either on an error condition (e.g. we failed to get a rendering context) or the pres context had an interrupt.

I'm still not entirely clear on when an interrupt happens in the pres context, so this could be part of the problem, but my understanding is that this is a fairly rare event. Will a horizontal resize automatically trigger an interrrupt? If not, I think we might be in a situation where we're leaving the |mReflowOnZoomPending| true when it should have been reset to false. 

Basically, I'm not clear on why we want to clear the reflow on zoom flag only if we were interrupted when inside of the call to DidDoReflow().

Comment 30

[Scott Johnson (:jwir3)]

Also, note that I split off bug 870788 as a separate task (the sub document issues we were discussing previously), so that we can track discrete issues, once the hresize part of this bug is closed.

Comment 31

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to Scott Johnson (:jwir3) from comment #29)
> Hi David:
> 
> I've made the changes you requested, but I have a question on the latter one
> that I just want to clarify before I land the change:
> 
> (In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from
> comment #28)
> > This is in the wrong place, since |target| might not be the root frame.  I
> > think the best fix is to put it is DidDoReflow, and pass |interrupted| from
> > ProcessReflowCommands to DidDoReflow (ResizeReflowIgnoreOverride would just
> > pass false for interrupted), and then only call ClearReflowOnZoomPending if
> > interrupted (as distinguished from aInterruptible, which says whether we're
> > allowed to interrupt rather than whether we actually did!) is true.
> 
> So, it seems to me that inside of ProcessReflowCommands, |interrupted| will
> be true iff DoReflow returns false. This can happen either on an error
> condition (e.g. we failed to get a rendering context) or the pres context
> had an interrupt.
> 
> I'm still not entirely clear on when an interrupt happens in the pres
> context, so this could be part of the problem, but my understanding is that
> this is a fairly rare event. Will a horizontal resize automatically trigger
> an interrrupt? If not, I think we might be in a situation where we're
> leaving the |mReflowOnZoomPending| true when it should have been reset to
> false. 
> 
> Basically, I'm not clear on why we want to clear the reflow on zoom flag
> only if we were interrupted when inside of the call to DidDoReflow().

No, we want to clear it only if we were *not* interrupted.  (And it is a fairly rare event; it means we didn't run the reflow through to completion.  So if we're interrupted, we'll still need the flag set the next time until the reflow is completed.)

Comment 32

[Scott Johnson (:jwir3)]

Inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aaa5b12b585e

Comment 33

[Scott Johnson (:jwir3)]

Try push for latest patch:
https://hg.mozilla.org/try/rev/9d9e68f74402

NOTE: The oranges in that patch are from the previous parent push, the one for bug 847872.

Comment 34

[Ryan VanderMeulen [:RyanVM]]

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