bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

SyncFrameMetrics override is disconnected and breaks compiles on gcc 4.7+

RESOLVED FIXED in mozilla23

Status

()

Core
Graphics: Layers
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: kats, Assigned: kats)

Tracking

23 Branch
mozilla23
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

In https://hg.mozilla.org/mozilla-central/rev/4b2b6c39adf9 (bug 839641) I added a SyncFrameMetrics method CompositorParent which was overridden in the AndroidCompositorParent subclass in Fennec. The AndroidCompositorParent subclass was introduced in bug 860613 a couple of weeks ago.

The refactoring in bug 861727 moved some of the stuff in CompositorParent out into a AsyncCompositionManager class, including the SyncFrameMetrics method. This means the override in AndroidCompositorParent is now on the wrong class, and therefore that code path is broken. The code path is disabled by default so it will not manifest anywhere.

However, the MOZ_OVERRIDE tag on the function causes builds with gcc 4.7 and up to fail.
Blocks: 776030
Glad to see MOZ_OVERRIDE catching real bugs :)
Created attachment 743837 [details] [diff] [review]
Patch

Still building this locally. It makes SyncFrameMetrics a non-virtual function, similar to how SetFirstPaintViewport, SetPageRect, and SyncViewportInfo work.
Assignee: nobody → bugmail.mozilla
Created attachment 743891 [details] [diff] [review]
Better patch
Attachment #743837 - Attachment is obsolete: true

Comment 4

5 years ago
Sorry about breaking your patch and thanks for the fix - bug 861727 turned into rebase hell, I presume I missed this in one of the rebases.
Attachment #743891 - Flags: review?(ncameron)

Comment 5

5 years ago
Comment on attachment 743891 [details] [diff] [review]
Better patch

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

lgtm
Attachment #743891 - Flags: review?(ncameron) → review+
I typo'd the ifdef check and didn't qref the fix for it. Oops. Landed as a trivial follow-up:

https://hg.mozilla.org/integration/mozilla-inbound/rev/9a0a3cb01e5c
https://hg.mozilla.org/mozilla-central/rev/ea568a1576e5
https://hg.mozilla.org/mozilla-central/rev/9a0a3cb01e5c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.