New issue
Advanced search Search tips

Issue 702762 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: ----

Blocking:
issue 695306



Sign in to add a comment

Do not save/restore empty NTPs (unless selected).

Project Member Reported by tedc...@chromium.org, Mar 17 2017

Issue description

Currently, Chrome saves out tab state files for NTPs even if there is no navigation.  During tab restore, there is a meta data file that contains the URL of all tabs to be restored as well as individual tab state files.  In the case of an NTP, there is no reason to save the state file as the URL is all you need (and avoids creating unnecessary file writes).

The initial part of this proposal is to stop saving NTP individual state files unless there is navigation associated with them.

The second part of the proposal is to stop restoring NTPs entirely unless they are the active tab.  We want to encourage users to do menu -> new tab without worrying about cluttering their tab stack for next time (or to think if they need to rummage around through their tabs in case there is an NTP).

With this change, there will be no state files stored as well as no NTPs restored (unless you were previously looking at it or it had any navigation associated with it).
 
Cc: klo...@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 20 2017

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

commit 3c7d798abb3a42da2bdaeeb0497a47575fda9bd2
Author: tedchoc <tedchoc@chromium.org>
Date: Mon Mar 20 23:41:01 2017

[Android] Do not restore NTPs from disk unless they are selected.

This also disables saving tab states for NTPs as they are not
needed for tab restore anyway (the metadata file will restore
based on URL if the state file is missing and can handle the
NTP selected case anyway).

Sidenote...we should consider not persisting tab states to disk
that have no navigations nor any web contents state.

BUG= 702762 

Review-Url: https://codereview.chromium.org/2757013002
Cr-Commit-Position: refs/heads/master@{#458241}

[modify] https://crrev.com/3c7d798abb3a42da2bdaeeb0497a47575fda9bd2/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java
[modify] https://crrev.com/3c7d798abb3a42da2bdaeeb0497a47575fda9bd2/chrome/android/java_sources.gni
[modify] https://crrev.com/3c7d798abb3a42da2bdaeeb0497a47575fda9bd2/chrome/android/javatests/src/org/chromium/chrome/browser/TabsTest.java
[add] https://crrev.com/3c7d798abb3a42da2bdaeeb0497a47575fda9bd2/chrome/android/junit/src/org/chromium/chrome/browser/tabmodel/TabPersistentStoreUnitTest.java

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 21 2017

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

commit 28865543d686d93a737842b56b623f019af8ddac
Author: dimich <dimich@chromium.org>
Date: Tue Mar 21 00:26:44 2017

Revert of [Android] Do not restore NTPs from disk unless they are selected. (patchset #3 id:40001 of https://codereview.chromium.org/2757013002/ )

Reason for revert:
Broke the compile:

https://build.chromium.org/p/chromium/builders/Android/builds/69977

Original issue's description:
> [Android] Do not restore NTPs from disk unless they are selected.
>
> This also disables saving tab states for NTPs as they are not
> needed for tab restore anyway (the metadata file will restore
> based on URL if the state file is missing and can handle the
> NTP selected case anyway).
>
> Sidenote...we should consider not persisting tab states to disk
> that have no navigations nor any web contents state.
>
> BUG= 702762 
>
> Review-Url: https://codereview.chromium.org/2757013002
> Cr-Commit-Position: refs/heads/master@{#458241}
> Committed: https://chromium.googlesource.com/chromium/src/+/3c7d798abb3a42da2bdaeeb0497a47575fda9bd2

TBR=dfalcantara@chromium.org,tedchoc@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 702762 

Review-Url: https://codereview.chromium.org/2766493002
Cr-Commit-Position: refs/heads/master@{#458251}

[modify] https://crrev.com/28865543d686d93a737842b56b623f019af8ddac/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java
[modify] https://crrev.com/28865543d686d93a737842b56b623f019af8ddac/chrome/android/java_sources.gni
[modify] https://crrev.com/28865543d686d93a737842b56b623f019af8ddac/chrome/android/javatests/src/org/chromium/chrome/browser/TabsTest.java
[delete] https://crrev.com/b4113a55840f0137241b6f6173375a6514318ff8/chrome/android/junit/src/org/chromium/chrome/browser/tabmodel/TabPersistentStoreUnitTest.java

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 21 2017

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

commit 61ea7ade8321d66dccb81984581e379adcb0aa42
Author: tedchoc <tedchoc@chromium.org>
Date: Tue Mar 21 22:17:05 2017

Revert "Revert of [Android] Do not restore NTPs from disk unless they are selected. (patchset #3 id:40001 of https://codereview.chromium.org/2757013002/ )"

This reverts commit 28865543d686d93a737842b56b623f019af8ddac.

I believe it to be an issue where the bot is building with java 1.7 and
locally I am building with 1.8.  In 1.8, the cast is not required from
isNull(), but that is not the case with 1.7 (according to the interwebs).

TBR=dfalcantara@chromium.org
BUG= 702762 

Review-Url: https://codereview.chromium.org/2762313002
Cr-Commit-Position: refs/heads/master@{#458578}

[modify] https://crrev.com/61ea7ade8321d66dccb81984581e379adcb0aa42/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java
[modify] https://crrev.com/61ea7ade8321d66dccb81984581e379adcb0aa42/chrome/android/java_sources.gni
[modify] https://crrev.com/61ea7ade8321d66dccb81984581e379adcb0aa42/chrome/android/javatests/src/org/chromium/chrome/browser/TabsTest.java
[add] https://crrev.com/61ea7ade8321d66dccb81984581e379adcb0aa42/chrome/android/junit/src/org/chromium/chrome/browser/tabmodel/TabPersistentStoreUnitTest.java

Status: Fixed (was: Started)

Sign in to add a comment