Closed Bug 799401 Opened 7 years ago Closed 7 years ago

URL bar scrolling out of view while a fling is in progress causes window to flicker

Categories

(Core :: Graphics: Layers, defect, P2)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
B2G C2 (20nov-10dec)
blocking-basecamp +
Tracking Status
firefox19 --- fixed
firefox20 --- fixed
b2g18 --- fixed

People

(Reporter: cjones, Assigned: ajones)

References

Details

Attachments

(2 files, 4 obsolete files)

STR
 (1) Load nytimes.com in browser
 (2) Zoom all the way so that all content is fit in the window, leaving the URL bar in view
 (3) Give the page a good solid fling down

If the fling animation is still continuing when the URL bar moves off the screen, there's a rather severe entire-screen flash.

I'm stumbling across this in bug 795657, but it's a pre-existing condition.  Bug 798194 might make it obsolete.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 797264
blocking-basecamp: ? → +
Actually, this is better described bug. Let's use this as the real bug and I'll dup bug 797264 on this.
Status: RESOLVED → UNCONFIRMED
Ever confirmed: false
Resolution: DUPLICATE → ---
Status: UNCONFIRMED → NEW
Ever confirmed: true
Duplicate of this bug: 797264
It doesn't always cause the content to flicker, though I have seen this:

