Closed Bug 731619 Opened 9 years ago Closed 9 years ago

[maple] Drawing of content stops completely after rotation

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: kats, Assigned: kats)

Details

(Keywords: regression, Whiteboard: maple)

Attachments

(2 files)

After a rotation, beginDrawing always returns false, and content never gets drawn. The compositor still works so you can scroll around what has already been drawn but if you scroll out of that all you get are checkerboards. My initial investigation shows that this appears to be because the ViewportHandler isn't processing resize events as expected, and this bug was previously masked because we were doing a bajillion draw events all over the place. Patch forthcoming.
Also I realized the infinite loop guard should be moved out of updateViewportSize because that gets called from somewhere else (when the meta information changes but the screen size does not change) and still needs to run in that case.
Attachment #601674 - Flags: review?(ehsan)
Comment on attachment 601674 [details] [diff] [review]
Make drawing work again

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

::: mobile/android/chrome/content/browser.js
@@ +2134,5 @@
>      this.userScrollPos.x = win.scrollX;
>      this.userScrollPos.y = win.scrollY;
>  
>      // If the browser width changes, we change the zoom proportionally. This ensures sensible
>      // behavior when rotating the device on pages with automatically-resizing viewports.

Remove this comment as well.

@@ +3148,5 @@
>        case "DOMMetaAdded":
> +        let target = aEvent.originalTarget;
> +        if (target.name != "viewport")
> +          break;
> +        let document = target.ownerDocument || target;

So by now target is a meta element, so you should remove the ` || target' part.

@@ +3153,5 @@
> +        let browser = BrowserApp.getBrowserForDocument(document);
> +        let tab = BrowserApp.getTabForBrowser(browser);
> +        if (!tab)
> +          break;
> +        this.updateMetadata(tab);

Nit:

if (tab)
  this.updateMetadata(tab);

@@ +3164,1 @@
>          BrowserApp.selectedTab.updateViewportSize();

Reverse this if condition as well.
Attachment #601674 - Flags: review?(ehsan) → review+
Whiteboard: maple
Landed with nits fixed:

https://hg.mozilla.org/projects/maple/rev/34af14be0ed9
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
There's another race condition that results in the same stopping of content drawing after rotation. LayerController.setViewportSize calls notifyLayerClientOfGeometryChange() first and mLayerClient.viewportSizeChanged() second. However, the flags set in viewportSizeChanged() could be read before they are set if gecko processes the resize event fast enough. This happened to me as seen in this log output:

02-29 15:50:43.810 D/GeckoLayerController( 2357): setViewportSize: v=RectF(0.0, 10301.339, 800.0, 11477.339) p=(800.0002,39685.66) z=0.5653711
02-29 15:50:43.810 D/GeckoLayerClient( 2357): ### sendResizeEventIfNecessary false
02-29 15:50:43.810 E/GeckoLayerClient( 2357): ### getBufferSize (800,1176)
02-29 15:50:43.810 I/GeckoLayerClient( 2357): ### Screen-size changed to (800,1280)
02-29 15:50:43.810 I/GeckoLayerClient( 2357): ### Window-size changed to (800,1176)
02-29 15:50:43.810 I/Gecko   ( 2357): AndroidGeckoEvent: 0x4082a318 : 8
02-29 15:50:43.810 I/Gecko   ( 2357): nsWindow[0x5c88c630]::Resize [0 0 800 1176] (repaint 1)
02-29 15:50:43.810 I/Gecko   ( 2357): nsWindow: 0x5c88c630 OnSizeChanged [800 1176]
02-29 15:50:43.810 I/Gecko   ( 2357): nsWindow[0x5c88cc70]::Resize [0 0 800 1176] (repaint 0)
02-29 15:50:43.810 I/Gecko   ( 2357): nsWindow: 0x5c88cc70 OnSizeChanged [800 1176]
02-29 15:50:43.810 E/Gecko   ( 2357): ### OnDraw()
02-29 15:50:43.820 E/GeckoConsole( 2357): ### JS side updateViewport true zoom 0.9045937
02-29 15:50:43.820 I/Gecko   ( 2357): void mozilla::AndroidBridge::HandleGeckoMessage(const nsAString_internal&, nsAString_internal&)
02-29 15:50:43.820 E/GeckoLayerClient( 2357): ### Java side Viewport:UpdateAndDraw()!

This call to UpdateAndDraw() sets mIgnorePaintsPendingViewportSizeChange to false. Shortly after this viewportSizeChanged() runs and sets it back to true.

02-29 15:50:43.820 I/Gecko   ( 2357): AndroidGeckoEvent: 0x408c2e20 : 7
02-29 15:50:43.820 I/Gecko   ( 2357): leaving void mozilla::AndroidBridge::HandleGeckoMessage(const nsAString_internal&, nsAString_internal&)
02-29 15:50:43.820 E/GeckoConsole( 2357): ### displayport margins=(0, 383, 480.00008549999984, 383) at zoom=0.9045937 and buffer amounts=(480.00008549999984, 766)
02-29 15:50:43.830 I/Gecko   ( 2357): AndroidGeckoEvent: 0x408960c0 : 20
02-29 15:50:43.830 E/GeckoLayerClient( 2357): ### Render requested, scheduling composite
02-29 15:50:43.830 E/Gecko   ( 2357): ### scheduleComposite()
[snip]
02-29 15:50:43.830 E/GeckoLayerClient( 2357): ### Render requested, scheduling composite
02-29 15:50:43.830 E/Gecko   ( 2357): ### scheduleComposite()
02-29 15:50:43.830 E/GeckoLayerClient( 2357): ### beginDrawing 800 1176
02-29 15:50:43.830 E/Gecko   ( 2357): ### BeginDrawing returned false!

Here the flag is still set to true, so BeginDrawing keeps returning false.

It looks like this bug has existed for a long time but never noticed because until recently we didn't have a mIgnorePaintsPendingViewportSizeChange flag and the other flag set in the method doesn't have as drastic an effect. Also it manifests more easily with the patch from bug 730966 applied because it reduces the amount of code that needs to execute for the race to be won.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch Fix the raceSplinter Review
Attachment #601732 - Flags: review?(ehsan)
Attachment #601732 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/projects/maple/rev/a1a85f27741a
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Verified fixed on:
Nightly Fennec 13.0a1 (2012-03-11)
Device: HTC Desire Z
OS: Android 2.3.3
Status: RESOLVED → VERIFIED
Please verify this on Maple, not Nightly.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Maybe i was not so clear but I've verified this on Nightly Maple
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.