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

Issue 18023022: Blob support for IDB [Chromium] (Closed)

Created:
7 years, 5 months ago by ericu
Modified:
6 years, 6 months ago
CC:
chromium-reviews, vandebo (ex-Chrome), Lei Zhang, tzik+watch_chromium.org, jam, joi+watch-content_chromium.org, tommycli, darin-cc_chromium.org, Greg Billock, kinuko+watch, cmumford
Visibility:
Public.

Description

This is the master CL that contains the whole remaining Chromium side. See https://codereview.chromium.org/18590006/ for the Blink side. See https://docs.google.com/a/chromium.org/document/d/1Kdr4pcFt4QBDLLQn-fY4kZgw6ptmK23lthGZdQMVh2Y/edit for the big picture document [chromium.org account required]. Read-only view at https://docs.google.com/document/d/1Kdr4pcFt4QBDLLQn-fY4kZgw6ptmK23lthGZdQMVh2Y/pub [no account required]. BUG=108012 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=274685

Patch Set 1 #

Patch Set 2 : Merged out; still haven't compiled #

Patch Set 3 : some compile fixes #

Patch Set 4 : It builds, with lots stubbed out. #

Patch Set 5 : Removed stubs, some hacks. Compiles, untested. #

Patch Set 6 : Running with new backend! #

Patch Set 7 : Fixed journal encoding bug #

Patch Set 8 : Cleanup: refactor out WriteStateHolder #

Patch Set 9 : Change ChainedBlobWriter to RefCounted #

Patch Set 10 : Fixed ABR lifetime, added back lost secondary journal cleanup [dropped in migration], style cleanu… #

Patch Set 11 : Implemented ReportBlobUnused #

Patch Set 12 : Add IndexedDBValue wrapper; make IDBActiveBlobRegistry store booleans. #

Patch Set 13 : Look up blob info in GetRecord; clean up IDBBlobInfo. #

Patch Set 14 : Add missing files, de-inline some heavy constructors. #

Patch Set 15 : non-compiling checkpoint #

Patch Set 16 : It compiles again. #

Patch Set 17 : Simplify closures for blob creation #

Patch Set 18 : Incomplete work toward registering blobs #

Patch Set 19 : Blob registration code compiles, untested. #

Patch Set 20 : Add in browser-side blob creation. #

Patch Set 21 : Blobs work again; Files still WIP. #

Patch Set 22 : Progress on writing files. #

Patch Set 23 : Fixed file timestamps. #

Patch Set 24 : Progress on passing blob data out from IPCs. #

Patch Set 25 : Got the rest of the data passed through. #

Patch Set 26 : Working blob round-trip. #

Patch Set 27 : Fix File metadata; Files now round-trip as well. #

Patch Set 28 : Fix constructor bug #

Patch Set 29 : Keepalive seems to be working. #

Patch Set 30 : Checkpointing debugging progress; 2-3 bugs fixed, another marked. #

Patch Set 31 : Secondary journal works. #

Patch Set 32 : Add timer hacks to fix refcounting and do more efficient journal cleanup. #

Patch Set 33 : Minor cleanup, simplifying IndexedDBActiveBlobRegistry. #

Patch Set 34 : Lots of small cleanups, implemented blob write abort. #

Patch Set 35 : More TODOs cleaned up; secondary journal now cleaned on boot. #

Patch Set 36 : Prefetch file metadata #

Patch Set 37 : Fix shutdown crash. #

Patch Set 38 : Remove empty blob entries. #

Patch Set 39 : Small cleanup. #

Patch Set 40 : Merged out [untested] #

Patch Set 41 : Merge fixes #

Patch Set 42 : Merge fixes [builds, untested] #

Total comments: 39

Patch Set 43 : merged out paternity leave changes--untested, uncompiled #

Patch Set 44 : two merges no compile #

Patch Set 45 : Checkpoint work toward compiling. #

