New issue
Advanced search Search tips

Issue 785412 link

Starred by 1 user
Status: Fixed
Owner:
mek@chromium.org
Closed: Nov 2017
Cc:
dmu...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 611935



Sign in to add a comment

[mojo-blobs] Blob.ReadAll/ReadRange incorrectly uses null for FileSystemContext

Project Member Reported by mek@chromium.org, Nov 15 2017 
This means that trying to read a blob containing filesystem URL elements through these methods will result in a crash.

Fixing is somewhat non-trivial, as it means BlobImpl needs to somehow get access to a FileSystemContext, and thus all sites that create a BlobImpl might also need fixing. 

Other options could include:
- store the FileSystemContext in the BlobDataHandle
- or store the FileSystemContext in the individual BlobDataItems when those items actually represent filesystem URLs

I actually kind of like that last option, as it gets us somewhat closer to items abstracting away how they are read (making BlobReader need to know less about individual item types), and possibly eventually moving to filesystem entry items, disk cache entry items etc all just being some mojo interface to another part of the system (hopefully ultimately removing all knowledge of disk cache and filesystem APIs from the blob system).
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 27 2017 

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

commit 80fe13f10991ce52520459df1c66f4b22fa2e896
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Mon Nov 27 12:25:47 2017

Make reading blobs work through Blob mojo interface.

I apparently just passed null as FileSystemContext when reading a
blob through its mojo interface, which means any filesystem references
can't be read. I'm guessing I was meaning to fix that before I landed
the change that added the Read methods, but well, I didn't.

To fix this, rather than somehow passing FileSystemContext to BlobImpl
so that it can pass it on to the BlobReader, this changes BlobDataItem
to hold a reference to the FileSystemContext that was used to create a
blob, and then uses that for reading.

The bulk of this CL is then cleaning up places that no no longer need
a FileSystemContext.

TBR: rdevlin.cronin@chromium.org
Bug:  785412 
Change-Id: I8b0131972ca1ade7b3f3fa3f732f5125b11d17e0
Reviewed-on: https://chromium-review.googlesource.com/773199
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519287}
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/chrome/browser/extensions/app_data_migrator_unittest.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/chrome/browser/sync_file_system/local/canned_syncable_file_system.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/chrome/browser/sync_file_system/local/local_file_change_tracker_unittest.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/chrome/browser/sync_file_system/local/syncable_file_operation_runner_unittest.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/blob_storage/blob_dispatcher_host.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/blob_storage/blob_transport_host_unittest.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/blob_storage/blob_url_loader_factory.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/blob_storage/blob_url_loader_factory.h
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/blob_storage/blob_url_unittest.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/cache_storage/cache_storage_blob_to_disk_cache_unittest.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/cache_storage/cache_storage_cache_unittest.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/cache_storage/cache_storage_manager_unittest.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/devtools/devtools_io_context.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/download/resource_downloader.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/download/resource_downloader.h
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/loader/upload_data_stream_builder.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/service_worker/service_worker_browsertest.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/service_worker/service_worker_url_loader_job.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/service_worker/service_worker_url_request_job.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/service_worker/service_worker_url_request_job_unittest.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/content/browser/storage_partition_impl_map.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_data_builder.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_data_builder.h
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_data_handle.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_data_handle.h
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_data_item.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_data_item.h
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_impl.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_reader.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_reader.h
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_reader_unittest.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_registry_impl.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_registry_impl_unittest.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_storage_context.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_storage_context_unittest.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_transport_host.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_transport_host.h
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_transport_request_builder.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_transport_request_builder.h
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_transport_request_builder_unittest.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_url_request_job.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_url_request_job.h
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_url_request_job_factory.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/blob_url_request_job_factory.h
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/mojo_blob_reader.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/mojo_blob_reader.h
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/upload_blob_element_reader.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/blob/upload_blob_element_reader.h
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/fileapi/file_system_operation_impl_write_unittest.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/test/mock_blob_url_request_context.cc
[modify] https://crrev.com/80fe13f10991ce52520459df1c66f4b22fa2e896/storage/browser/test/mock_blob_url_request_context.h

Comment 2 by mek@chromium.org, Nov 27 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment