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

commit fd10fe94ba828949dcddd148fb73c3797ab83ef3
Author: carlosk <carlosk@chromium.org>
Date: Wed Jan 18 03:41:49 2017

Minor improvements to RecentTabHelper.

Adds minor improvements to RecentTabHelper:

- Better organize the state kept during the async call chain of a snapshot
  save: always invalidate pointers when needed and convert if-checks of
  |download_info| to DCHECKs.
- Add more verifications the the navigation type before accepting to
  save a snapshot for it.
- Removed the unneeded IsSamePage method.
- Added isSnapshottingForLastN to make it clearer what the request_id checks
  exist for.
- Move member initializers to the header file.
- Some minor code simplifications.

This is a preparation step to change the snapshot mechanism to use user
interaction triggers instead of navigation events.

BUG= 678367 

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

[modify] https://crrev.com/fd10fe94ba828949dcddd148fb73c3797ab83ef3/chrome/browser/android/offline_pages/recent_tab_helper.cc
[modify] https://crrev.com/fd10fe94ba828949dcddd148fb73c3797ab83ef3/chrome/browser/android/offline_pages/recent_tab_helper.h
[modify] https://crrev.com/fd10fe94ba828949dcddd148fb73c3797ab83ef3/chrome/browser/android/offline_pages/recent_tab_helper_unittest.cc

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

commit 6b26b7a9ba6b62730399c632dac0fcbfce0a15bd
Author: carlosk <carlosk@chromium.org>
Date: Fri Jan 20 22:21:08 2017

Offline Page Cache uses user interaction triggers for saving pages.

This switches Offline Page Cache (aka last_n) to listen to user interaction
triggers instead of navigation events for starting page snapshots. The latter
are still monitored and important to allow the quality estimation of the page
being loaded but they just don't cause snapshot creation by themselves anymore.

Tests were also updated to work with this new mechanism.

BUG= 678367 

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

[modify] https://crrev.com/6b26b7a9ba6b62730399c632dac0fcbfce0a15bd/chrome/browser/android/offline_pages/recent_tab_helper.cc
[modify] https://crrev.com/6b26b7a9ba6b62730399c632dac0fcbfce0a15bd/chrome/browser/android/offline_pages/recent_tab_helper.h
[modify] https://crrev.com/6b26b7a9ba6b62730399c632dac0fcbfce0a15bd/chrome/browser/android/offline_pages/recent_tab_helper_unittest.cc
[modify] https://crrev.com/6b26b7a9ba6b62730399c632dac0fcbfce0a15bd/components/offline_pages/core/snapshot_controller.cc
[modify] https://crrev.com/6b26b7a9ba6b62730399c632dac0fcbfce0a15bd/components/offline_pages/core/snapshot_controller.h

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

commit fe02d6b0fd390b03345187b65e29831f4fb89ac7
Author: carlosk <carlosk@chromium.org>
Date: Sat Jan 21 01:35:36 2017

Last_n: Minor follow-up fixes post user interaction triggers.

This change fixes two issues with the recently landed CL that made
last_n listen to user interaction triggers for snapshot creation:

- Removes the PageQuality alias from the RecentTabHelper header.

So that this alias is not spread to other files importing this header.

- Don't ignore same page navigations for creating new snapshots.

I found out that in same page navigations the contents of the page
can change considerably. So the subcase of segment navigations I was
considering where we would like to avoid extra saves, is only a special
case among all that fit a same page one.

BUG= 678367 
TBR=dimich@chromium.org

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

[modify] https://crrev.com/fe02d6b0fd390b03345187b65e29831f4fb89ac7/chrome/browser/android/offline_pages/recent_tab_helper.cc
[modify] https://crrev.com/fe02d6b0fd390b03345187b65e29831f4fb89ac7/chrome/browser/android/offline_pages/recent_tab_helper.h

I'd like to request the merge of changes 6b26b7a9ba6b62730399c632dac0fcbfce0a15bd and fe02d6b0fd390b03345187b65e29831f4fb89ac7.

Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

