New issue
Advanced search Search tips

Issue 803585 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Refactor browser side blob code to no longer depend on network::DataElement

Project Member Reported by mek@chromium.org, Jan 18 2018

Issue description

Currently network::DataElement is used for two different purposes, making its API slightly confusing. The blob code could instead just inline whatever parts of DataElement it uses into BlobDataItem, eliminating one layer of indirection.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 18 2018

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 19 2018

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

commit 4a83eec0ca6bee3d742d4b4eafb2b90245966f10
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Fri Jan 19 21:09:22 2018

Change how ownership of BlobDataBuilder works.

Before the caller of BuildBlob/AddFinishedBlob retained ownership of the
BlobDataBuilder, only passing in a reference. After this change instead
ownership is passed to BlobStorageContext.

This change will help with some follow-up refactoring of BlobDataBuilder.

TBR=kinuko@chromium.org, xingliu@chromium.org

Bug:  803585 
Change-Id: Iccbad9b3ecd3c6e510afc9fe6e65c34a2c738dbf
Reviewed-on: https://chromium-review.googlesource.com/876499
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530610}
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/components/download/internal/blob_task_proxy.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/content/browser/background_fetch/background_fetch_data_manager.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/content/browser/blob_storage/blob_url_unittest.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/content/browser/blob_storage/chrome_blob_storage_context.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/content/browser/cache_storage/cache_storage_blob_to_disk_cache_unittest.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/content/browser/cache_storage/cache_storage_cache.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/content/browser/cache_storage/cache_storage_cache_unittest.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/content/browser/cache_storage/cache_storage_manager_unittest.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/content/browser/indexed_db/indexed_db_dispatcher_host.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/content/browser/loader/upload_data_stream_builder_unittest.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/content/browser/service_worker/service_worker_url_loader_job_unittest.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/content/browser/service_worker/service_worker_url_request_job.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/content/browser/service_worker/service_worker_url_request_job_unittest.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/storage/browser/blob/blob_data_builder.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/storage/browser/blob/blob_data_builder.h
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/storage/browser/blob/blob_data_snapshot.h
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/storage/browser/blob/blob_flattener_unittest.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/storage/browser/blob/blob_impl_unittest.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/storage/browser/blob/blob_memory_controller_unittest.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/storage/browser/blob/blob_reader_unittest.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/storage/browser/blob/blob_registry_impl.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/storage/browser/blob/blob_registry_impl_unittest.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/storage/browser/blob/blob_storage_context.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/storage/browser/blob/blob_storage_context.h
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/storage/browser/blob/blob_storage_context_unittest.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/storage/browser/blob/blob_url_store_impl_unittest.cc
[modify] https://crrev.com/4a83eec0ca6bee3d742d4b4eafb2b90245966f10/storage/browser/test/mock_blob_url_request_context.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 19 2018

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

commit a23805be7cc445a07a96a235706e509b6c298647
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Fri Jan 19 23:27:17 2018

Merge BlobFlattener and BlobSlice into BlobDataBuilder.

This paves the way for getting rid of TYPE_BLOB elements (or
at least not having to reintroduce TYPE_BLOB when BlobDataItem
is updated to no longer depend on network::DataElement).

TBR=kinuko@chromium.org

Bug:  803585 
Change-Id: I344b7a16a1686059a2b4058cc46711444f96f8f5
Reviewed-on: https://chromium-review.googlesource.com/876527
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530655}
[modify] https://crrev.com/a23805be7cc445a07a96a235706e509b6c298647/content/browser/service_worker/service_worker_url_request_job.cc
[modify] https://crrev.com/a23805be7cc445a07a96a235706e509b6c298647/services/network/public/cpp/data_element.cc
[modify] https://crrev.com/a23805be7cc445a07a96a235706e509b6c298647/storage/browser/blob/blob_data_builder.cc
[modify] https://crrev.com/a23805be7cc445a07a96a235706e509b6c298647/storage/browser/blob/blob_data_builder.h
[modify] https://crrev.com/a23805be7cc445a07a96a235706e509b6c298647/storage/browser/blob/blob_data_builder_unittest.cc
[modify] https://crrev.com/a23805be7cc445a07a96a235706e509b6c298647/storage/browser/blob/blob_entry.cc
[modify] https://crrev.com/a23805be7cc445a07a96a235706e509b6c298647/storage/browser/blob/blob_entry.h
[modify] https://crrev.com/a23805be7cc445a07a96a235706e509b6c298647/storage/browser/blob/blob_flattener_unittest.cc
[modify] https://crrev.com/a23805be7cc445a07a96a235706e509b6c298647/storage/browser/blob/blob_memory_controller_unittest.cc
[modify] https://crrev.com/a23805be7cc445a07a96a235706e509b6c298647/storage/browser/blob/blob_registry_impl.cc
[modify] https://crrev.com/a23805be7cc445a07a96a235706e509b6c298647/storage/browser/blob/blob_slice_unittest.cc
[modify] https://crrev.com/a23805be7cc445a07a96a235706e509b6c298647/storage/browser/blob/blob_storage_context.cc
[modify] https://crrev.com/a23805be7cc445a07a96a235706e509b6c298647/storage/browser/blob/blob_storage_context.h
[modify] https://crrev.com/a23805be7cc445a07a96a235706e509b6c298647/storage/browser/blob/blob_storage_context_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 23 2018

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

