New issue
Advanced search Search tips

Issue 896604 link

Starred by 1 user
Status: Fixed
Owner:
bashi@chromium.org
Closed: Oct 18
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

BlobRequest::ReadDiskCacheItem async events sometimes don't finish

Project Member Reported by bashi@chromium.org, Oct 18 
What steps will reproduce the problem?
(1) Install docs offline extension (ID: ghbmnnjooekpmoecnnnilnnbdlolhkhi)
(2) Visit https://docs.google.com and enable docs offline. You will need an account with service worker enabled. I think a corp account should work. 
(3) Open chrome://tracing and start recording a trace with Blob category enabled.
(4) Open an arbitrary docs. I used https://docs.google.com/document/d/1GvzDP-tTLmJ0myRhUAfTYWs3ZUFilUICg8psNHyccwQ/edit
(5) Finish recording the trace.

What is the expected result?
All BlobRequest::ReadDiskCacheItem slices have end events.

What happens instead?
Some ones did not finish. Please see attached capture and trace.
read-disk-cache-not-finished.png
35.0 KB View Download
read-disk-cache-not-finished.html
5.8 MB View Download
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 18 

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

commit e994cf8cae5bea504a17fe6fc65e60bfb4a0a480
Author: Kenichi Ishibashi <bashi@chromium.org>
Date: Thu Oct 18 10:34:44 2018

Record async events only after ERR_IO_PENDING in BlobReader

According to the comment on DiskCache::ReadData[1], it's not guaranteed
that the completion callback is called when the read request immediately
succeeds. We should use TRACE_EVENT_ASYNC_BEGIN1() macro only after
ERR_IO_PENDING because otherwise async events never finish.

[1] https://cs.chromium.org/chromium/src/net/disk_cache/disk_cache.h?l=301&rcl=93f9ef9762b0c9158dd8acb0ceb52dbd14a308a3

Change-Id: Ie70730b8dfffd0777a68eabd9970216054cdc7ab
Bug:  896604 
Reviewed-on: https://chromium-review.googlesource.com/c/1288336
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600718}
[modify] https://crrev.com/e994cf8cae5bea504a17fe6fc65e60bfb4a0a480/storage/browser/blob/blob_reader.cc

Comment 2 by bashi@chromium.org, Oct 18

Status: Fixed (was: Assigned)
►

Sign in to add a comment