882716 - Implement "tabs from last session" sub-fragment in the new about:home

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect, P1)

Description

[Lucas Rocha (:lucasr)]

Tapping on the "Tabs from last session" button should open a new fragment inside homepager.

Comment 1

[Lucas Rocha (:lucasr)]

Attachment: Add getFaviconBytesForUrl() to BrowserDB

Comment 2

[Lucas Rocha (:lucasr)]

Attachment: Implement "Last Tabs" page for new about:home

Comment 3

[Lucas Rocha (:lucasr)]

FYI: Again, the code to actually show the fragment is coming in a separate patch. This one is self-contained enough to be reviewed separately.

Comment 4

[:Margaret Leibovic]

Drive-by: Isn't "Tabs from last time" supposed to be a sub-section of the "Visited" page? Or has that changed? If not, I feel like we shouldn't put "Page" in the name for this fragment.

Comment 5

[Brian Nicholson (:bnicholson)]

Comment on attachment 767743 [details] [diff] [review]
Implement "Last Tabs" page for new about:home

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

::: mobile/android/base/home/LastTabsPage.java
@@ +126,5 @@
> +    @Override
> +    public void onActivityCreated(Bundle savedInstanceState) {
> +        super.onActivityCreated(savedInstanceState);
> +
> +        // Intialize adapter

Nit: s/Intialize/Initialize/

@@ +131,5 @@
> +        mAdapter = new LastTabsAdapter(getActivity());
> +        mList.setAdapter(mAdapter);
> +
> +        // Create callbacks before the initial loader is started
> +        mCursorLoaderCallbacks = new CursorLoaderCallbacks();

Does mCursorLoaderCallbacks need to be a class variable? This is the only place you use it, so you could probably make it local unless you planned on referencing it elsewhere.

Comment 6

[Wesley Johnston (:wesj)]

Comment on attachment 767739 [details] [diff] [review]
Add getFaviconBytesForUrl() to BrowserDB

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

::: mobile/android/base/db/LocalBrowserDB.java
@@ +685,5 @@
> +                         Combined.URL + " = ?",
> +                         new String[] { uri },
> +                         null);
> +
> +            if (!c.moveToFirst()) {

cr.query can return null.

Comment 7

[Lucas Rocha (:lucasr)]

Pushed:
http://hg.mozilla.org/projects/fig/rev/c4a0d7678b34
http://hg.mozilla.org/projects/fig/rev/950e12ef1eb8

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c4a0d7678b34
https://hg.mozilla.org/mozilla-central/rev/950e12ef1eb8