Patch Set 46 : Compiles with hack, untested. #

Patch Set 47 : working but leaking blobs #

Patch Set 48 : New blob creation method in, but leaky. #

Patch Set 49 : Leak fixed via backchannel to ack blobs. #

Patch Set 50 : Some code review feedback. #

Patch Set 51 : Change UUID to be an 8-bit std::string. #

Patch Set 52 : Added a comment. #

Patch Set 53 : Make sure blobs aren't GCed while in use; fix a resource leak. #

Patch Set 54 : Remove some fprintfs #

Patch Set 55 : Merged out #

Patch Set 56 : small cleanup #

Patch Set 57 : Handle IndexedDBBackingStore/Factory lifetime issue. #

Patch Set 58 : Settle on one name for the live blob journal. #

Total comments: 4

Patch Set 59 : Starting work on existing unit tests. #

Patch Set 60 : All existing unit tests now work. #

Patch Set 61 : fix bugs found by layout tests #

Patch Set 62 : small cleanup #

Patch Set 63 : More test improvements. #

Patch Set 64 : New lifetime management for ActiveBlobRegistry/BackingStore/Factory #

Patch Set 65 : Fix bugs in new lifetime code #

Patch Set 66 : fix race condition #

Patch Set 67 : Minor cleanup #

Patch Set 68 : Added blob read/write round-trip unit test #

Patch Set 69 : Add test for the live blob journal #

Patch Set 70 : Clean up IndexedDBTransaction #

Patch Set 71 : Use ScopedVector and stl_utils for BlobDataHandles. #

Total comments: 74

Patch Set 72 : small cleanup #

Total comments: 5

Patch Set 73 : Checkpointing work on code review feedback [more to come]. #

Patch Set 74 : Addressed most of Josh's feedback. #

Patch Set 75 : Handle the rest of Josh's feedback. #

Total comments: 29

Patch Set 76 : merged out--haven't compiled yet #

Patch Set 77 : build fixes #

Patch Set 78 : compiles; untested #

Patch Set 79 : Build fixes for content_unittests. #

Patch Set 80 : A missing line caused a renderer crashing bug. Fixed. #

Patch Set 81 : Fix content_unittests. #

Patch Set 82 : Remove unnecessary Compare member from BlobEntryKey. #

Patch Set 83 : More code review feedback rolled in. #

Patch Set 84 : Test fixes, more code review feedback changes. #

Patch Set 85 : Turn a refptr raw. #

Patch Set 86 : Directory naming tweaks [code + comments]. #

Patch Set 87 : WIP for an ActiveBlobRegistry unit test #

Patch Set 88 : active blob registry unit test complete #

Patch Set 89 : Bump schema version, improve metadata init. #

Patch Set 90 : Add wrapper class IndexedDBPendingConnection #

Patch Set 91 : Merged out #

Patch Set 92 : Convert some boolean functions to leveldb::Status. #

Patch Set 93 : Formatting fixes. #

Patch Set 94 : Merge back small fixes. #

Patch Set 95 : #

Patch Set 96 : Merged out #

Patch Set 97 : Merged out #

Patch Set 98 : Merged out #

Patch Set 99 : Fix some merge bugs. #

Patch Set 100 : Pull out extra TaskRunner paramst #

Patch Set 101 : Merged out #

Patch Set 102 : Merged out #

Patch Set 103 : Add permissions check for the file path the renderer sends. #

Patch Set 104 : Merged out. #

Patch Set 105 : Nit fixes #

Patch Set 106 : Merged out #

Patch Set 107 : Merged out #

Patch Set 108 : Merged out #

Patch Set 109 : Merged out #

Patch Set 110 : Merged out, some tiny tweaks + comments. #

Patch Set 111 : Added incognito support and support for reading uncommitted blobs. #

Patch Set 112 : Fix bad test pattern. #

Patch Set 113 : Merged out [untested]. #

