Other findings:
- This happens for both PDFs and other types of downloads.
- Even when the download is complete, the page does not refresh and it keeps the offline dino page.

Looks like once that download is handed off to the download manager the offline page component doesn't cancel/remove it's download.  Some logs:

- Turn off wifi
- Go to pdf link
- Click download when offline

notifyDownloadProgress: LEGACY_OFFLINE_PAGE_0c6a8d93-4754-433a-9eda-042b5c45180d, null
notifyDownloadInterrupted: LEGACY_OFFLINE_PAGE_0c6a8d93-4754-433a-9eda-042b5c45180d

- Turn on wifi

notifyDownloadProgress: LEGACY_OFFLINE_PAGE_0c6a8d93-4754-433a-9eda-042b5c45180d, null
notifyDownloadProgress: LEGACY_DOWNLOAD_5e60f6aa-0973-44ec-955a-efa897d2ced9, http://www.ohchr.org/Documents/ProfessionalInterest/cat.pdf
notifyDownloadSuccessful: LEGACY_DOWNLOAD_5e60f6aa-0973-44ec-955a-efa897d2ced9

The actual pdf finished downloading, the page did not.  We don't really 'hand off' notifications well, but that download notification should go away.


Adding shaktisahu@ who changed some of the offline pages <-> notification bridge and dimich@/chili@ from the offline pages team.

Is this on ToT?

Current behavior for pdfs *should* be: (tested on beta)
-- Offline pages try to download page
-- No download is started
-- Offline pages times out after 5 minutes
-- retries 2 more times for timeout
-- fail download

I've a CL in progress that would fix the following:
-- Offline pages try to download a pdf
-- If browser actions:
-- -- Allow download
-- -- delete offline pages notification
-- Else:
-- -- Deny download
-- -- report download failed immediately

Oops did not see original bug. I'm checking to see if WebContentsDelegate::CanDownload no longer blocks downloads (could this be related to NetworkService??).

BTW, current behavior as described here and observed in Canary 65.0.3299.0 may actually be close to what we need. The 'page' link in search results is usually a redirect to PDF. We do a redirect, and then hand off the PDF download to DownloadManager. The PDF is downloaded. The only negative is the remaining notification.

If we hand off notifications as well (by removing page-related one) it seems we'll fix a class of issues, is that correct?

Yes, if we allow offline pages to initiate downloads regardless of browser actions or not, we will be able to fix this class of issues.

design doc: https://docs.google.com/document/d/1jyOWKL0SzMN3PB9_4JkMvIJjeKipAsiPGtNny2Bdiwo/edit#

@5 - Yeah I think we just need the notification to go away.  It seems like the rest of the flow is working as expected, but nothing is notifying the UI to remove the notification.

@6 - IIUC I think the issue is technically 'fixed' from a functionality standpoint.  We just don't have the final cleanup call through download_ui_adapter.

Assigning to chili@ for tracking.  Does comment #7 make sense?

#7 makes sense.  What doesn't make sense is why this started happening all of a sudden. Our background web contents blocks all downloads by returning false when WebContentsDelegate::canDownload() is called. If for some reason that code path is not being used, I'm not sure how we can recognize that a download has been initiated and we should abort...

Tracing still

After putting in a gazillion logging statements, it appears this is what is happening:

Scenario:
1. turn off wifi
2. navigate to pdf page
3. click download
4. turn on wifi *while still having chrome as foreground and on pdf page*

What happens:
1. Offline pages will try to download the page. It will appear to infinite download (fix on the way to fast-fail).  This should not currently download anything.
2. Because chrome is in foreground, turning on wifi will trigger automatic reload of the active tab page, which is the pdf.
3. The automatic reload triggers the download of the pdf, resulting in pdf downloaded.

Upcoming fixes:
1. Offline pages will fast-fail when we encounter a download rather than fail from timing out.
2. With browser actions flag turned on, offline pages will trigger downloads rather than fast-fail, and gracefully remove notification

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

commit 1d2614b0b554c0d64c3b05b037d12313dc2306f5
Author: Cathy Li <chili@chromium.org>
Date: Mon Feb 12 20:25:25 2018

[Offline pages] Allow downloads to start from background offliner.

Also introduces 2 new final statuses to track how often this happens.

Bug: 788615, 796247,  796204 
Change-Id: Iec7e09c390c0f7accefed19a011913aec4541aab
Reviewed-on: https://chromium-review.googlesource.com/789615
Commit-Queue: Cathy Li <chili@chromium.org>
Reviewed-by: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536171}
[modify] https://crrev.com/1d2614b0b554c0d64c3b05b037d12313dc2306f5/chrome/browser/offline_pages/background_loader_offliner.cc
[modify] https://crrev.com/1d2614b0b554c0d64c3b05b037d12313dc2306f5/chrome/browser/offline_pages/background_loader_offliner.h
[modify] https://crrev.com/1d2614b0b554c0d64c3b05b037d12313dc2306f5/chrome/browser/offline_pages/background_loader_offliner_unittest.cc
[modify] https://crrev.com/1d2614b0b554c0d64c3b05b037d12313dc2306f5/components/offline_pages/content/background_loader/background_loader_contents.cc
[modify] https://crrev.com/1d2614b0b554c0d64c3b05b037d12313dc2306f5/components/offline_pages/content/background_loader/background_loader_contents.h
[modify] https://crrev.com/1d2614b0b554c0d64c3b05b037d12313dc2306f5/components/offline_pages/content/background_loader/background_loader_contents_unittest.cc
[modify] https://crrev.com/1d2614b0b554c0d64c3b05b037d12313dc2306f5/components/offline_pages/core/background/offliner.h
[modify] https://crrev.com/1d2614b0b554c0d64c3b05b037d12313dc2306f5/components/offline_pages/core/background/request_coordinator.cc
[modify] https://crrev.com/1d2614b0b554c0d64c3b05b037d12313dc2306f5/components/offline_pages/core/background/request_coordinator_event_logger.cc
[modify] https://crrev.com/1d2614b0b554c0d64c3b05b037d12313dc2306f5/components/offline_pages/core/background/request_notifier.h
[modify] https://crrev.com/1d2614b0b554c0d64c3b05b037d12313dc2306f5/components/offline_pages/core/downloads/download_notifying_observer.cc

We still see this issue on Chrome:66.0.3348.0 Device:Samsung Galaxy S7 Edge(SM-G935A)/NRD90M as per the steps mentioned in C#0 

Observed behavior:
PDF downloads successfully but there is another notification that claims download failed.

@Video:http://go/chrome-androidlogs1/8/796204

@prashanthpola, that is expected, per explanation in comment 10

The fix I landed is for the indefinite download - it now fails immediately rather than hangs indefinitely