New issue
Advanced search Search tips

Issue 756743 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug
Proj-Servicification

Blocked on:
issue 800901

Blocking:
issue 611935



Sign in to add a comment

[mojo-blobs] Support registering Blob URL via Mojo interface

Project Member Reported by kinuko@chromium.org, Aug 18 2017

Issue description

Currently it is only supported via legacy IPC, which means we need to have a reference (i.e. BlobDispatcherHost::blobs_inuse_map_) in BlobDispatcherHost before registering it, which complicates Blob lifetime issue a lot.
 

Comment 1 by kinuko@chromium.org, Aug 18 2017

Blocking: 715640
Cc: mek@chromium.org dmu...@chromium.org
(I looked for existing bugs but wasn't able to find it so filing this one. Please feel free to merge otherwise)

Comment 2 by kinuko@chromium.org, Aug 18 2017

Blocking: 611935
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 29 2017

Comment 6 by laforge@google.com, Nov 7 2017

Components: Internals>Network>Service

Comment 7 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.

Comment 8 by laforge@google.com, Nov 8 2017

Components: Internals>Services>Storage
Setting Internals>Services>Storage to all children of issue 611935
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 13 2017

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

commit 19ecd2fccbfd489b55422ee7fc142434799f30cd
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Wed Dec 13 08:57:55 2017

Remove logic to revoke URLs by UUID of the pointed to object.

These methods are never called, so get rid of them to make future
refactoring and cleanup easier. Also change the map in PublicURLManager
to now map from URL to Registry, rather than the other way around.

Bug:  756743 
Change-Id: I1a7fec77f498b11c916fce1adc520ffda9268545
Reviewed-on: https://chromium-review.googlesource.com/822153
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523724}
[modify] https://crrev.com/19ecd2fccbfd489b55422ee7fc142434799f30cd/third_party/WebKit/Source/core/fileapi/URLFileAPI.cpp
[modify] https://crrev.com/19ecd2fccbfd489b55422ee7fc142434799f30cd/third_party/WebKit/Source/core/html/PublicURLManager.cpp
[modify] https://crrev.com/19ecd2fccbfd489b55422ee7fc142434799f30cd/third_party/WebKit/Source/core/html/PublicURLManager.h
[modify] https://crrev.com/19ecd2fccbfd489b55422ee7fc142434799f30cd/third_party/WebKit/Source/core/url/DOMURL.cpp
[modify] https://crrev.com/19ecd2fccbfd489b55422ee7fc142434799f30cd/third_party/WebKit/Source/core/url/DOMURL.h

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 8 2018

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

commit 5af715b5e1ddba56932bfbdf59ccd42c692ec656
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Mon Jan 08 20:38:05 2018

Change the mojo interface for creating/revoking Blob URLs.

Also lays the groundwork for making the web API use the mojo interface.
Doesn't actually switch over to the mojo interface for this yet though
since that will requiring refactoring how blob URLs are resolved/loaded
first.

Bug:  756743 
Change-Id: I8f303fe11aef1359fee1033b30b2169f534ecd9a
Reviewed-on: https://chromium-review.googlesource.com/823828
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527743}
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/storage/browser/BUILD.gn
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/storage/browser/blob/blob_registry_impl.cc
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/storage/browser/blob/blob_registry_impl.h
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/storage/browser/blob/blob_registry_impl_unittest.cc
[add] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/storage/browser/blob/blob_url_store_impl.cc
[add] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/storage/browser/blob/blob_url_store_impl.h
[add] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/storage/browser/blob/blob_url_store_impl_unittest.cc
[add] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/storage/browser/test/mock_blob_registry_delegate.cc
[add] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/storage/browser/test/mock_blob_registry_delegate.h
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/third_party/WebKit/Source/core/fileapi/Blob.cpp
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/third_party/WebKit/Source/core/fileapi/Blob.h
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/third_party/WebKit/Source/core/html/PublicURLManager.cpp
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/third_party/WebKit/Source/core/html/PublicURLManager.h
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/third_party/WebKit/Source/core/html/URLRegistry.h
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/third_party/WebKit/Source/core/url/DOMURL.cpp
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/third_party/WebKit/Source/platform/blob/BlobData.cpp
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/third_party/WebKit/Source/platform/blob/BlobData.h
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/third_party/WebKit/Source/platform/blob/BlobDataTest.cpp
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/third_party/WebKit/Source/platform/runtime_enabled_features.json5
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/third_party/WebKit/common/BUILD.gn
[modify] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/third_party/WebKit/common/blob/blob_registry.mojom
[add] https://crrev.com/5af715b5e1ddba56932bfbdf59ccd42c692ec656/third_party/WebKit/common/blob/blob_url_store.mojom