I'm sorry but as we found a bug in this change I will not be merging it into M57.

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

commit 01fea95b3e06b6b270b816e798e03a7995589d1a
Author: carlosk <carlosk@chromium.org>
Date: Sat Jan 28 02:07:35 2017

RecentTabHelper won't accept overlapping requests from Downloads anymore.

This change fixes a free-after-use bug by changing the way RecentTabHelper
handles page snapshot requests from Downloads. The main change is that it now
will ignore overlapping requests from Downloads but a few other changes were
made, many of them required to get this properly fixed:
- Moved SnapshotProgressInfo definition into the CC file.
- Fixed issues with overlapping snapshot requests from Downloads and page
  quality status events.
- Fixed potential issue where last_n and Downloads requests would be mixed up in
  case Background Downloader was disabled.
- Added enforcement of acceptable Offline namespaces for requests.
- Changed the logic for automatically replacing Downloads snapshots once the
  page attains high quality ("fully" loaded).
- Other minor code and comments improvements.

Note: snapshot overlap between last_n and Downloads requests are still accepted.

BUG= 678367 ,683580

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

[modify] https://crrev.com/01fea95b3e06b6b270b816e798e03a7995589d1a/chrome/browser/android/offline_pages/recent_tab_helper.cc
[modify] https://crrev.com/01fea95b3e06b6b270b816e798e03a7995589d1a/chrome/browser/android/offline_pages/recent_tab_helper.h
[modify] https://crrev.com/01fea95b3e06b6b270b816e798e03a7995589d1a/chrome/browser/android/offline_pages/recent_tab_helper_unittest.cc

Hi, M58 BP is in 9 days, what is the current progress on this?

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

commit f0b30fafe9d73f73c430895d289ab83c3e818d20
Author: carlosk <carlosk@chromium.org>
Date: Tue Feb 21 19:37:00 2017

Last_n: create snapshots from user triggers even if similar quality ones exist.

With this change when a tab-hidden event happens a last_n snapshot will
be created even if one already exists with the same expected quality
level. This should fix cases of very dynamic pagesnot being properly
saved, where contents are completely changed without clear navigation or
loading events being triggered.

This also re-introduces the ignoring of same-page navigations when
listening to DidFinishNavigation so that we don't reset the loading
state of the page in those cases.

BUG= 691506 , 678367 

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

[modify] https://crrev.com/f0b30fafe9d73f73c430895d289ab83c3e818d20/chrome/browser/android/offline_pages/recent_tab_helper.cc
[modify] https://crrev.com/f0b30fafe9d73f73c430895d289ab83c3e818d20/chrome/browser/android/offline_pages/recent_tab_helper.h
[modify] https://crrev.com/f0b30fafe9d73f73c430895d289ab83c3e818d20/chrome/browser/android/offline_pages/recent_tab_helper_unittest.cc

Re #14: I am still confident in a M58 launch but once more things were more complicated than initially expected. ;)

The main blocker right now is proper tab closure support (blocking  issue 685791 ): its patch depends on another patch that misses one LGTM from someone who's OOO until next week.

Finally, reported bugs were all dealt with.

Just providing some more details on the change landed in #15.

Why are we saving snapshots on every tab hidden event? And why are we "ignoring" same-page navigations?

These two changes are tied together. The problem we were dealing with was that dynamic pages (aka with heavy javascript usage) could fully change the contents of a page without triggering a navigation event (see  issue 691506 ). Most times these would still fire a same-page navigation and that's why we were previously "tracking" them.

The problem is that even though these navigations happened they were not generating the usual page-loading events we expected after detecting a navigation (DOM parsed, on load). This caused these pages to be considered at "early loading stage" for ever and we'd never allow snapshots to happen as we always wait for pages to be "minimally loaded". 

So we decided to go back to "ignoring" same-page navigations so that the "load status" would not be reset but still be based on the latest non-same-page navigation loading events.

But beyond that we also had an optimization that would avoid a new snapshot to be created if the page load state hadn't changed since the previous one. Now that we learned how much pages can change without those loading events being triggered we also had to remove this restriction.