Ouch. Justin, is offline pages support coming back in M52?

Offline pages are coming back in M52, as automatic caching of last n visited pages and bookmarks.

We had a discussion with dimich, bauerb, and treib, and we came to the conclusion that the delay is likely imperceptible given the cases where it will appear, so this won't block launch.

The only time this will delay the NTP is if Chrome is starting up from scratch and the very first tab that opens is the NTP.  The amount of the delay will be equivalent to reading one file from disk that contains metadata on the offline pages.  Then there will be no delay for subsequent loads of the NTP.

Unfortunately we don't yet have UMA that characterizes the delay, I'm going to add it for m52 so we can disable things if it causes a problem.

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

commit 97d87bb593370ec0a9f92437bfe7c7ee08c8409b
Author: dewittj <dewittj@chromium.org>
Date: Sat May 07 02:22:56 2016

Offline Pages: Add Model Load Time UMA.

Since many offline pages API calls now wait for the model to be loaded,
we want to characterize the amount of time it can take.

BUG= 607573 

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

[modify] https://crrev.com/97d87bb593370ec0a9f92437bfe7c7ee08c8409b/components/offline_pages/offline_page_model.cc
[modify] https://crrev.com/97d87bb593370ec0a9f92437bfe7c7ee08c8409b/components/offline_pages/offline_page_model.h
[modify] https://crrev.com/97d87bb593370ec0a9f92437bfe7c7ee08c8409b/tools/metrics/histograms/histograms.xml

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

commit a814ec0b9c16e31e664121ad948ae89070d96229
Author: dewittj <dewittj@chromium.org>
Date: Mon May 09 17:52:45 2016

NTP - Add a histogram tracking the time to load offline content.

This will be used to determine if a better UI is required while waiting
for disk operations to complete.

BUG= 607573 

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

[modify] https://crrev.com/a814ec0b9c16e31e664121ad948ae89070d96229/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
[modify] https://crrev.com/a814ec0b9c16e31e664121ad948ae89070d96229/tools/metrics/histograms/histograms.xml

Not blocking M52. Reach out to Justin for more comments.

Are we sure that this could not cause really long delays for rendering the NTP on start-up? I don't know the code, but since this is affecting start-up situations, is it possible that the offline pages model can't be queried immediately, because relevant subsystems or APIs are not fully initialized, yet?

https://uma.googleplex.com/p/chrome/histograms/?endDate=05-11-2016&dayCount=28&histograms=NewTabPage.OfflineUrlsLoadTime%2COfflinePages.Model.ConstructionToLoadedEventTime&fixupData=true&showMax=true&filters=isofficial%2Ceq%2CTrue&implicitFilters=isofficial

We don't have a lot of data yet, but total time for construction of the offline page model is in the <300ms range.  Because the model is constructed early on in the lifetime of the tab, the NTP is not waiting for that much time.

On my N4 phone locally, the NewTabPage.OfflineUrlsLoadTime for my most recent run of Chromium (Debug) is 83 ms.  This is the amount of time the NTP waited for the offline URLs in NewTabPageManager#getUrlsAvailableOffline()

We can add a workaround for this milestone - check to see if the model is loaded (still available as a synchronous call) and simply fail to badge if not.  This matches the previous behavior.

I'm leaving for the weekend, but put together a POC patch here: https://codereview.chromium.org/1982483002/

There are probably better ways to do this, and in fact I'd be interested in collecting the time we *would* have spent had this hack not landed.  But since branch point is not far away we could land something like this.

Thanks! The POC looks good to me, IMO it would be great if we could get this into M52.

I have no objections to adding "how long would we have waited" metrics, but I don't think it's all that important right now. I'd add "wait time" metrics once we switch to "real" async handling.

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

commit 9a12ce07eb8c36407ae27beba7b2082229cf5678
Author: dewittj <dewittj@chromium.org>
Date: Fri May 20 16:42:25 2016

New Tab Page: Work around potentially long delays if model is not loaded.

