Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(511)

Issue 1287303002: [BlobAsync] Patch 1: BlobStorageRegistry (Closed)

Created:
5 years, 4 months ago by dmurph
Modified:
4 years, 6 months ago
CC:
chromium-reviews, darin-cc_chromium.org, jam
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[BlobAsync] Patch 1: BlobStorageRegistry Note: This patch is a no-op, and will be hooked up in a later patch. Patches: 1: https://codereview.chromium.org/1287303002 2: https://codereview.chromium.org/1288373002 3: https://codereview.chromium.org/1292523002 4: https://codereview.chromium.org/1098853003 Hookup: https://codereview.chromium.org/1234813004 BUG=375297 Committed: https://crrev.com/d0e06a58f51ee83a670829425cd65a5693909eee Cr-Commit-Position: refs/heads/master@{#354050}

Patch Set 1 #

Total comments: 45

Patch Set 2 : comments #

Total comments: 20

Patch Set 3 : comments #

Patch Set 4 : correction #

Total comments: 20

Patch Set 5 : comments #

Patch Set 6 : comments #

Patch Set 7 : comments #

Patch Set 8 : Added export to struct #

Unified diffs Side-by-side diffs Delta from patch set Stats (+317 lines, -0 lines) Patch
A content/browser/blob_storage/blob_storage_registry_unittest.cc View 1 2 3 4 5 6 1 chunk +87 lines, -0 lines 0 comments Download
M content/content_tests.gypi View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M storage/browser/BUILD.gn View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
A storage/browser/blob/blob_storage_registry.h View 1 2 3 4 5 6 7 1 chunk +108 lines, -0 lines 0 comments Download
A storage/browser/blob/blob_storage_registry.cc View 1 2 3 4 5 1 chunk +116 lines, -0 lines 0 comments Download
M storage/browser/blob/internal_blob_data.h View 1 chunk +1 line, -0 lines 0 comments Download
M storage/storage_browser.gyp View 1 2 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 27 (7 generated)
dmurph
Hello Michael & Kinuko: I'm splitting the giant part 1 patch into segments that are ...
5 years, 4 months ago (2015-08-13 12:53:49 UTC) #2
kinuko
looking good https://codereview.chromium.org/1287303002/diff/1/content/browser/blob_storage/blob_storage_registry_unittest.cc File content/browser/blob_storage/blob_storage_registry_unittest.cc (right): https://codereview.chromium.org/1287303002/diff/1/content/browser/blob_storage/blob_storage_registry_unittest.cc#newcode63 content/browser/blob_storage/blob_storage_registry_unittest.cc:63: BlobRegistryEntry* entry = registry.RegisterBlobUUID(kBlob); nit: I'd put ...
5 years, 4 months ago (2015-08-14 10:12:19 UTC) #3
michaeln
thnx for spitting the patch up, i have no excuse for being so slow to ...
5 years, 4 months ago (2015-08-18 02:25:47 UTC) #4
dmurph
Hey guys, Sorry this took so long, I was working on the reader patch which ...
5 years, 3 months ago (2015-09-15 00:54:35 UTC) #5
dmurph
Ping on this patch :)
5 years, 2 months ago (2015-09-30 17:54:26 UTC) #6
michaeln
https://codereview.chromium.org/1287303002/diff/1/storage/browser/blob/blob_storage_registry.cc File storage/browser/blob/blob_storage_registry.cc (right): https://codereview.chromium.org/1287303002/diff/1/storage/browser/blob/blob_storage_registry.cc#newcode51 storage/browser/blob/blob_storage_registry.cc:51: flags &= !flag; On 2015/09/15 00:54:34, dmurph wrote: > ...
5 years, 2 months ago (2015-10-09 02:16:14 UTC) #7
dmurph
Done! Thanks for the comments! https://codereview.chromium.org/1287303002/diff/1/storage/browser/blob/blob_storage_registry.cc File storage/browser/blob/blob_storage_registry.cc (right): https://codereview.chromium.org/1287303002/diff/1/storage/browser/blob/blob_storage_registry.cc#newcode51 storage/browser/blob/blob_storage_registry.cc:51: flags &= !flag; On ...
5 years, 2 months ago (2015-10-10 00:11:33 UTC) #8
kinuko
Sorry for super slow review! https://codereview.chromium.org/1287303002/diff/60001/storage/browser/blob/blob_storage_registry.cc File storage/browser/blob/blob_storage_registry.cc (right): https://codereview.chromium.org/1287303002/diff/60001/storage/browser/blob/blob_storage_registry.cc#newcode105 storage/browser/blob/blob_storage_registry.cc:105: } nit: some one-line ...
5 years, 2 months ago (2015-10-12 14:34:59 UTC) #9
michaeln
lgtm https://codereview.chromium.org/1287303002/diff/60001/storage/browser/blob/blob_storage_registry.h File storage/browser/blob/blob_storage_registry.h (right): https://codereview.chromium.org/1287303002/diff/60001/storage/browser/blob/blob_storage_registry.h#newcode31 storage/browser/blob/blob_storage_registry.h:31: UNKNOWN = 0, Thnx for flattening the helpers ...
5 years, 2 months ago (2015-10-12 19:50:14 UTC) #10
dmurph
https://codereview.chromium.org/1287303002/diff/60001/storage/browser/blob/blob_storage_registry.cc File storage/browser/blob/blob_storage_registry.cc (right): https://codereview.chromium.org/1287303002/diff/60001/storage/browser/blob/blob_storage_registry.cc#newcode105 storage/browser/blob/blob_storage_registry.cc:105: } On 2015/10/12 at 14:34:58, kinuko wrote: > nit: ...
5 years, 2 months ago (2015-10-12 20:08:28 UTC) #11
michaeln
https://codereview.chromium.org/1287303002/diff/60001/storage/browser/blob/blob_storage_registry.h File storage/browser/blob/blob_storage_registry.h (right): https://codereview.chromium.org/1287303002/diff/60001/storage/browser/blob/blob_storage_registry.h#newcode31 storage/browser/blob/blob_storage_registry.h:31: UNKNOWN = 0, On 2015/10/12 20:08:28, dmurph wrote: > ...
5 years, 2 months ago (2015-10-12 22:17:14 UTC) #12
dmurph
On 2015/10/12 at 22:17:14, michaeln wrote: > https://codereview.chromium.org/1287303002/diff/60001/storage/browser/blob/blob_storage_registry.h > File storage/browser/blob/blob_storage_registry.h (right): > > https://codereview.chromium.org/1287303002/diff/60001/storage/browser/blob/blob_storage_registry.h#newcode31 ...
5 years, 2 months ago (2015-10-12 22:53:39 UTC) #13
michaeln
On 2015/10/12 22:53:39, dmurph wrote: > On 2015/10/12 at 22:17:14, michaeln wrote: > > > ...
5 years, 2 months ago (2015-10-13 00:18:09 UTC) #14
kinuko
lgtm I'm still not sure if we need exceeded_memory in this CL as no one ...
5 years, 2 months ago (2015-10-13 08:20:18 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1287303002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1287303002/120001
5 years, 2 months ago (2015-10-14 00:14:26 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_compile_dbg_ng/builds/95112)
5 years, 2 months ago (2015-10-14 00:43:15 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1287303002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1287303002/140001
5 years, 2 months ago (2015-10-14 16:25:41 UTC) #23
commit-bot: I haz the power
Committed patchset #8 (id:140001)
5 years, 2 months ago (2015-10-14 17:25:35 UTC) #24
commit-bot: I haz the power
Patchset 8 (id:??) landed as https://crrev.com/d0e06a58f51ee83a670829425cd65a5693909eee Cr-Commit-Position: refs/heads/master@{#354050}
5 years, 2 months ago (2015-10-14 17:26:33 UTC) #25
CapuchinomoccaZ
4 years, 6 months ago (2016-06-01 14:45:32 UTC) #27
Message was sent while issue was closed.
lgtm

อีเมลและกูเกิลเพลหยุด ทำงาน

Powered by Google App Engine
This is Rietveld 408576698