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

Issue 638276 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Aug 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocking:
issue 684916
issue 634832



Sign in to add a comment

N=1 Offline Pages thrashing per tab

Project Member Reported by dewittj@chromium.org, Aug 16 2016

Issue description

The Recent Tab helper creates pages in the following way:
* Wait for an interesting event
* Look for any other pages for the current tab.
 - if any exist delete them
* Save the page.

Because there are multiple "interesting" events (timeout, onload) it is possible for a single navigation to cause the creation of an offline copy:

* navigation commits
* timeout -> SNAPSHOT 1
* onload -> DELETE 1 -> SNAPSHOT 2

This can be problematic for UI that is trying to display the current offline pages, since there is thrashing; UI might have to delete and then re-add a page in a short period of time even though conceptually the page should just have been updated.
 
Cc: dim...@chromium.org
Dmitry, any thoughts on this behavior?  NTP is finding it problematic that pages disappear transiently for a given URL, since while they are able to remove content after load has completed, they endeavor never to add content to the page post-lod.

Comment 2 by dim...@chromium.org, Aug 22 2016

So, if they don't want to add items to UI, would an interface that has Added/Changed/Removed kind of observer work for them? One problem we currently have is that we don't have the Changed, but now that SavePage takes proposed_offline_id as a parameter, we might as well provide Change.

Comment 3 by pke@google.com, Aug 29 2016

Blocking: 634832

Comment 4 by pke@google.com, Aug 29 2016

Cc: treib@chromium.org

Comment 5 by treib@chromium.org, Sep 22 2016

Cc: vitaliii@chromium.org
Owner: dewittj@chromium.org
Status: Assigned (was: Untriaged)
Needs two improvements:
1) Allow overwriting of offline pages
2) Update observer to fire changed events
Status: Started (was: Assigned)
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 8 2016

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

commit 00c85a032fea6997e8b14e88b288d19618e62b15
Author: dewittj <dewittj@chromium.org>
Date: Thu Dec 08 17:12:52 2016

[Offline Pages] Convert Download UI Adapter to using OfflinePageAdded

Recently OfflinePageModelChanged was renamed to OfflinePageAdded to reflect
the reality of when that function was being called.  As a result, the
OfflinePageAdded function now receives as a parameter the OfflinePageItem
that was added.  This allows for a much simpler observer function that
directly operates on the new page in the download cache.

BUG= 638276 

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

[modify] https://crrev.com/00c85a032fea6997e8b14e88b288d19618e62b15/components/offline_pages/core/downloads/download_ui_adapter.cc
[modify] https://crrev.com/00c85a032fea6997e8b14e88b288d19618e62b15/components/offline_pages/core/downloads/download_ui_adapter.h
[modify] https://crrev.com/00c85a032fea6997e8b14e88b288d19618e62b15/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 9 2016

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

commit 804d3453c04c1caf0bf66cd73d5e2ae36dde7ae7
Author: dewittj <dewittj@chromium.org>
Date: Fri Dec 09 20:14:38 2016

[Offline Pages] Convert Download Suggestions Provider to using OfflinePageAdded

Recently OfflinePageModelChanged was renamed to OfflinePageAdded to reflect
the reality of when that function was being called.  As a result, the
OfflinePageAdded function now receives as a parameter the OfflinePageItem
that was added.  This allows for a much simpler observer function that
directly operates on the new page in the download cache.

BUG= 638276 

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

[modify] https://crrev.com/804d3453c04c1caf0bf66cd73d5e2ae36dde7ae7/chrome/browser/ntp_snippets/download_suggestions_provider.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 3 2017

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

commit 26b982a36584ba683f760383210ab64a83b24bb3
Author: dewittj <dewittj@chromium.org>
Date: Tue Jan 03 23:15:34 2017

[Offline Pages] Make new archives have random file names.

Currently offline pages with the same offline ID, title and URL will
produce archive files with the same filename.  This leads to unexpected
circumstances where archives could be overwritten even if the offline
page model is not updated.

This improvement also allows for more reliable updates to existing offline
pages by making the update process 3-step: create an archive, then update
the offline page to point to the newly created archive, then clean up the
old archive.

BUG= 638276 

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

[modify] https://crrev.com/26b982a36584ba683f760383210ab64a83b24bb3/chrome/browser/android/offline_pages/offline_page_mhtml_archiver.cc
[modify] https://crrev.com/26b982a36584ba683f760383210ab64a83b24bb3/chrome/browser/android/offline_pages/offline_page_mhtml_archiver.h
[modify] https://crrev.com/26b982a36584ba683f760383210ab64a83b24bb3/chrome/browser/android/offline_pages/offline_page_mhtml_archiver_unittest.cc
[modify] https://crrev.com/26b982a36584ba683f760383210ab64a83b24bb3/components/offline_pages/core/offline_page_model_impl.cc
[modify] https://crrev.com/26b982a36584ba683f760383210ab64a83b24bb3/components/offline_pages/core/offline_page_model_impl.h

Cc: carlosk@chromium.org
+Carlos who is interested in an Update method.
Blocking: 684916
Labels: Hotlist-Fixit
Cc: -vitaliii@chromium.org
Status: WontFix (was: Started)

Sign in to add a comment