Bug 1245493 - Don't animate when showing toolbar when FF is first unhidden. r=margaret
After this patch, I still occasionally see the toolbar positioned part
way down from the top of the screen. However, this state looks slightly
less janky without the animation I removed and I can't consistently
reproduce it anymore. Given the DynamicToolbar.setVisible calls I make,
I'd guess this is likely to be a bug caused by BrowserApp.onTabChanged
(and thus DynamicToolbar.setVisible) not getting called instantly and
so the DynamicToolbar is initialized to a different location on screen.
I'd guess it's a bug in DynamicToolbar as to why it's positioned partially
off-screen.
There is a little bit of code duplication, but that is because the code
to load a url on a new intent is duplicated (i.e. once from GeckoApp.initialize
- the initial load - and once from GeckoApp.onNewIntent). This could
potentially be cleaned up if we moved the app loading code into onResume,
but that may not be possible since we need to wait for Gecko to start
up.
Additionally, this patch adds a lot of hard-to-follow global state, which is
also not good.
Preferred solution (
bug 1269041): show the toolbar each time onStart is
called (i.e. FF is unhidden). This is good for the user - they'll be
more aware which page they're on - but it's janky with the current
implementation, where the page content does not scroll when the toolbar
is shown so previously visible content is hidden. Thus, I went with the
other approach. fwiw, Chrome does this behavior, but scrolls the toolbar
offscreen shortly after it is shown.
This solution is blocked on
bug 1245523.
MozReview-Commit-ID: 7JjCrIf4KTm
--- a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ -336,17 +336,22 @@ public class BrowserApp extends GeckoApp
}
return;
}
Log.d(LOGTAG, "BrowserApp.onTabChanged: " + tab.getId() + ": " + msg);
switch (msg) {
case SELECTED:
if (Tabs.getInstance().isSelectedTab(tab) && mDynamicToolbar.isEnabled()) {
- mDynamicToolbar.setVisible(true, VisibilityTransition.ANIMATE);
+ final VisibilityTransition transition = (tab.getShouldShowToolbarWithoutAnimationOnFirstSelection()) ?
+ VisibilityTransition.IMMEDIATE : VisibilityTransition.ANIMATE;
+ mDynamicToolbar.setVisible(true, transition);
+
+ // The first selection has happened - reset the state.
+ tab.setShouldShowToolbarWithoutAnimationOnFirstSelection(false);
}
// fall through
case LOCATION_CHANGE:
if (mZoomedView != null) {
mZoomedView.stopZoomDisplay(false);
}
if (Tabs.getInstance().isSelectedTab(tab)) {
updateHomePagerForTab(tab);
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
@@ -203,16 +203,18 @@ public abstract class GeckoApp
private String mPrivateBrowsingSession;
private volatile HealthRecorder mHealthRecorder;
private volatile Locale mLastLocale;
private Intent mRestartIntent;
+ private boolean mWasFirstTabShownAfterActivityUnhidden;
+
abstract public int getLayout();
protected void processTabQueue() {};
protected void openQueuedTabs() {};
@SuppressWarnings("serial")
class SessionRestoreException extends Exception {
@@ -1338,16 +1340,22 @@ public abstract class GeckoApp
BrowserLocaleManager.storeAndNotifyOSLocale(GeckoSharedPrefs.forProfile(GeckoApp.this), osLocale);
}
});
GeckoAppShell.setNotificationClient(makeNotificationClient());
IntentHelper.init(this);
}
+ @Override
+ public void onStart() {
+ super.onStart();
+ mWasFirstTabShownAfterActivityUnhidden = false; // onStart indicates we were hidden.
+ }
+
/**
* At this point, the resource system and the rest of the browser are
* aware of the locale.
*
* Now we can display strings!
*
* You can think of this as being something like a second phase of onCreate,
* where you can do string-related operations. Use this in place of embedding
@@ -1429,16 +1437,19 @@ public abstract class GeckoApp
public String getHomepage() {
return null;
}
private void initialize() {
mInitialized = true;
+ final boolean isFirstTab = !mWasFirstTabShownAfterActivityUnhidden;
+ mWasFirstTabShownAfterActivityUnhidden = true; // Reset since we'll be loading a tab.
+
final SafeIntent intent = new SafeIntent(getIntent());
final String action = intent.getAction();
final String uri = getURIFromIntent(intent);
final String passedUri;
if (!TextUtils.isEmpty(uri)) {
passedUri = uri;
@@ -1486,16 +1497,19 @@ public abstract class GeckoApp
Tabs.getInstance().notifyListeners(null, Tabs.TabEvents.RESTORED);
processActionViewIntent(new Runnable() {
@Override
public void run() {
int flags = Tabs.LOADURL_NEW_TAB | Tabs.LOADURL_USER_ENTERED | Tabs.LOADURL_EXTERNAL;
if (ACTION_HOMESCREEN_SHORTCUT.equals(action)) {
flags |= Tabs.LOADURL_PINNED;
}
+ if (isFirstTab) {
+ flags |= Tabs.LOADURL_FIRST_AFTER_ACTIVITY_UNHIDDEN;
+ }
loadStartupTab(passedUri, intent, flags);
}
});
} else {
if (!mIsRestoringActivity) {
loadStartupTab(Tabs.LOADURL_NEW_TAB);
}
@@ -1881,16 +1895,19 @@ public abstract class GeckoApp
}
handleNotification(ACTION_ALERT_CALLBACK, alertName, alertCookie);
}
@Override
protected void onNewIntent(Intent externalIntent) {
final SafeIntent intent = new SafeIntent(externalIntent);
+ final boolean isFirstTab = !mWasFirstTabShownAfterActivityUnhidden;
+ mWasFirstTabShownAfterActivityUnhidden = true; // Reset since we'll be loading a tab.
+
// if we were previously OOM killed, we can end up here when launching
// from external shortcuts, so set this as the intent for initialization
if (!mInitialized) {
setIntent(externalIntent);
return;
}
final String action = intent.getAction();
@@ -1905,19 +1922,21 @@ public abstract class GeckoApp
if (ACTION_LOAD.equals(action)) {
Tabs.getInstance().loadUrl(intent.getDataString());
} else if (Intent.ACTION_VIEW.equals(action)) {
processActionViewIntent(new Runnable() {
@Override
public void run() {
final String url = intent.getDataString();
- Tabs.getInstance().loadUrlWithIntentExtras(url, intent, Tabs.LOADURL_NEW_TAB |
- Tabs.LOADURL_USER_ENTERED |
- Tabs.LOADURL_EXTERNAL);
+ int flags = Tabs.LOADURL_NEW_TAB | Tabs.LOADURL_USER_ENTERED | Tabs.LOADURL_EXTERNAL;
+ if (isFirstTab) {
+ flags |= Tabs.LOADURL_FIRST_AFTER_ACTIVITY_UNHIDDEN;
+ }
+ Tabs.getInstance().loadUrlWithIntentExtras(url, intent, flags);
}
});
} else if (ACTION_HOMESCREEN_SHORTCUT.equals(action)) {
GeckoAppShell.sendEventToGecko(GeckoEvent.createBookmarkLoadEvent(uri));
} else if (Intent.ACTION_SEARCH.equals(action)) {
GeckoAppShell.sendEventToGecko(GeckoEvent.createURILoadEvent(uri));
} else if (ACTION_ALERT_CALLBACK.equals(action)) {
processAlertCallback(intent);
--- a/mobile/android/base/java/org/mozilla/gecko/Tab.java
+++ b/mobile/android/base/java/org/mozilla/gecko/Tab.java
@@ -81,16 +81,17 @@ public class Tab {
private boolean mDesktopMode;
private boolean mEnteringReaderMode;
private final Context mAppContext;
private ErrorType mErrorType = ErrorType.NONE;
private volatile int mLoadProgress;
private volatile int mRecordingCount;
private volatile boolean mIsAudioPlaying;
private String mMostRecentHomePanel;
+ private boolean mShouldShowToolbarWithoutAnimationOnFirstSelection;
private int mHistoryIndex;
private int mHistorySize;
private boolean mCanDoBack;
private boolean mCanDoForward;
private boolean mIsEditing;
private final TabEditingState mEditingState = new TabEditingState();
@@ -894,9 +895,17 @@ public class Tab {
public void setIsEditing(final boolean isEditing) {
this.mIsEditing = isEditing;
}
public TabEditingState getEditingState() {
return mEditingState;
}
+
+ public void setShouldShowToolbarWithoutAnimationOnFirstSelection(final boolean shouldShowWithoutAnimation) {
+ mShouldShowToolbarWithoutAnimationOnFirstSelection = shouldShowWithoutAnimation;
+ }
+
+ public boolean getShouldShowToolbarWithoutAnimationOnFirstSelection() {
+ return mShouldShowToolbarWithoutAnimationOnFirstSelection;
+ }
}
--- a/mobile/android/base/java/org/mozilla/gecko/Tabs.java
+++ b/mobile/android/base/java/org/mozilla/gecko/Tabs.java
@@ -60,16 +60,18 @@ public class Tabs implements GeckoEventL
public static final int LOADURL_USER_ENTERED = 1 << 1;
public static final int LOADURL_PRIVATE = 1 << 2;
public static final int LOADURL_PINNED = 1 << 3;
public static final int LOADURL_DELAY_LOAD = 1 << 4;
public static final int LOADURL_DESKTOP = 1 << 5;
public static final int LOADURL_BACKGROUND = 1 << 6;
/** Indicates the url has been specified by a source external to the app. */
public static final int LOADURL_EXTERNAL = 1 << 7;
+ /** Indicates the tab is the first shown after Firefox is hidden and restored. */
+ public static final int LOADURL_FIRST_AFTER_ACTIVITY_UNHIDDEN = 1 << 8;
private static final long PERSIST_TABS_AFTER_MILLISECONDS = 1000 * 2;
public static final int INVALID_TAB_ID = -1;
private static final AtomicInteger sTabId = new AtomicInteger(0);
private volatile boolean mInitialTabsAdded;
@@ -829,16 +831,17 @@ public class Tabs implements GeckoEventL
// delayLoad implies background tab
boolean background = delayLoad || (flags & LOADURL_BACKGROUND) != 0;
try {
boolean isPrivate = (flags & LOADURL_PRIVATE) != 0;
boolean userEntered = (flags & LOADURL_USER_ENTERED) != 0;
boolean desktopMode = (flags & LOADURL_DESKTOP) != 0;
boolean external = (flags & LOADURL_EXTERNAL) != 0;
+ final boolean isFirstShownAfterActivityUnhidden = (flags & LOADURL_FIRST_AFTER_ACTIVITY_UNHIDDEN) != 0;
args.put("url", url);
args.put("engine", searchEngine);
args.put("parentId", parentId);
args.put("userEntered", userEntered);
args.put("isPrivate", isPrivate);
args.put("pinned", (flags & LOADURL_PINNED) != 0);
args.put("desktopMode", desktopMode);
@@ -889,16 +892,21 @@ public class Tabs implements GeckoEventL
String tabUrl = (url != null && Uri.parse(url).getScheme() != null) ? url : null;
// Add the new tab to the end of the tab order.
final int tabIndex = -1;
tabToSelect = addTab(tabId, tabUrl, external, parentId, url, isPrivate, tabIndex);
tabToSelect.setDesktopMode(desktopMode);
tabToSelect.setApplicationId(applicationId);
+ if (isFirstShownAfterActivityUnhidden) {
+ // We just opened Firefox so we want to show
+ // the toolbar but not animate it to avoid jank.
+ tabToSelect.setShouldShowToolbarWithoutAnimationOnFirstSelection(true);
+ }
}
} catch (Exception e) {
Log.w(LOGTAG, "Error building JSON arguments for loadUrl.", e);
}
GeckoAppShell.notifyObservers("Tab:Load", args.toString());
if (tabToSelect == null) {