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

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

+++ This bug was initially created as a clone of Bug #1058614 +++

We are transitioning all fields of FrameMetrics to use getters/setter.

Anyone: feel free to pick one of the public fields in FrameMetrics (mMayHaveTouchListeners is an easy one), transition it, and post a patch here, flagging me for review. Thanks! See bug 1058614 for an example.

Comment 1

[Himanshu Singh]

i want to work on this bug.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Great! It's very similar to the last one you worked on. I suggest transitioning the mMayHaveTouchListeners field. This time we'll leave it up to you to find all the places in the code the field is used - you can use local text-search tools or you can use our online code indexing tools at http://mxr.mozilla.org/mozilla-central/ or http://dxr.mozilla.org/mozilla-central/source/

Feel free to ask any questions here or on IRC.

Comment 3

[Himanshu Singh]

Attachment: 1087478.patch

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8509702 [details] [diff] [review]
1087478.patch

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

Almost correct, just one little problem... please upload a new patch with the comment below addressed. Thanks!

Also, as you work on additional patches in the mozilla codebase you'll learn how to run some of the tests suites to exercise your code and make sure nothing broke. In this case, running the gtest code would have caught this error. You can run "mach gtest" (without the quotes) to run this code. It would be a good exercise for you to take this patch as-is, run the gtests, and observe the test failures. Then correct the patch and rebuild firefox, and re-run the tests and observe them passing. (This is optional, since I've already done it, but highly recommended if you care to learn more about our code development process).

::: gfx/layers/FrameMetrics.h
@@ +535,5 @@
>    }
>  
> +  bool GetMayHaveTouchListeners() const
> +  {
> +    return mMayHaveTouchCaret;

This is a copy-paste error

Comment 5

[Himanshu Singh]

Attachment: 1087478.patch

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8510009 [details] [diff] [review]
1087478.patch

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

Perfect, thanks!

Comment 7

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/b2g-inbound/rev/126479bbdc25

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Although there are more transitions to be done I'm going to hold off on filing another bug for it until bug 1076163 is landed, because that changes a lot of code in FrameMetrics and is going to be annoying to rebase if we keep changing fields from under it.

Himanshu, if you're interested in working on additional bugs I recommend taking a look at http://www.joshmatthews.net/bugsahoy/ to find some, or if you want I can try to look for a couple that might be suitable for you. Let me know!

Comment 9

[Himanshu Singh]

Please Kartikaya if you find any bug let me know.And thanks for help

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Himanshu, bug 1020184 looks like a good one (somebody seems to have started with it but hasn't touched it in months, so you should be able to pick it up). I can also CC you when I file a new bug for more FrameMetrics field transitions.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/126479bbdc25