Patch Set 114 : Fix merge #

Patch Set 115 : WIP on cleaning up blob journal encode/decode. #

Patch Set 116 : Tests for journal encode/decode. #

Patch Set 117 : Merged out #

Patch Set 118 : Undo obsolete changes #

Patch Set 119 : Merged out #

Patch Set 120 : Cleanup, formatting #

Patch Set 121 : Merge fixes #

Patch Set 122 : Merged out, plus DeleteRange WIP with failing tests. #

Patch Set 123 : Fix some tests; more still broken. #

Patch Set 124 : DeleteRange now seems to work. #

Total comments: 4

Patch Set 125 : Rolled in Josh's feedback #

Patch Set 126 : Added one DeleteRange test. #

Patch Set 127 : More tests working. #

Patch Set 128 : Merged out. #

Patch Set 129 : Merged out #

Patch Set 130 : More DeleteRange tests, cleanup. #

Patch Set 131 : Minor cleanup. #

Patch Set 132 : Nit fix. #

Patch Set 133 : Merged out #

Patch Set 134 : Repair branch #

Patch Set 135 : Build fixes #

Patch Set 136 : More small build fixes. #

Total comments: 54

Patch Set 137 : Yet still more build fixes. #

Total comments: 2

Patch Set 138 : Code review feedback, build fix fix #

Patch Set 139 : Nit fixes. #

Patch Set 140 : Test build fix #