Comment 11 by mek@chromium.org, Jan 10 2018

Blockedon: 800901
Blocking: -715640
Project Member

Comment 13 by bugdroid1@chromium.org, Jan 17 2018

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

commit 12a9e0ffab09714a20d74bb73ddad343fb9028c8
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Wed Jan 17 22:07:23 2018

Make all blink::BlobRegistry methods private.

With mojofied blob URLs the methods in this class won't work anymore, so
to prevent new usage of them from being introduced mark them as private
with friend declarations for existing usage.

Bug:  756743 
Change-Id: Idf047d772684c25f821d74952c7a38611e7b89de
Reviewed-on: https://chromium-review.googlesource.com/871212
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529909}
[modify] https://crrev.com/12a9e0ffab09714a20d74bb73ddad343fb9028c8/third_party/WebKit/Source/core/fileapi/Blob.cpp
[modify] https://crrev.com/12a9e0ffab09714a20d74bb73ddad343fb9028c8/third_party/WebKit/Source/platform/blob/BlobRegistry.h

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 24 2018

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

commit c1f7232f5d009dfe04f12d99801cf9b007f18131
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Wed Jan 24 22:36:43 2018

Move URLOriginMap to PublicURLManager rather than BlobRegistry.

With the future mojo code path BlobRegistry will no longer be involved in
blob URLs, so this makes sure that even in that case URLOriginMap is still
properly populated.

Also some refactoring of Blob related test helper code to make it possible
to reuse some of that in PublicURLManager tests.

Bug:  756743 
Change-Id: I80dd0ea551a3e05134995a380b118be48b11571c
Reviewed-on: https://chromium-review.googlesource.com/832936
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531724}
[modify] https://crrev.com/c1f7232f5d009dfe04f12d99801cf9b007f18131/third_party/WebKit/Source/core/fileapi/BUILD.gn
[modify] https://crrev.com/c1f7232f5d009dfe04f12d99801cf9b007f18131/third_party/WebKit/Source/core/fileapi/PublicURLManager.cpp
[modify] https://crrev.com/c1f7232f5d009dfe04f12d99801cf9b007f18131/third_party/WebKit/Source/core/fileapi/PublicURLManager.h
[add] https://crrev.com/c1f7232f5d009dfe04f12d99801cf9b007f18131/third_party/WebKit/Source/core/fileapi/PublicURLManagerTest.cpp
[modify] https://crrev.com/c1f7232f5d009dfe04f12d99801cf9b007f18131/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/c1f7232f5d009dfe04f12d99801cf9b007f18131/third_party/WebKit/Source/platform/blob/BUILD.gn
[modify] https://crrev.com/c1f7232f5d009dfe04f12d99801cf9b007f18131/third_party/WebKit/Source/platform/blob/BlobDataTest.cpp
[modify] https://crrev.com/c1f7232f5d009dfe04f12d99801cf9b007f18131/third_party/WebKit/Source/platform/blob/BlobRegistry.cpp
[add] https://crrev.com/c1f7232f5d009dfe04f12d99801cf9b007f18131/third_party/WebKit/Source/platform/blob/testing/FakeBlob.cpp
[add] https://crrev.com/c1f7232f5d009dfe04f12d99801cf9b007f18131/third_party/WebKit/Source/platform/blob/testing/FakeBlob.h
[add] https://crrev.com/c1f7232f5d009dfe04f12d99801cf9b007f18131/third_party/WebKit/Source/platform/blob/testing/FakeBlobRegistry.cpp
[add] https://crrev.com/c1f7232f5d009dfe04f12d99801cf9b007f18131/third_party/WebKit/Source/platform/blob/testing/FakeBlobRegistry.h
[add] https://crrev.com/c1f7232f5d009dfe04f12d99801cf9b007f18131/third_party/WebKit/Source/platform/blob/testing/FakeBlobURLStore.cpp
[add] https://crrev.com/c1f7232f5d009dfe04f12d99801cf9b007f18131/third_party/WebKit/Source/platform/blob/testing/FakeBlobURLStore.h

