1120203 - Transition more fields of FrameMetrics to use getters/setters

Status: RESOLVED FIXED

Description

[Kartikaya Gupta (away Jan 12-26; email:[e-mail])]

+++ 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.

Comment 1

[surabhi anand]

Created attachment 8547680 [details] [diff] [review]
1120203.patch

Please review the patch.

Comment 2

[Kartikaya Gupta (away Jan 12-26; email:[e-mail])]

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

Comment 3

[Kartikaya Gupta (away Jan 12-26; email:[e-mail])]

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!

Comment 4

[Kartikaya Gupta (away Jan 12-26; email:[e-mail])]

Hi surabhi, are you still planning on finishing this up?

Comment 5

[surabhi anand]

ya i will send the patch today.

Comment 6

[Kartikaya Gupta (away Jan 12-26; email:[e-mail])]

Great, thanks! :)

Comment 7

[surabhi anand]

Created attachment 8550349 [details] [diff] [review]
1120203.patch

Here's the updated patch ,please review it.

Comment 8

[Kartikaya Gupta (away Jan 12-26; email:[e-mail])]

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

Comment 9

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/f3df1027b8b2

Comment 10

[surabhi anand]

Thanks :)

Comment 11

[Kartikaya Gupta (away Jan 12-26; email:[e-mail])]

Thank you! :)