Closed
Bug 1320298
Opened 7 years ago
Closed 7 years ago
[findbugs] [MS] mutable arrays
Categories
(Firefox for Android Graveyard :: General, defect, P3)
Tracking
(firefox53 fixed)
RESOLVED
FIXED
Firefox 53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: sebastian, Assigned: Shan11812)
References
Details
Attachments
(1 file)
+++ This bug was initially created as a clone of Bug #1316009 +++ * org.mozilla.gecko.AboutPages.DEFAULT_ICON_PAGES is a mutable array * org.mozilla.gecko.background.common.GlobalConstants.DEFAULT_CIPHER_SUITES is a mutable array * org.mozilla.gecko.background.common.GlobalConstants.DEFAULT_PROTOCOLS is a mutable array
Assignee | ||
Comment 1•7 years ago
|
||
(In reply to Sebastian Kaspari (:sebastian) from comment #0) > +++ This bug was initially created as a clone of Bug #1316009 +++ > > * org.mozilla.gecko.AboutPages.DEFAULT_ICON_PAGES is a mutable array > > * org.mozilla.gecko.background.common.GlobalConstants.DEFAULT_CIPHER_SUITES > is a mutable array > > * org.mozilla.gecko.background.common.GlobalConstants.DEFAULT_PROTOCOLS is a > mutable array As far as I know, we cant make an array immutable in java. I can only think of unmodifiableCollection (and may be enum?)
Flags: needinfo?(s.kaspari)
Reporter | ||
Comment 2•7 years ago
|
||
Yeah, see bug 1316009 comment 14 and the following ones. It's very likely that we are going to suppress the warning for GlobalConstants (because we will need an array anyways). For AboutPages.DEFAULT_ICON_PAGES we might change the code. But I haven't looked at all the use cases yet.
Flags: needinfo?(s.kaspari)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
Changed AboutPages.DEFAULT_ICON_PAGES to list. (Also changed GlobalConstants.DEFAULT_PROTOCOLS just in case you want to change it too?)
Reporter | ||
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8815254 [details] Bug 1320298 - Change mutable array to unmutable list; https://reviewboard.mozilla.org/r/96268/#review96738 I don't think we should change the values in GlobalConstants to a List. All callers require an array so we would just convert between list and array representation all the time. Changing DEFAULT_ICON_PAGES on the other hand makes sense. ::: mobile/android/base/java/org/mozilla/gecko/AboutPages.java:102 (Diff revision 1) > - for (int i = 0; i < DEFAULT_ICON_PAGES.length; ++i) { > - if (DEFAULT_ICON_PAGES[i].equals(url)) { > + for (int i = 0; i < DEFAULT_ICON_PAGES.size(); ++i) { > + if (DEFAULT_ICON_PAGES.get(i).equals(url)) { After turning this into a list we can use a nicer version of the loop: > for (String page : DEFAULT_ICON_PAGES) { > ..
Attachment #8815254 -
Flags: review?(s.kaspari) → review-
Comment hidden (mozreview-request) |
Reporter | ||
Updated•7 years ago
|
Assignee: nobody → 11812r
Status: NEW → ASSIGNED
Reporter | ||
Comment 7•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb3aa30b89f6
Reporter | ||
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8815254 [details] Bug 1320298 - Change mutable array to unmutable list; https://reviewboard.mozilla.org/r/96268/#review98456 This failed to build in automation: https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb3aa30b89f6&selectedJob=32362807 > /builds/slave/try-and-api-15-000000000000000/build/src/mobile/android/base/java/org/mozilla/gecko/icons/preparation/AboutPagesPreparer.java:26: error: method addAll in class Collections cannot be applied to given types; Can you fix this and upload a new patch?
Attachment #8815254 -
Flags: review?(s.kaspari) → review-
Comment hidden (mozreview-request) |
Reporter | ||
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8815254 [details] Bug 1320298 - Change mutable array to unmutable list; https://reviewboard.mozilla.org/r/96268/#review99396
Attachment #8815254 -
Flags: review?(s.kaspari) → review+
Reporter | ||
Comment 11•7 years ago
|
||
I pushed the new patch to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b01c39c34df2
Comment 12•7 years ago
|
||
Pushed by s.kaspari@gmail.com: https://hg.mozilla.org/integration/autoland/rev/fc105f5be22c Change mutable array to unmutable list; r=sebastian
Comment 13•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fc105f5be22c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•