commit 5e06947bdc9a7d4f54d2ded8c09d17c02d84833e
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Tue Jan 23 00:32:08 2018

Stop using network::DataElement in BlobDataItem.

Instead inline the parts of DataElement that are used by BlobDataItem.

TBR=xingliu@chromium.org, jsbell@chromium.org

Bug:  803585 
Change-Id: Idab8a6ca7d2756f8a90065586da1a358ac476197
Reviewed-on: https://chromium-review.googlesource.com/876991
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531086}
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/components/download/internal/blob_task_proxy.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/content/browser/cache_storage/cache_storage_cache_unittest.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/content/browser/service_worker/service_worker_browsertest.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/README.md
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/blob_data_builder.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/blob_data_builder.h
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/blob_data_builder_unittest.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/blob_data_item.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/blob_data_item.h
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/blob_data_snapshot.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/blob_entry.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/blob_flattener_unittest.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/blob_memory_controller.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/blob_memory_controller_unittest.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/blob_reader.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/blob_registry_impl_unittest.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/blob_slice_unittest.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/blob_storage_context.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/shareable_blob_data_item.cc
[modify] https://crrev.com/5e06947bdc9a7d4f54d2ded8c09d17c02d84833e/storage/browser/blob/view_blob_internals_job.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 23 2018

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

commit 5ed1942e0d49397f14d1df21b6da622e7bbf3ed2
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Tue Jan 23 18:09:33 2018

Remove no longer used DataElement::Type values.

The blob system no longer uses network::DataElement, leaving these types
no longer used. Also clean up some left-over other usages (that
presumably couldn't be reached anymore) of TYPE_FILE_FILESYSTEM.

Bug:  803585 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Icac28f4f3b1c52b1a95bf13011e85a871cc1b633
Reviewed-on: https://chromium-review.googlesource.com/877185
Reviewed-by: Patrick Noland <pnoland@google.com>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531280}
[modify] https://crrev.com/5ed1942e0d49397f14d1df21b6da622e7bbf3ed2/content/browser/child_process_security_policy_impl.cc
[modify] https://crrev.com/5ed1942e0d49397f14d1df21b6da622e7bbf3ed2/content/browser/loader/upload_data_stream_builder.cc
[modify] https://crrev.com/5ed1942e0d49397f14d1df21b6da622e7bbf3ed2/content/common/page_state.mojom
[modify] https://crrev.com/5ed1942e0d49397f14d1df21b6da622e7bbf3ed2/content/common/page_state_serialization.cc
[modify] https://crrev.com/5ed1942e0d49397f14d1df21b6da622e7bbf3ed2/content/common/page_state_serialization_unittest.cc
[modify] https://crrev.com/5ed1942e0d49397f14d1df21b6da622e7bbf3ed2/content/network/url_loader.cc
[modify] https://crrev.com/5ed1942e0d49397f14d1df21b6da622e7bbf3ed2/content/renderer/loader/web_url_request_util.cc
[modify] https://crrev.com/5ed1942e0d49397f14d1df21b6da622e7bbf3ed2/services/network/public/cpp/data_element.cc
[modify] https://crrev.com/5ed1942e0d49397f14d1df21b6da622e7bbf3ed2/services/network/public/cpp/data_element.h
[modify] https://crrev.com/5ed1942e0d49397f14d1df21b6da622e7bbf3ed2/services/network/public/cpp/network_param_ipc_traits.cc
[modify] https://crrev.com/5ed1942e0d49397f14d1df21b6da622e7bbf3ed2/services/network/public/cpp/resource_request_body.cc
[modify] https://crrev.com/5ed1942e0d49397f14d1df21b6da622e7bbf3ed2/services/network/public/cpp/resource_request_body.h
[modify] https://crrev.com/5ed1942e0d49397f14d1df21b6da622e7bbf3ed2/storage/browser/blob/blob_data_builder.cc
[modify] https://crrev.com/5ed1942e0d49397f14d1df21b6da622e7bbf3ed2/third_party/WebKit/public/platform/WebHTTPBody.h
[modify] https://crrev.com/5ed1942e0d49397f14d1df21b6da622e7bbf3ed2/tools/ipc_fuzzer/fuzzer/fuzzer.cc

Comment 6 by mek@chromium.org, Jan 23 2018

Status: Fixed (was: Started)

Sign in to add a comment