1047971 - Drop even more redundant field initialisers

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Chris Kitching [:ckitching]]

See bug 1041836.

Two things occurred to me:

1) I found a regex bug that caused some of the problems detected by the analyser to be omitted from the patch, so I'm providing a patch here to finish the job.

2) I gave the analyser the ability to detect equivalently-redundant assignments given explicitly in constructors (also not fixable by proguard, as this is to what the other sort of initialiser is converted at compile-time anyway).

It turns out we have quite a few of these: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/Tab.java#96

Comment 1

[Chris Kitching [:ckitching]]

Attachment: Part 1/3: Delete redundant initialisers-to-default-values from constructors

Comment 2

[Chris Kitching [:ckitching]]

Attachment: Part 2/3: Delete redundant initialisers-to-default-values from constructors (android-sync)

Comment 3

[Chris Kitching [:ckitching]]

Attachment: Part 3/3: Delete even more redundant field initialisers

Comment 4

[Chris Kitching [:ckitching]]

https://tbpl.mozilla.org/?tree=Try&rev=23795b78bd58

Comment 5

[Richard Newman [:rnewman]]

Comment on attachment 8466754 [details] [diff] [review]
Part 1/3: Delete redundant initialisers-to-default-values from constructors

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

r+ except for TwoWayView.

::: mobile/android/base/widget/TwoWayView.java
@@ -316,5 @@
>      public TwoWayView(Context context, AttributeSet attrs) {
>          this(context, attrs, 0);
>      }
>  
>      public TwoWayView(Context context, AttributeSet attrs, int defStyle) {

Please split out the TwoWayView changes into a separate patch, and ask lucasr for review; it's effectively a third-party dependency.

Comment 6

[Richard Newman [:rnewman]]

Comment on attachment 8466755 [details] [diff] [review]
Part 2/3: Delete redundant initialisers-to-default-values from constructors (android-sync)

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

Don't forget uplift (either by sending a pull request, or asking me as you land this).

Comment 7

[Chris Kitching [:ckitching]]

Attachment: Part 4/3: Delete redundant initialisers-to-default-values from constructors in TwoWayView

Splitting out changes to TwoWayView.

Lucas: Explicit initialisation of fields to zero, null, or false are redundant and not stripped during any compilation or optimisation phase. This patch removes them, slightly reducing binary size and cleaning up code.
In general, Java guarantees that all non-final fields are initialised to zero, null, or false.

Comment 8

[Lucas Rocha (:lucasr)]

Comment on attachment 8466866 [details] [diff] [review]
Part 4/3: Delete redundant initialisers-to-default-values from constructors in TwoWayView

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

Looks good. For future patches, you should submit changes to TwoWayView directly to the upstream project in: https://github.com/lucasr/twoway-view/

I've already pushed these changes there: https://github.com/lucasr/twoway-view/commit/de9082db9824c38c7c7e52947170e6312d0a594b

Comment 9

[Chris Kitching [:ckitching]]

https://hg.mozilla.org/integration/fx-team/rev/c092f9e4310d
https://hg.mozilla.org/integration/fx-team/rev/4bd09430f063

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c092f9e4310d
https://hg.mozilla.org/mozilla-central/rev/4bd09430f063

Comment 11

[Lucas Rocha (:lucasr)]

(In reply to Lucas Rocha (:lucasr) from comment #8)
> Comment on attachment 8466866 [details] [diff] [review]
> Part 4/3: Delete redundant initialisers-to-default-values from constructors
> in TwoWayView
> 
> Review of attachment 8466866 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good. For future patches, you should submit changes to TwoWayView
> directly to the upstream project in: https://github.com/lucasr/twoway-view/
> 
> I've already pushed these changes there:
> https://github.com/lucasr/twoway-view/commit/
> de9082db9824c38c7c7e52947170e6312d0a594b

Just to be clear: I pushed the patch to the upstream repo but you still have to land it in m-c.

Comment 12

[Chris Kitching [:ckitching]]

https://hg.mozilla.org/integration/fx-team/rev/cf087beb1f9f

Comment 13

[Chris Kitching [:ckitching]]

Ah.

Comment 14

[Ed Morley [:emorley]]

(In reply to Chris Kitching [:ckitching] from comment #9)
> https://hg.mozilla.org/integration/fx-team/rev/c092f9e4310d
> https://hg.mozilla.org/integration/fx-team/rev/4bd09430f063

One of these two caused bug 1037617 to go perma-orange - attempts to back these out lead to conflicts - could you either fix the failure or back this out? :-)

Comment 15

[Ed Morley [:emorley]]

https://tbpl.mozilla.org/?tree=Fx-Team&jobname=panda.*robocop-3&fromchange=725c4b034064&tochange=a45200559a09

Comment 16

[Ed Morley [:emorley]]

The failures can also be seen in the try run:
https://tbpl.mozilla.org/?tree=Try&rev=23795b78bd58
(though since they match a known intermittent it's easily missed)

Comment 17

[Chris Kitching [:ckitching]]

Righto: working on it now.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

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

Comment 19

[Ed Morley [:emorley]]

(In reply to Chris Kitching [:ckitching] from comment #17)
> Righto: working on it now.

Thank you :-)