Closed Bug 1120203 Opened 11 years ago Closed 11 years ago

Transition more fields of FrameMetrics to use getters/setters

Categories

(Core :: Panning and Zooming, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla38

People

(Reporter: kats, Assigned: san13692, Mentored)

Details

(Whiteboard: [lang=c++] [good first bug] gfx-noted)

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #1116008 +++ We are transitioning all fields of FrameMetrics to use getters/setter. Anyone: feel free to pick one of the public fields in FrameMetrics (mPresShellResolution is a good one to take), transition it, and post a patch here, flagging me for review. Thanks! See bug 1116009 for an example.
Whiteboard: [lang=c++] [good first bug] → [lang=c++] [good first bug] gfx-noted
Attached patch 1120203.patch (obsolete) — Splinter Review
Please review the patch.
Attachment #8547680 - Flags: review?(bugmail.mozilla)
Comment on attachment 8547680 [details] [diff] [review] 1120203.patch Review of attachment 8547680 [details] [diff] [review]: ----------------------------------------------------------------- Looks pretty good! Just a a couple of small changes below, and please add a commit message to the patch. Also for future reference it's a good idea to announce you're working on a bug when you start, so we can assign the bug to you and somebody else doesn't pick it up while you're working on it :) I've also pushed this patch to try to make sure it passes builds across all platforms: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=5604de657103 ::: browser/config/mozconfig @@ +6,5 @@ > # . $topsrcdir/browser/config/mozconfig > # to the top of your mozconfig file. > > ac_add_options --enable-application=browser > +mk_add_options AUTOCLOBBER=1 Remove this change ::: gfx/layers/FrameMetrics.h @@ +521,5 @@ > > private: > + // --------------------------------------------------------------------------- > + // The following metrics are dimensionless. > + // You can get rid of this comment
Attachment #8547680 - Flags: review?(bugmail.mozilla) → feedback+
Assignee: nobody → san13692
Please also take a look at the link to the tryserver I posted in comment 2. It shows some build failures on Android; there are uses of mPresShellResolution that are inside an #ifdef so they only get compiled on Android. Your patch will need to update those as well. Thanks!
Hi surabhi, are you still planning on finishing this up?
Flags: needinfo?(san13692)
ya i will send the patch today.
Flags: needinfo?(san13692)
Great, thanks! :)
Attached patch 1120203.patchSplinter Review
Here's the updated patch ,please review it.
Attachment #8547680 - Attachment is obsolete: true
Attachment #8550349 - Flags: review?(bugmail.mozilla)
Comment on attachment 8550349 [details] [diff] [review] 1120203.patch Review of attachment 8550349 [details] [diff] [review]: ----------------------------------------------------------------- For some reason the generated patch was corrupt; one of the hunks in FrameMetrics.h didn't have the right line numbers so I had to fix it up by hand. Not sure how you generated it, but it doesn't really matter now I guess. Patch looks good and the try run at https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f232dbc7833 looks green also. Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/f3df1027b8b2
Attachment #8550349 - Flags: review?(bugmail.mozilla) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Thanks :)
Thank you! :)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: