Closed
Bug 1120203
Opened 9 years ago
Closed 9 years ago
Transition more fields of FrameMetrics to use getters/setters
Categories
(Core :: Panning and Zooming, defect)
Core
Panning and Zooming
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)
22.82 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
+++ 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.
Reporter | ||
Updated•9 years ago
|
Whiteboard: [lang=c++] [good first bug] → [lang=c++] [good first bug] gfx-noted
Assignee | ||
Comment 1•9 years ago
|
||
Please review the patch.
Attachment #8547680 -
Flags: review?(bugmail.mozilla)
Reporter | ||
Comment 2•9 years ago
|
||
bustage try |
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+
Reporter | ||
Updated•9 years ago
|
Assignee: nobody → san13692
Reporter | ||
Comment 3•9 years ago
|
||
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!
Reporter | ||
Comment 4•9 years ago
|
||
Hi surabhi, are you still planning on finishing this up?
Flags: needinfo?(san13692)
Reporter | ||
Comment 6•9 years ago
|
||
Great, thanks! :)
Assignee | ||
Comment 7•9 years ago
|
||
Here's the updated patch ,please review it.
Attachment #8547680 -
Attachment is obsolete: true
Attachment #8550349 -
Flags: review?(bugmail.mozilla)
Reporter | ||
Comment 8•9 years ago
|
||
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+
https://hg.mozilla.org/mozilla-central/rev/f3df1027b8b2
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Assignee | ||
Comment 10•9 years ago
|
||
Thanks :)
Reporter | ||
Comment 11•9 years ago
|
||
Thank you! :)
You need to log in
before you can comment on or make changes to this bug.
Description
•