695836 - sync last screen and last uri preference

Status: RESOLVED FIXED

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

Comment 1

[Doug Turner (:dougt)]

Attachment: work in progress.

pretty close to what we want.  I want to see if we can also store the offset

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Doug Turner (:dougt) from comment #1)
> Created attachment 568994 [details] [diff] [review] [diff] [details] [review]
> work in progress.
> 
> pretty close to what we want.  I want to see if we can also store the offset

Sounds like something we want in session restore anyway

Comment 3

[Doug Turner (:dougt)]

if so, maybe we go with this as-is.  it is a ton better now.

the basic idea is that we can't keep the last uri in prefs because we need it before gecko starts up.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 568994 [details] [diff] [review]
work in progress.


>diff --git a/embedding/android/GeckoApp.java b/embedding/android/GeckoApp.java

>+                    // this is already true
>+                    // surfaceView.showStartupBitmap();

Remove these

>+                    final String awesomeURI = uri; 
>+                    mMainHandler.post(new Runnable() {
>+                      public void run() {
>+                        mBrowserToolbar.setTitle(awesomeURI);
>+                      }
>+                    });

Does this pass the blassey-test for mMainHandler vs. mHandler ?

Also, can't we try to use "last-title" pref here?

>+    private void rememberLastScreen(boolean sync) {

>+        if (!tab.getHistory().empty()) {
>+
>+            SharedPreferences prefs = getSharedPreferences ("GeckoApp", 0);

nits: no blank line needed at top of block and drop the space from | getSharedPreferences ( | (you do it every time you use getSharedPreferences)

>+            surfaceView.saveLast(sync);
>+        }
>+
>+    }

no blank line needed here

>+    private boolean restoreLastScreen() {

>+        return true;
>+        //        surfaceView.saveLast(sync);

remove

>     private void quit() {

>+        System.exit(0);
>         GeckoAppShell.nativeQuit();
>         finish();

Brad removed System.exit(0) and added the 2 new lines in this patch:
http://hg.mozilla.org/projects/birch/rev/653aa43e9643

Do we need it?

>diff --git a/embedding/android/GeckoSurfaceView.java b/embedding/android/GeckoSurfaceView.java

>     public void surfaceCreated(SurfaceHolder holder) {

>+        Log.i(LOG_FILE_NAME, "!! surface created");
>+
>+        // Delay sending this event if we are painting the
>+        // load screen.  The native access paint path will
>+        // paint directly to the screen and we will see a
>+        // black screen while content is initally being
>+        // drawn.
>+        if (!mShowingLoadScreen) {
>+            GeckoEvent e = new GeckoEvent(GeckoEvent.SURFACE_CREATED);
>+            GeckoAppShell.sendEventToGecko(e);
>+        }

You might want to move the Log into the if block, so we know if this is _really_ called

>diff --git a/mobile/components/BrowserCLH.js b/mobile/components/BrowserCLH.js

> BrowserCLH.prototype = {
>   handle: function fs_handle(aCmdLine) {
>-    dump("fs_handle");
> 
>-    let urlParam = aCmdLine.handleFlagWithParam("remote", false);

Don't leave the leading blank line

>+    let urlParam = "about:";

about:home? or do you want to wait?

>+    try {
>+        urlParam = aCmdLine.handleFlagWithParam("remote", false);

2 spaces

>+    } catch (e) { dump(""+e); }

nit: dump("" + e);

r+ with nits and some thought for the System.exit(0)

the mMainHandler things is up to you :)

Comment 5

[Doug Turner (:dougt)]

> Does this pass the blassey-test for mMainHandler vs. mHandler ?

There are like 2 dozen similar patterns here for setting the ui widgets.  we can have a follow up.

> Also, can't we try to use "last-title" pref here?

Not sure what you mean.  We can set the title instead, either way.

Comment 6

[Doug Turner (:dougt)]

http://hg.mozilla.org/projects/birch/rev/45c91268cfee