This avoids querying for offline pages if the model is not loaded, so
that the maximum possible delay is minimal (once the model is loaded,
all pages are stored in memory).

Additionally checks for null when obtaining the offline page model.

BUG= 607573 , 613359 

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

[modify] https://crrev.com/9a12ce07eb8c36407ae27beba7b2082229cf5678/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

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

commit fac0bea283906de6dffd7e9fe7af6ba5872949fe
Author: Justin DeWitt <dewittj@chromium.org>
Date: Tue May 24 17:54:20 2016

New Tab Page: Work around potentially long delays if model is not loaded.

This avoids querying for offline pages if the model is not loaded, so
that the maximum possible delay is minimal (once the model is loaded,
all pages are stored in memory).

Additionally checks for null when obtaining the offline page model.

BUG= 607573 , 613359 

Review-Url: https://codereview.chromium.org/1982483002
Cr-Commit-Position: refs/heads/master@{#395096}
(cherry picked from commit 9a12ce07eb8c36407ae27beba7b2082229cf5678)

Review URL: https://codereview.chromium.org/2007563005 .

Cr-Commit-Position: refs/branch-heads/2743@{#38}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/fac0bea283906de6dffd7e9fe7af6ba5872949fe/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

We assume that NTP UI is solving it by becoming more asynchronous

We do appreciate having the data quickly, so that the UI becomes stable.

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

commit 3dc6123ccb4f56866e5e25bee09b850d16ba7729
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Fri Sep 30 09:14:12 2016

Ntp: experiment to initially scroll below the fold.

Also deletes custom command line switch as all experiments can be controlled already via:
force-fieldtrials
enable-features
disable-features
force-fieldtrial-params

Enforces rendering the Most Visited items synchronously during initialization so that the layout is stable and the scroll position can be correctly initialized as well.

BUG=649727, 607573 

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

[modify] https://crrev.com/3dc6123ccb4f56866e5e25bee09b850d16ba7729/chrome/android/java/res/layout/new_tab_page_layout.xml
[modify] https://crrev.com/3dc6123ccb4f56866e5e25bee09b850d16ba7729/chrome/android/java/res/values/dimens.xml
[modify] https://crrev.com/3dc6123ccb4f56866e5e25bee09b850d16ba7729/chrome/android/java/src/org/chromium/chrome/browser/ntp/IncognitoNewTabPage.java
[modify] https://crrev.com/3dc6123ccb4f56866e5e25bee09b850d16ba7729/chrome/android/java/src/org/chromium/chrome/browser/ntp/IncognitoNewTabPageView.java
[modify] https://crrev.com/3dc6123ccb4f56866e5e25bee09b850d16ba7729/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
[modify] https://crrev.com/3dc6123ccb4f56866e5e25bee09b850d16ba7729/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageLayout.java
[modify] https://crrev.com/3dc6123ccb4f56866e5e25bee09b850d16ba7729/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageToolbar.java
[modify] https://crrev.com/3dc6123ccb4f56866e5e25bee09b850d16ba7729/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
[delete] https://crrev.com/5d919d6e4866aa240c11d21b917f2ac8ea5d5eb8/chrome/android/java/src/org/chromium/chrome/browser/ntp/NtpColorUtils.java
[add] https://crrev.com/3dc6123ccb4f56866e5e25bee09b850d16ba7729/chrome/android/java/src/org/chromium/chrome/browser/ntp/NtpStyleUtils.java
[modify] https://crrev.com/3dc6123ccb4f56866e5e25bee09b850d16ba7729/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/CardsVariationParameters.java
[modify] https://crrev.com/3dc6123ccb4f56866e5e25bee09b850d16ba7729/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java
[modify] https://crrev.com/3dc6123ccb4f56866e5e25bee09b850d16ba7729/chrome/android/java/src/org/chromium/chrome/browser/util/ColorUtils.java
[modify] https://crrev.com/3dc6123ccb4f56866e5e25bee09b850d16ba7729/chrome/android/java_sources.gni