New issue
Advanced search Search tips

Issue 755646 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Make it clearer what code from FirstRunActivity is run by LightweightFirstRunActivity

Project Member Reported by pkotw...@chromium.org, Aug 15 2017

Issue description

Make it clearer what code from FirstRunActivity is run by LightweightFirstRunActivity

I suggest making LightweightFirstRunActivity inherit from FirstRunActivityBase instead of FirstRunActivity. The current setup with LightweightFirstRunActivity inheriting from FirstRunActivity is begging for subtle regressions

 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2fa4c0ec65560e7a093d18aed1df6a17f87fb566

commit 2fa4c0ec65560e7a093d18aed1df6a17f87fb566
Author: Peter Kotwicz <pkotwicz@chromium.org>
Date: Thu Aug 24 03:02:19 2017

[Android FRE] Remove redundant ChromeTabbedActivity#launchFirstRunExperience()

As a result of https://codereview.chromium.org/2833673003 having landed,
ChromeTabbedActivity checks for whether the First Run Experience needs
to be shown in AsyncInitializationActivity#onCreateInternal() which
occurs prior to ChromeTabbedActivity#preInflationStartup()

BUG= 755646 

Change-Id: I442b33a0ef6eb5b5a72e72bdf25cd97014b8e641
Reviewed-on: https://chromium-review.googlesource.com/616440
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Ted Choc (OOO 8.21-25) <tedchoc@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496932}
[modify] https://crrev.com/2fa4c0ec65560e7a093d18aed1df6a17f87fb566/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
[modify] https://crrev.com/2fa4c0ec65560e7a093d18aed1df6a17f87fb566/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71cc1fabacd07a91bbddbdf37f70832962fe930c

commit 71cc1fabacd07a91bbddbdf37f70832962fe930c
Author: Peter Kotwicz <pkotwicz@chromium.org>
Date: Thu Aug 24 16:09:36 2017

[Android FRE] Remove unnecessary conditional FRE completion in FirstRunActivity#start()

https://codereview.chromium.org/2833673003 made FirstRunActivity singleInstance.
Remove handling in FirstRunActivity#start() for scenario where user completed
first run via a different FirstRunActivity.

BUG= 755646 

Change-Id: I4d8fbed8014033b43a3f3bfcd04241957b9c7f25
Reviewed-on: https://chromium-review.googlesource.com/617020
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Reviewed-by: Ted Choc (OOO 8.21-25) <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497069}
[modify] https://crrev.com/71cc1fabacd07a91bbddbdf37f70832962fe930c/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4ac1c54dc14b13fb07a5b08ce4c1bc2b5bec2e33

commit 4ac1c54dc14b13fb07a5b08ce4c1bc2b5bec2e33
Author: Peter Kotwicz <pkotwicz@chromium.org>
Date: Thu Aug 24 20:14:34 2017

[Android FRE] Remove useless FINISH_ON_TOUCH_OUTSIDE intent extra

As of https://chromium-review.googlesource.com/c/616440 the FirstRunActivity is
always launched with the FirstRunActivity.EXTRA_FINISH_TOUCH_OUTSIDE intent
extra set to true. This CL removes the intent extra.

BUG= 755646 

Change-Id: I060219efdbf8cd7f2862cebd6364fa99a15d7ed3
Reviewed-on: https://chromium-review.googlesource.com/617061
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Ted Choc (OOO 8.21-25) <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497171}
[modify] https://crrev.com/4ac1c54dc14b13fb07a5b08ce4c1bc2b5bec2e33/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java
[modify] https://crrev.com/4ac1c54dc14b13fb07a5b08ce4c1bc2b5bec2e33/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencer.java

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1461f544b674b8cbf7d76a5ff328af2b9536a481

commit 1461f544b674b8cbf7d76a5ff328af2b9536a481
Author: Peter Kotwicz <pkotwicz@chromium.org>
Date: Wed Aug 30 15:14:35 2017

[Android FRE] Replace finishAllTheActivities() with Activity#finish()

There can be at most one FirstRunActivity and one LightweightFirstRunActivity
because FirstRunActivity is android:launchMode="singleInstance"

BUG= 755646 

