Closed Bug 1262343 Opened 8 years ago Closed 8 years ago

Remove History and Synced Tabs code

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox48 fixed)

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: liuche, Assigned: liuche)

References

Details

Attachments

(3 files)

Remove the code associated with the old History panel.
No longer depends on: 1260321
Most of this is sheer code removal, but I did add a reference to the sync setup layout (3rd patch) because we will need that layout and those strings.

The other non-trivial part is that we can't remove HISTORY and REMOTE_TABS PanelTypes completely, because they're used in previous migrations [1] like adding Synced Tabs or combining History/Synced Tabs. However, the only thing we really need are the string uuids [2] that reference each panel because we need them to get the PanelType when unpickling panels from JSON [3] in the fromId call [4].

("id" is a little overloaded in this code, so I included some references.)

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/HomeConfigPrefsBackend.java#315
[2] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/HomeConfig.java#1675
[3] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/HomeConfigPrefsBackend.java#167
[4] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/HomeConfig.java#137
Assignee: nobody → liuche
Comment on attachment 8743609 [details]
MozReview Request: Bug 1262343 - Remove old History panel code. r=sebastian

https://reviewboard.mozilla.org/r/47923/#review44775

Awesome!

I suggest running lint to find new unused resources, if you haven't already. :)

::: mobile/android/base/java/org/mozilla/gecko/home/HomeConfig.java:46
(Diff revision 1)
>       */
>      @RobocopTarget
>      public static enum PanelType implements Parcelable {
>          TOP_SITES("top_sites", TopSitesPanel.class),
>          BOOKMARKS("bookmarks", BookmarksPanel.class),
> -        HISTORY("history", HistoryPanel.class),
> +        HISTORY("history", CombinedHistoryPanel.class),

Do we still need this PanelType now after the migration?
Attachment #8743609 - Flags: review?(s.kaspari) → review+
Comment on attachment 8743610 [details]
MozReview Request: Bug 1262343 - Remove old Synced panel code. r=sebastian

https://reviewboard.mozilla.org/r/47925/#review44779

Red diffs are best diffs.
Attachment #8743610 - Flags: review?(s.kaspari) → review+
Comment on attachment 8743611 [details]
MozReview Request: Bug 1262343 - Add reference to remote tabs empty layout for future use. r=sebastian

https://reviewboard.mozilla.org/r/47927/#review44781
Attachment #8743611 - Flags: review?(s.kaspari) → review+
(In reply to Chenxia Liu [:liuche] from comment #5)
> The other non-trivial part is that we can't remove HISTORY and REMOTE_TABS
> PanelTypes completely, because they're used in previous migrations [1] like
> adding Synced Tabs or combining History/Synced Tabs. However, the only thing
> we really need are the string uuids [2] that reference each panel because we
> need them to get the PanelType when unpickling panels from JSON [3] in the
> fromId call [4].

Oh, I see. Can we at least move them to the bottom/top, add a comment and maybe prefix the enum constant? Something like DEPRECATED_HISTORY?
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.