Patch Set 141 : Merged out--bot failed due to staleness. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1068 lines, -121 lines) Patch
M content/browser/indexed_db/indexed_db_backing_store.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 4 chunks +17 lines, -2 lines 0 comments Download
M content/browser/indexed_db/indexed_db_backing_store.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 10 chunks +302 lines, -46 lines 0 comments Download
M content/browser/indexed_db/indexed_db_backing_store_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 6 chunks +632 lines, -10 lines 0 comments Download
M content/browser/indexed_db/indexed_db_context_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 2 chunks +4 lines, -1 line 0 comments Download
M content/browser/indexed_db/indexed_db_database.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 2 chunks +8 lines, -27 lines 0 comments Download
M content/browser/indexed_db/indexed_db_database_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 3 chunks +5 lines, -3 lines 0 comments Download
M content/browser/indexed_db/indexed_db_fake_backing_store.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 2 chunks +5 lines, -4 lines 0 comments Download
M content/browser/indexed_db/indexed_db_fake_backing_store.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 2 chunks +18 lines, -14 lines 0 comments Download
M content/browser/indexed_db/indexed_db_leveldb_coding.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 1 chunk +1 line, -1 line 0 comments Download
M content/browser/indexed_db/indexed_db_transaction.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 4 chunks +10 lines, -3 lines 0 comments Download
M content/browser/indexed_db/indexed_db_transaction.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 7 chunks +55 lines, -2 lines 0 comments Download
M content/browser/indexed_db/indexed_db_transaction_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 6 chunks +7 lines, -7 lines 0 comments Download
M content/browser/indexed_db/leveldb/leveldb_comparator.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 1 chunk +2 lines, -1 line 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 52 (0 generated)
jsbell
Just some initial observations; I haven't gone through everything. https://codereview.chromium.org/18023022/diff/178001/content/browser/indexed_db/indexed_db_active_blob_registry.cc File content/browser/indexed_db/indexed_db_active_blob_registry.cc (right): https://codereview.chromium.org/18023022/diff/178001/content/browser/indexed_db/indexed_db_active_blob_registry.cc#newcode30 content/browser/indexed_db/indexed_db_active_blob_registry.cc:30: ...
7 years, 3 months ago (2013-09-13 00:12:20 UTC) #1
ericu
It's not fully baked yet, but I've repaired the bit rot and done a bunch ...
7 years, 1 month ago (2013-11-20 23:05:38 UTC) #2
jsbell
https://codereview.chromium.org/18023022/diff/178001/content/browser/indexed_db/indexed_db_leveldb_coding.cc File content/browser/indexed_db/indexed_db_leveldb_coding.cc (right): https://codereview.chromium.org/18023022/diff/178001/content/browser/indexed_db/indexed_db_leveldb_coding.cc#newcode1887 content/browser/indexed_db/indexed_db_leveldb_coding.cc:1887: kSpecialIndexNumber + 1)); Got it, I wasn't paying attention ...
7 years, 1 month ago (2013-11-21 22:54:31 UTC) #3
ericu
https://codereview.chromium.org/18023022/diff/178001/content/browser/indexed_db/indexed_db_backing_store.cc File content/browser/indexed_db/indexed_db_backing_store.cc (right): https://codereview.chromium.org/18023022/diff/178001/content/browser/indexed_db/indexed_db_backing_store.cc#newcode2267 content/browser/indexed_db/indexed_db_backing_store.cc:2267: FROM_HERE, base::TimeDelta::FromSeconds(5), this, On 2013/09/13 00:12:21, jsbell wrote: > ...
7 years ago (2013-12-02 23:35:18 UTC) #4
jsbell
https://codereview.chromium.org/18023022/diff/625001/content/browser/indexed_db/indexed_db_factory.cc File content/browser/indexed_db/indexed_db_factory.cc (right): https://codereview.chromium.org/18023022/diff/625001/content/browser/indexed_db/indexed_db_factory.cc#newcode94 content/browser/indexed_db/indexed_db_factory.cc:94: ptr->active_blob_registry()->ForceShutdown(); Hrm, unfortunate placement (in a Has.... method). If ...
7 years ago (2013-12-03 00:10:02 UTC) #5
jsbell
https://codereview.chromium.org/18023022/diff/625001/content/browser/indexed_db/indexed_db_factory.cc File content/browser/indexed_db/indexed_db_factory.cc (right): https://codereview.chromium.org/18023022/diff/625001/content/browser/indexed_db/indexed_db_factory.cc#newcode94 content/browser/indexed_db/indexed_db_factory.cc:94: ptr->active_blob_registry()->ForceShutdown(); ISTM this is a good reason to move ...
7 years ago (2013-12-03 00:11:23 UTC) #6
ericu
https://codereview.chromium.org/18023022/diff/625001/content/browser/indexed_db/indexed_db_factory.cc File content/browser/indexed_db/indexed_db_factory.cc (right): https://codereview.chromium.org/18023022/diff/625001/content/browser/indexed_db/indexed_db_factory.cc#newcode94 content/browser/indexed_db/indexed_db_factory.cc:94: ptr->active_blob_registry()->ForceShutdown(); On 2013/12/03 00:10:03, jsbell wrote: > Hrm, unfortunate ...
7 years ago (2013-12-03 02:03:54 UTC) #7
jsbell
I'm only about halfway through, but lots of little things already. https://codereview.chromium.org/18023022/diff/865001/chrome/browser/media_galleries/fileapi/media_file_system_backend.cc File chrome/browser/media_galleries/fileapi/media_file_system_backend.cc (right): ...
7 years ago (2013-12-18 23:04:39 UTC) #8
ericu
Awesome--thanks! I'll start digging through these. BTW, so far the things I've come up with ...
7 years ago (2013-12-18 23:37:51 UTC) #9
jsbell
https://codereview.chromium.org/18023022/diff/885001/content/browser/indexed_db/indexed_db_dispatcher_host.cc File content/browser/indexed_db/indexed_db_dispatcher_host.cc (right): https://codereview.chromium.org/18023022/diff/885001/content/browser/indexed_db/indexed_db_dispatcher_host.cc#newcode49 content/browser/indexed_db/indexed_db_dispatcher_host.cc:49: ipc_process_id_(ipc_process_id_) { Note the trailing _ inside the ()
7 years ago (2013-12-19 00:39:16 UTC) #10
jsbell
https://codereview.chromium.org/18023022/diff/885001/content/browser/indexed_db/indexed_db_dispatcher_host.cc File content/browser/indexed_db/indexed_db_dispatcher_host.cc (right): https://codereview.chromium.org/18023022/diff/885001/content/browser/indexed_db/indexed_db_dispatcher_host.cc#newcode604 content/browser/indexed_db/indexed_db_dispatcher_host.cc:604: void IndexedDBDispatcherHost::DatabaseDispatcherHost::OnPutWrapper( This could definitely use a comment that ...
7 years ago (2013-12-19 00:55:48 UTC) #11
ericu
https://codereview.chromium.org/18023022/diff/865001/chrome/browser/media_galleries/fileapi/media_file_system_backend.cc File chrome/browser/media_galleries/fileapi/media_file_system_backend.cc (right): https://codereview.chromium.org/18023022/diff/865001/chrome/browser/media_galleries/fileapi/media_file_system_backend.cc#newcode201 chrome/browser/media_galleries/fileapi/media_file_system_backend.cc:201: url.path(), offset, true)); On 2013/12/18 23:04:40, jsbell wrote: > ...
7 years ago (2013-12-19 05:19:10 UTC) #12
jsbell
Another batch of feedback. I feel like it's all starting to sink in. :) https://codereview.chromium.org/18023022/diff/945001/content/browser/indexed_db/indexed_db_factory.cc ...
7 years ago (2013-12-20 00:44:19 UTC) #13
jsbell
https://codereview.chromium.org/18023022/diff/865001/content/browser/indexed_db/indexed_db_backing_store.cc File content/browser/indexed_db/indexed_db_backing_store.cc (right): https://codereview.chromium.org/18023022/diff/865001/content/browser/indexed_db/indexed_db_backing_store.cc#newcode437 content/browser/indexed_db/indexed_db_backing_store.cc:437: if (!DecodeVarInt(&slice, &cur_number) || On 2013/12/19 05:19:11, ericu wrote: ...
7 years ago (2013-12-20 00:59:26 UTC) #14
ericu
I'm still working on the factory lifetime big picture, but I've got the small stuff ...
6 years, 10 months ago (2014-02-20 00:50:28 UTC) #15
jsbell
Just replies to comments. I haven't gone through the patch again. https://codereview.chromium.org/18023022/diff/945001/content/browser/indexed_db/indexed_db_index_writer.cc File content/browser/indexed_db/indexed_db_index_writer.cc (right): ...
6 years, 10 months ago (2014-02-20 22:55:49 UTC) #16
ericu
https://codereview.chromium.org/18023022/diff/865001/content/browser/indexed_db/indexed_db_backing_store.cc File content/browser/indexed_db/indexed_db_backing_store.cc (right): https://codereview.chromium.org/18023022/diff/865001/content/browser/indexed_db/indexed_db_backing_store.cc#newcode55 content/browser/indexed_db/indexed_db_backing_store.cc:55: base::StringPrintf("%x", static_cast<int>(key & 0x0000ff00) >> 8)); On 2013/12/19 05:19:11, ...
6 years, 10 months ago (2014-02-25 21:19:43 UTC) #17
ericu
https://codereview.chromium.org/18023022/diff/865001/content/browser/indexed_db/indexed_db_database.cc File content/browser/indexed_db/indexed_db_database.cc (right): https://codereview.chromium.org/18023022/diff/865001/content/browser/indexed_db/indexed_db_database.cc#newcode45 content/browser/indexed_db/indexed_db_database.cc:45: : callbacks_(callbacks), On 2013/12/19 05:19:11, ericu wrote: > On ...
6 years, 9 months ago (2014-03-05 23:30:38 UTC) #18
jsbell
https://codereview.chromium.org/18023022/diff/2755001/content/browser/indexed_db/indexed_db_backing_store_unittest.cc File content/browser/indexed_db/indexed_db_backing_store_unittest.cc (right): https://codereview.chromium.org/18023022/diff/2755001/content/browser/indexed_db/indexed_db_backing_store_unittest.cc#newcode267 content/browser/indexed_db/indexed_db_backing_store_unittest.cc:267: for (size_t i = 0; i < reads.size(); ++i) ...
6 years, 7 months ago (2014-05-20 22:02:17 UTC) #19
ericu
https://codereview.chromium.org/18023022/diff/2755001/content/browser/indexed_db/indexed_db_backing_store_unittest.cc File content/browser/indexed_db/indexed_db_backing_store_unittest.cc (right): https://codereview.chromium.org/18023022/diff/2755001/content/browser/indexed_db/indexed_db_backing_store_unittest.cc#newcode267 content/browser/indexed_db/indexed_db_backing_store_unittest.cc:267: for (size_t i = 0; i < reads.size(); ++i) ...
6 years, 7 months ago (2014-05-21 18:14:45 UTC) #20
jsbell
On 2014/05/21 18:14:45, ericu wrote: > Should we also block this at the frontend, or ...
6 years, 7 months ago (2014-05-21 19:22:50 UTC) #21
ericu
Chris, Josh: It's time for the big one. I had hoped to split out DeleteRange ...
6 years, 7 months ago (2014-05-27 23:42:43 UTC) #22
cmumford
https://codereview.chromium.org/18023022/diff/2985001/content/browser/indexed_db/indexed_db_backing_store.cc File content/browser/indexed_db/indexed_db_backing_store.cc (right): https://codereview.chromium.org/18023022/diff/2985001/content/browser/indexed_db/indexed_db_backing_store.cc#newcode1369 content/browser/indexed_db/indexed_db_backing_store.cc:1369: std::string start_key, end_key; Nit: You're already using "stop_key" elsewhere, ...
6 years, 6 months ago (2014-05-28 20:34:53 UTC) #23
jsbell
overall lgtm, with some nits, suggestions (and cmumford's, of course!) https://codereview.chromium.org/18023022/diff/2985001/content/browser/indexed_db/indexed_db_backing_store.cc File content/browser/indexed_db/indexed_db_backing_store.cc (right): https://codereview.chromium.org/18023022/diff/2985001/content/browser/indexed_db/indexed_db_backing_store.cc#newcode1994 ...
6 years, 6 months ago (2014-05-28 21:29:13 UTC) #24
cmumford
That's it for my pass. Aside from my comments it's looking great. https://codereview.chromium.org/18023022/diff/3005001/content/browser/indexed_db/indexed_db_transaction.cc File content/browser/indexed_db/indexed_db_transaction.cc ...
6 years, 6 months ago (2014-05-28 22:35:00 UTC) #25
ericu
https://codereview.chromium.org/18023022/diff/2985001/content/browser/indexed_db/indexed_db_backing_store.cc File content/browser/indexed_db/indexed_db_backing_store.cc (right): https://codereview.chromium.org/18023022/diff/2985001/content/browser/indexed_db/indexed_db_backing_store.cc#newcode1369 content/browser/indexed_db/indexed_db_backing_store.cc:1369: std::string start_key, end_key; On 2014/05/28 20:34:54, cmumford wrote: > ...
6 years, 6 months ago (2014-05-28 22:50:41 UTC) #26
cmumford
lgtm
6 years, 6 months ago (2014-05-28 23:17:55 UTC) #27
jsbell
https://codereview.chromium.org/18023022/diff/2985001/content/browser/indexed_db/indexed_db_backing_store.cc File content/browser/indexed_db/indexed_db_backing_store.cc (right): https://codereview.chromium.org/18023022/diff/2985001/content/browser/indexed_db/indexed_db_backing_store.cc#newcode2001 content/browser/indexed_db/indexed_db_backing_store.cc:2001: ExistsEntryKey::Encode(database_id, object_store_id, start_cursor->key()); On 2014/05/28 22:50:42, ericu wrote: > ...
6 years, 6 months ago (2014-05-28 23:35:17 UTC) #28
jsbell
Don't forget to update the CL description before landing!
6 years, 6 months ago (2014-05-28 23:36:01 UTC) #29
ericu
On 2014/05/28 23:36:01, jsbell wrote: > Don't forget to update the CL description before landing! ...
6 years, 6 months ago (2014-05-28 23:48:47 UTC) #30
ericu
https://codereview.chromium.org/18023022/diff/2985001/content/browser/indexed_db/indexed_db_transaction.cc File content/browser/indexed_db/indexed_db_transaction.cc (right): https://codereview.chromium.org/18023022/diff/2985001/content/browser/indexed_db/indexed_db_transaction.cc#newcode358 content/browser/indexed_db/indexed_db_transaction.cc:358: if (!HasPendingTasks() && !IsStatePastStarted() && commit_pending_) { On 2014/05/28 ...
6 years, 6 months ago (2014-05-28 23:49:11 UTC) #31
ericu
The CQ bit was checked by ericu@chromium.org
6 years, 6 months ago (2014-05-28 23:52:23 UTC) #32
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ericu@chromium.org/18023022/3045001
6 years, 6 months ago (2014-05-28 23:54:24 UTC) #33
ericu
+jar@ as owner of tools/metrics/histograms/histograms.xml. Jim, could you take a quick look? It's a trivial ...
6 years, 6 months ago (2014-05-29 01:14:48 UTC) #34
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: chromium_presubmit on tryserver.chromium ...
6 years, 6 months ago (2014-05-29 05:05:01 UTC) #35
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-05-29 05:10:33 UTC) #36
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/builds/70336)
6 years, 6 months ago (2014-05-29 05:10:34 UTC) #37
ericu
Ilya, could you give me a quick owner's review on histograms.xml? It's a tiny addition.
6 years, 6 months ago (2014-06-02 19:38:13 UTC) #38
Ilya Sherman
histograms.xml lgtm
6 years, 6 months ago (2014-06-02 19:54:26 UTC) #39
Ilya Sherman
The CQ bit was checked by isherman@chromium.org
6 years, 6 months ago (2014-06-02 19:55:16 UTC) #40
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ericu@chromium.org/18023022/3065001
6 years, 6 months ago (2014-06-02 19:55:43 UTC) #41
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: chromium_presubmit on tryserver.chromium ...
6 years, 6 months ago (2014-06-02 20:27:48 UTC) #42
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-06-02 20:30:01 UTC) #43
commit-bot: I haz the power
Try jobs failed on following builders: win_gpu_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered_tests/builds/12712)
6 years, 6 months ago (2014-06-02 20:30:01 UTC) #44
ericu
The CQ bit was checked by ericu@chromium.org
6 years, 6 months ago (2014-06-02 23:46:46 UTC) #45
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ericu@chromium.org/18023022/3085001
6 years, 6 months ago (2014-06-02 23:47:28 UTC) #46
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_chromium_rel on tryserver.chromium ...
6 years, 6 months ago (2014-06-03 03:36:27 UTC) #47
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-06-03 06:51:36 UTC) #48
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/builds/22860)
6 years, 6 months ago (2014-06-03 06:51:36 UTC) #49
jsbell
The CQ bit was checked by jsbell@chromium.org
6 years, 6 months ago (2014-06-03 16:36:19 UTC) #50
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ericu@chromium.org/18023022/3085001
6 years, 6 months ago (2014-06-03 16:37:16 UTC) #51
commit-bot: I haz the power
6 years, 6 months ago (2014-06-04 00:34:21 UTC) #52
Message was sent while issue was closed.
Change committed as 274685

Powered by Google App Engine
This is Rietveld 408576698