Project Member

Comment 15 by bugdroid1@chromium.org, Jan 26 2018

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

commit d465a90d3c443b5579127f852eddefb3de5dd083
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Fri Jan 26 19:21:36 2018

Better deal with invalid blob URLs in future mojo code path.

BlobEntryRegistry will DCHECK when presented with invalid blob URLs,
so both make sure the renderer doesn't send such URLs, and kill the
renderer if it does happen to do so.

Bug:  756743 

Change-Id: Ife653cc4d8687fc30c8a03336caa66905951556b
Reviewed-on: https://chromium-review.googlesource.com/884522
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532036}
[modify] https://crrev.com/d465a90d3c443b5579127f852eddefb3de5dd083/storage/browser/BUILD.gn
[modify] https://crrev.com/d465a90d3c443b5579127f852eddefb3de5dd083/storage/browser/blob/blob_storage_registry.cc
[modify] https://crrev.com/d465a90d3c443b5579127f852eddefb3de5dd083/storage/browser/blob/blob_url_store_impl.cc
[modify] https://crrev.com/d465a90d3c443b5579127f852eddefb3de5dd083/storage/browser/blob/blob_url_store_impl_unittest.cc
[add] https://crrev.com/d465a90d3c443b5579127f852eddefb3de5dd083/storage/browser/blob/blob_url_utils.cc
[add] https://crrev.com/d465a90d3c443b5579127f852eddefb3de5dd083/storage/browser/blob/blob_url_utils.h
[modify] https://crrev.com/d465a90d3c443b5579127f852eddefb3de5dd083/third_party/WebKit/Source/core/fileapi/PublicURLManager.cpp
[modify] https://crrev.com/d465a90d3c443b5579127f852eddefb3de5dd083/third_party/WebKit/Source/core/fileapi/PublicURLManagerTest.cpp
[modify] https://crrev.com/d465a90d3c443b5579127f852eddefb3de5dd083/third_party/WebKit/Source/platform/blob/testing/FakeBlobURLStore.cpp
[modify] https://crrev.com/d465a90d3c443b5579127f852eddefb3de5dd083/third_party/WebKit/Source/platform/blob/testing/FakeBlobURLStore.h

Comment 16 by dxie@chromium.org, May 17 2018

Labels: -Pri-2 Proj-Servicification-Canary OS-All Pri-1
Owner: mek@chromium.org
Status: Assigned (was: Available)

Comment 17 by dxie@chromium.org, May 18 2018

Labels: -OS-All OS-Windows OS-Linux OS-Mac OS-Chrome OS-Android

Comment 18 by mek@chromium.org, May 31 2018

Status: Fixed (was: Assigned)
This is pretty much done actually. Registering and unregistering blob URLs via the mojo interface is possible, it just isn't actually done yet by blink since doing that needs changes in the loading code (that are fully implemented with network service enabled, but still need some work when network service isn't enabled). That work is tracked in 800901

Sign in to add a comment