Otoro phone, build 2012-10-09 us
Taken from default.xml in b2g-distro: 
* "platform_build" revision= 0d6d050bc37d3167cc82a1885fd7660456bb0f4e
* "gaia" revision= 5f7dbe06490e6b52a356e033f52fb31c0fa0e2d0 
* "releases-mozilla-central" revision= 1f41d3603a035d64c3b1d7c16dba5fd1ce322957
* "gonk-misc" revision= bac8185ab931c9bc868e9bcb9b636620b8032e5e
Priority: -- → P2
Jet, Joe, we need to get this one assigned out.
Since Anthony's looking into bug 798194, I'll let him weigh in on this after he's investigated that one.
Assignee: nobody → ajones
Milestoning for C2 (deadline of 12/10), as this meets the criteria of "known P2 bugs found before or during C1".
Target Milestone: --- → B2G C2 (20nov-10dec)
What is the status here? No progress in 20 days for a basecamp blocker is unacceptable.
Not sure how to r(In reply to Andreas Gal :gal from comment #8)
> What is the status here? No progress in 20 days for a basecamp blocker is
> unacceptable.

I'm not sure how to respond to that.
Looks to me like the repainting isn't keeping up with the scrolling. There may also be an extra repaint occurring during the resize.
Attached patch Skip obsolete paint requests (obsolete) — Splinter Review
I had trouble reproducing a flash but this patch makes the nytimes site perform a lot better. About 2/3rds of paint requests were out of date before even starting to paint. Skipping obsolete paint requests avoids a backlog of stale requests.

Would like some feedback on class names and file locations and the general concept.
Attachment #685964 - Flags: feedback?(roc)
Comment on attachment 685964 [details] [diff] [review]
Skip obsolete paint requests

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

Looks good to me but this is more cjones' territory I think
Attachment #685964 - Flags: feedback?(roc) → feedback+
Attachment #686366 - Flags: review?(jones.chris.g)
Status: NEW → ASSIGNED
Comment on attachment 686366 [details] [diff] [review]
Bug 799401 - Throttle APZC paint requests; r=cjones

This would have been easier to review as a revert+rebase of
http://hg.mozilla.org/mozilla-central/rev/9698936945c6 optionally
followed by a refactoring patch, since the original code was well
understood.  But this is fine.

>diff --git a/gfx/layers/ipc/AsyncPanZoomController.cpp b/gfx/layers/ipc/AsyncPanZoomController.cpp

>-  mGeckoContentController->RequestContentRepaint(mFrameMetrics);
>+  mPaintThrottler.PostTask(
>+    FROM_HERE,
>+    NewRunnableMethod(mGeckoContentController.get(),
>+                      &GeckoContentController::RequestContentRepaint,
>+                      mFrameMetrics));

The task created here doesn't hold a strong reference to
mGeckoContentController, so it can create use-after-free bugs where
they couldn't exist previously.

I think this is OK because TaskThrottler is scoped to APZC, which also
holds a strong ref to mGeckoContentController, and
mGeckoContentController is effectively immutable.  But this deserves a
second, close, look.

>diff --git a/gfx/layers/ipc/TaskThrottler.cpp b/gfx/layers/ipc/TaskThrottler.cpp

This seems mostly OK, but two issues

 - It would be a lot simpler to use

  nsAutoPtr<CancelableTask> mQueuedTask;

 - please document the memory ownership model at the TaskThrottler
   boundary

Would like to see the next version.
Attachment #686366 - Flags: review?(jones.chris.g)
(In reply to Chris Jones [:cjones] [:warhammer] from comment #15)
> The task created here doesn't hold a strong reference to
> mGeckoContentController, so it can create use-after-free bugs where
> they couldn't exist previously.

RunnableMethod retains a counted reference to the callee at http://mxr.mozilla.org/mozilla-central/source/ipc/chromium/src/base/task.h#299
Attachment #687591 - Attachment description: Bug 799401 - Throttle APZC paint requests; r=cjones → Throttle APZC paint requests v2
Attachment #687591 - Flags: review?(jones.chris.g)
Attachment #686366 - Attachment is obsolete: true
Attachment #687591 - Flags: review?(jones.chris.g) → review+
Attachment #685964 - Attachment is obsolete: true
Try run for b8b54946d90c is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=b8b54946d90c
Results (out of 15 total builds):
    success: 14
    warnings: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-b8b54946d90c
Attachment #689038 - Attachment description: Bug 799401 - Fix flash of blur on viewport change; r=cjones → Fix flash of blur on viewport change
Attachment #689038 - Flags: review?(jones.chris.g)
Comment on attachment 689038 [details] [diff] [review]
Fix flash of blur on viewport change

Did you attach this to the bug you meant to?
Attachment #689038 - Flags: review?(jones.chris.g) → review+
(In reply to Chris Jones [:cjones] [:warhammer] from comment #22)
> Did you attach this to the bug you meant to?

Yes.

I can reproduce a full screen flash here http://people.mozilla.com/~nhirata/html_tp by pulling the screen up until the URL bar disappears then flicking down. When the URL bar reappears you get a flash which turns out to be a single frame that is rendered at the wrong zoom.

Throttling of paint requests helps reduce scrolling to unpainted regions (i.e. white screen) which was as close to reproducing the bug as I could get at nytimes.com. I managed to properly reproduce the issue while looking at bug 804808.
Try run for 03df1b061cc9 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=03df1b061cc9
Results (out of 97 total builds):
    success: 84
    warnings: 13
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-03df1b061cc9
Removed broken MOZ_ASSERT.
Attachment #687591 - Attachment is obsolete: true
Fixed build break on Windows
Attachment #689038 - Attachment is obsolete: true
Try run for 03df1b061cc9 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=03df1b061cc9
Results (out of 98 total builds):
    success: 85
    warnings: 13
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-03df1b061cc9
https://hg.mozilla.org/mozilla-central/rev/8b1d3367ca5d
https://hg.mozilla.org/mozilla-central/rev/195b2469641e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Whiteboard: [status-b2g18:fixed]
Try run for 03df1b061cc9 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=03df1b061cc9
Results (out of 99 total builds):
    exception: 1
    success: 85
    warnings: 13
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-03df1b061cc9
Try run for b8b54946d90c is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=b8b54946d90c
Results (out of 16 total builds):
    success: 14
    warnings: 1
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-b8b54946d90c
Try run for 84d1137b8573 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=84d1137b8573
Results (out of 33 total builds):
    success: 19
    warnings: 9
    failure: 5
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-84d1137b8573
Depends on: 825809
You need to log in before you can comment on or make changes to this bug.