764621 - Remove unnecessary static variables from GeckoApp

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, defect)

Description

[Sriram Ramasubramanian [:sriram]]

There are so many "public" "static" variables in GeckoApp (mostly due to copy-paste problems). It's better to kill them to avoid having them in bizarre states.

Comment 1

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch: FormAssistant

Comment 2

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch: DoorHanger, TabsPanel

Comment 3

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch: Orientation

Comment 4

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

Comment 5

[:Margaret Leibovic]

Comment on attachment 632932 [details] [diff] [review]
Patch: FormAssistant

Any reason not to also change the mFormAssistPopup.hide() call in BrowserApp to hideFormAssistPopup()?

Comment 6

[Sriram Ramasubramanian [:sriram]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/59a5abb11899
http://hg.mozilla.org/integration/mozilla-inbound/rev/f324ce27518a
http://hg.mozilla.org/integration/mozilla-inbound/rev/e3d4ca557ce4
http://hg.mozilla.org/integration/mozilla-inbound/rev/7b7194063277

Comment 7

[Sriram Ramasubramanian [:sriram]]

In BrowserApp, the mFormAssistPopup is a member variable, and so directly calling hide() on it felt easier to me.

Comment 8

[:Margaret Leibovic]

(In reply to Sriram Ramasubramanian [:sriram] from comment #6)
> http://hg.mozilla.org/integration/mozilla-inbound/rev/59a5abb11899

Whoopsies, I didn't r+ this one yet! But I'm fine with your reasoning in comment 7.

Comment 9

[Matt Brubeck (:mbrubeck)]

Sorry, I backed this out because something in the push caused various failures in mochitest-1, 2, 3, and 8.
https://hg.mozilla.org/integration/mozilla-inbound/rev/c820098cb62d

Comment 10

[Matt Brubeck (:mbrubeck)]

It's not clear yet whether this backout fixed the failures or not; at least some are still broken after the backout, though some failures in M1 and M8 might be fixed.  At least some of the M2 failures were actually bug 765106, and there's also a lot of noise because of new frequent oranges from bug 761125.  Still investigating this...

Comment 11

[:Cykesiopka]

FWIW,

Bug 769269 Patch 3/4 changed the following to non-static:
  mDoorHangerPopup
  mFormAssistPopup
  mFindInPageBar
( https://hg.mozilla.org/mozilla-central/rev/3ce3068cb1df )

Bug 778247 Patch 9 changed the following to protected:
  mDoorHangerPopup
  mFormAssistPopup
  mTabsPanel
( https://hg.mozilla.org/mozilla-central/rev/e98e417adc2f )

Comment 12

[Owais Kazi]

Anymore changes to done in this bug?

Comment 13

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.