New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 297839 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Oct 2013
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Launching Chrome with a URL when first run hasn't yet been run puts Tab with URL in background

Project Member Reported by dfalcant...@chromium.org, Sep 24 2013

Issue description

Application Version (from "Chrome Settings > About Chrome"): Chrome, ToT

Steps to reproduce: 
1) Start fresh and clear your data.  Clear any command line flags that disable FRE.
2) Fire an intent to launch Chrome with a particular URL
adb shell am start -n com.google.android.apps.chrome/.Main -d google.com --ez create_new_tab true
3) Finish FRE, don't sign in

Observed behavior: 
The welcome page is shown, with the tab for the URL on a separate background tab.

Expected behavior: 
The tab for the URL is shown, with the welcome tab in the background.

Frequency: 
5/5

Additional comments: 
IIRC one of the examples where this behavior was necessary was for situations where a user had to sign in to something (like a hotel wireless network) and bring up Chrome for the first time.  Users would get confused because they didn't know that another page had been launched.

Assigning to Alan since he seems to have touched this code path recently.
 
Status: Started
I have introduced a hidden ShardedPreference to keep track if the user has seen the Welcome page and force it to foreground if she hasn't seen it. I suspect it didn't play well with this case.

Looking...
This is strange. It seems like the code part didn't change from before (at least for this case).

However, TabModel.moveTab() isn't doing what I am expected so the later call to TabModel.setIndex() isn't setting the correct tab.

@dan / yusufo: Are you guys familar with what moveTab suppose to do?
Cc: dtrainor@chromium.org tedc...@chromium.org
+dtrainor, tedchoc

You guys know what that function's supposed to do?
I'm not familiar with moveTab.  I would assume it was added for drag/drop tab movement on the tablet, but that is purely speculation.
Owner: dtrainor@chromium.org
I have figured out what the problem is. Both moveTab() and setIndex() are behaving exactly as we expect. The issue is that DeferredTabModel is undoing some of the work.

So here is what happen: Before the AndroidEDU CL: https://gerrit-int.chromium.org/#/c/44288/5/java/apps/chrome/src/com/google/android/apps/chrome/Main.java
everything in FRE was launched synchronously. This is no longer the case.

Normally the DeferredTabModel would listens to the ChromeNotifications created by the actual TabModel and both would still be in sync. But right now, the TAB_CREATED boardcast came after the TAB_MOVED boardcast. Since it never saw the initial TAB_CREATED, it ignored the TAB_MOVED altogether.

One solution might be to use boardcastImmediately for TAB_CREATED so everything would still be in sync but I am not sure what are the consequences.

Another solution should be to completely remove the dependency on DeferredModel during FRE.

David's probably a better person to make that change.

 
I can try to take a look this week.  Thanks for tracking this down!
Labels: Cr-UI-Browser-FirstRun
Opening from a bookmark and going through the first run on a tablet doesn't seem to cause this anymore on M32.
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 17 2013

Project: .../internal/apps
Branch : master
Author : David Trainor <dtrainor@chromium.org>
Commit : 189660cfdcd761e3702cf84bd9c9c13dc2b3505e

Code Review +2: David Trainor, Ted Choc
Verified    +1: David Trainor
Change-Id     : If258f88f64a4a3bf9572fb0ada70fb839388b856
Reviewed-at   : https://gerrit-int.chromium.org/45078
Labels: Merge-Requested

Comment 11 by laforge@google.com, Oct 18 2013

Labels: -Merge-Requested Merge-Approved
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 22 2013

Project: .../internal/apps
Branch : 1650
Author : David Trainor <dtrainor@chromium.org>
Commit : 249f1ee101699161ca90e6915021563d38d2d1a9

Code Review +2: David Trainor
Verified    +1: David Trainor
Change-Id     : I9185ddccefdab28544b104a72cfd75369382186e
Reviewed-at   : https://gerrit-int.chromium.org/45246
Labels: -Merge-Approved Merge-Merged
Status: Fixed
Status: Verified
Verification Attempt:
Chrome Version: 31.0.1650.32
Devices: S3/ JZO54K and TF201/JZO54K

Steps: Same as #0.
Verification succeeded: Yes

Sign in to add a comment