Change-Id: Id19b38e25224539274fac733677cbd09a1117eaf
Reviewed-on: https://chromium-review.googlesource.com/641932
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498466}
[modify] https://crrev.com/1461f544b674b8cbf7d76a5ff328af2b9536a481/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java
[modify] https://crrev.com/1461f544b674b8cbf7d76a5ff328af2b9536a481/chrome/android/java/src/org/chromium/chrome/browser/firstrun/LightweightFirstRunActivity.java

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59a4c0473660a22be20b5ad4169255dea2f0ace8

commit 59a4c0473660a22be20b5ad4169255dea2f0ace8
Author: Peter Kotwicz <pkotwicz@chromium.org>
Date: Tue Sep 12 04:03:31 2017

[Android FRE] Remove LightweightFirstRunActivity dependence on FirstRunActivity

This CL introduces FirstRunActivityBase which contains logic to be shared
between FirstRunActivity and LightweightFirstRunActivity.
This CL changes LightweightFirstRunActivity to extend
FirstRunActivityBase instead of FirstRunActivity.

BUG= 755646 

Change-Id: Id839e78361030aef6d7a5d2e94544760f3c117c8
Reviewed-on: https://chromium-review.googlesource.com/619252
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501173}
[modify] https://crrev.com/59a4c0473660a22be20b5ad4169255dea2f0ace8/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java
[add] https://crrev.com/59a4c0473660a22be20b5ad4169255dea2f0ace8/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivityBase.java
[modify] https://crrev.com/59a4c0473660a22be20b5ad4169255dea2f0ace8/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencer.java
[modify] https://crrev.com/59a4c0473660a22be20b5ad4169255dea2f0ace8/chrome/android/java/src/org/chromium/chrome/browser/firstrun/LightweightFirstRunActivity.java
[modify] https://crrev.com/59a4c0473660a22be20b5ad4169255dea2f0ace8/chrome/android/java_sources.gni

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b6de262a1c493935983e068ccb506008022ab79f

commit b6de262a1c493935983e068ccb506008022ab79f
Author: Peter Kotwicz <pkotwicz@chromium.org>
Date: Wed Oct 04 18:03:38 2017

[Android FRE] Simplify FirstRunActivity state restoring

This CL limits the state that FirstRunActivity restores to the state sent via
FirstRunFlowSequencer#createGenericFirstRunIntent(). This enables
deleting the FirstRunActivity#EXTRA_USE_FRE_FLOW_SEQUENCER and
FirstRunActivity#POST_NATIVE_SETUP_NEEDED extras

Bug= 755646 

Change-Id: I1a8f4dd7c13ab5649ac2656e5a0b20c34b875e90
Reviewed-on: https://chromium-review.googlesource.com/670359
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506442}
[modify] https://crrev.com/b6de262a1c493935983e068ccb506008022ab79f/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java
[modify] https://crrev.com/b6de262a1c493935983e068ccb506008022ab79f/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencer.java
[modify] https://crrev.com/b6de262a1c493935983e068ccb506008022ab79f/chrome/android/java/src/org/chromium/chrome/browser/firstrun/LightweightFirstRunActivity.java
[modify] https://crrev.com/b6de262a1c493935983e068ccb506008022ab79f/chrome/android/junit/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencerTest.java

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/24ec56d9888bb04f00dec09a39754e34064a0bc6

commit 24ec56d9888bb04f00dec09a39754e34064a0bc6
Author: Peter Kotwicz <pkotwicz@chromium.org>
Date: Wed Oct 04 21:41:40 2017

[Android FRE] Simplify logic for showing the search engine page

As a result of
https://chromium-review.googlesource.com/c/chromium/src/+/670359
the values of SHOW_SEARCH_ENGINE_PAGE in FirstRunActivity#mFreProperties and
FirstRunFlowSequencer#shouldShowSearchEnginePage() are guaranteed to
be the same.

Bug= 755646 

Change-Id: I8a0eef268f34319af3c1129aec8066bee214582d
Reviewed-on: https://chromium-review.googlesource.com/700426
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506528}
[modify] https://crrev.com/24ec56d9888bb04f00dec09a39754e34064a0bc6/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java

Status: Fixed (was: Started)

Sign in to add a comment