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

Issue 2784003002: [IndexedDB] Mojo testing harness. (Closed)

Created:
3 years, 8 months ago by dmurph
Modified:
3 years, 7 months ago
CC:
Aaron Boodman, abarth-chromium, chromium-reviews, cmumford, darin (slow to review), darin-cc_chromium.org, jam, jsbell+idb_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[IndexedDB] Mojo testing harness & tests. This creates a testing harness for the IndexedDB system from the mojo layer. Included tests: * Opening connection and immediately closing. * Opening connection, upgrading database, and closing. * Opening two connection, upgrading on one. * Calling 'put' with an invalid blob. There is some minimal state storage for each connection, and we have mock objects for the callbacks. Review-Url: https://codereview.chromium.org/2784003002 Cr-Commit-Position: refs/heads/master@{#472563} Committed: https://chromium.googlesource.com/chromium/src/+/a4c5acae0c20cb76eb34f2703ab14b8ca6d0a6cf

Patch Set 1 #

Patch Set 2 : Working! #

Total comments: 13

Patch Set 3 : comments #

Total comments: 1

Patch Set 4 : Restructured #

Total comments: 4

Patch Set 5 : comments #

Total comments: 10

Patch Set 6 : comments, removed thread flushing #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+662 lines, -2 lines) Patch
M content/browser/indexed_db/indexed_db_dispatcher_host.h View 1 2 chunks +2 lines, -0 lines 0 comments Download
A content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc View 1 2 3 4 5 1 chunk +445 lines, -0 lines 1 comment Download
A content/browser/indexed_db/mock_mojo_indexed_db_callbacks.h View 1 2 3 1 chunk +121 lines, -0 lines 0 comments Download
A content/browser/indexed_db/mock_mojo_indexed_db_callbacks.cc View 1 2 3 1 chunk +19 lines, -0 lines 0 comments Download
A content/browser/indexed_db/mock_mojo_indexed_db_database_callbacks.h View 1 2 3 1 chunk +48 lines, -0 lines 0 comments Download
A content/browser/indexed_db/mock_mojo_indexed_db_database_callbacks.cc View 1 2 3 1 chunk +20 lines, -0 lines 0 comments Download
M content/common/indexed_db/indexed_db_metadata.h View 1 1 chunk +1 line, -1 line 0 comments Download
M content/common/indexed_db/indexed_db_metadata.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M content/test/BUILD.gn View 1 2 3 4 5 2 chunks +5 lines, -0 lines 0 comments Download

Messages

Total messages: 44 (22 generated)
dmurph
Hello John & Reilly, Can you PTAL at this WIP patch? Please look especially at ...
3 years, 7 months ago (2017-05-11 00:12:01 UTC) #3
dmurph
+Rockot, same request as above, can you PTAL at this patch? It's a WIP (still ...
3 years, 7 months ago (2017-05-11 00:15:51 UTC) #5
Reilly Grant (use Gerrit)
https://codereview.chromium.org/2784003002/diff/20001/content/browser/indexed_db/indexed_db_callbacks.h File content/browser/indexed_db/indexed_db_callbacks.h (right): https://codereview.chromium.org/2784003002/diff/20001/content/browser/indexed_db/indexed_db_callbacks.h#newcode149 content/browser/indexed_db/indexed_db_callbacks.h:149: std::unique_ptr<IOThreadHelper, DeleteOnIOThreadTaskRunner> io_helper_; Why did you copy this code ...
3 years, 7 months ago (2017-05-11 01:57:09 UTC) #6
Reilly Grant (use Gerrit)
https://codereview.chromium.org/2784003002/diff/20001/content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc File content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc (right): https://codereview.chromium.org/2784003002/diff/20001/content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc#newcode92 content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc:92: DatabaseAssociatedPtr* database; It's hard to tell what is going ...
3 years, 7 months ago (2017-05-11 02:12:02 UTC) #7
Ken Rockot(use gerrit already)
On Wed, May 10, 2017 at 7:12 PM, <reillyg@chromium.org> wrote: > > https://codereview.chromium.org/2784003002/diff/20001/ > content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc ...
3 years, 7 months ago (2017-05-11 02:19:32 UTC) #8
Ken Rockot(use gerrit already)
Seems mostly reasonable to me. The whole gmock-not-supporting-move-only-parameters thing is kind of sad. Are the ...
3 years, 7 months ago (2017-05-11 17:21:03 UTC) #9
dmurph
Thanks so much for the comments! What do you think now? It's cleaned up a ...
3 years, 7 months ago (2017-05-12 01:22:06 UTC) #10
Reilly Grant (use Gerrit)
https://codereview.chromium.org/2784003002/diff/20001/content/browser/indexed_db/indexed_db_callbacks.h File content/browser/indexed_db/indexed_db_callbacks.h (right): https://codereview.chromium.org/2784003002/diff/20001/content/browser/indexed_db/indexed_db_callbacks.h#newcode149 content/browser/indexed_db/indexed_db_callbacks.h:149: std::unique_ptr<IOThreadHelper, DeleteOnIOThreadTaskRunner> io_helper_; On 2017/05/12 01:22:06, dmurph wrote: > ...
3 years, 7 months ago (2017-05-12 01:46:52 UTC) #13
dmurph
Thanks for the comments! Please take another look - I'd actually like to submit something ...
3 years, 7 months ago (2017-05-13 01:24:47 UTC) #19
dmurph
(test failures from test dcheck I put in there- forgot to remove)
3 years, 7 months ago (2017-05-15 20:52:45 UTC) #22
Reilly Grant (use Gerrit)
https://codereview.chromium.org/2784003002/diff/60001/content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc File content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc (right): https://codereview.chromium.org/2784003002/diff/60001/content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc#newcode426 content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc:426: new StrictMock<MockMojoIndexedDBCallbacks>()); auto put_callbacks = base::MakeUnique<StrictMock<MockMojoIndexedDBCallbacks>>(); https://codereview.chromium.org/2784003002/diff/60001/content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc#newcode459 content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc:459: connection.database.FlushForTesting(); ...
3 years, 7 months ago (2017-05-15 21:24:33 UTC) #23
dmurph
Ken & Reilly: can you PTAL? John: This patch is an FYI about what is ...
3 years, 7 months ago (2017-05-16 20:43:25 UTC) #27
Reilly Grant (use Gerrit)
https://codereview.chromium.org/2784003002/diff/80001/content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc File content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc (right): https://codereview.chromium.org/2784003002/diff/80001/content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc#newcode288 content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc:288: // extra comment? https://codereview.chromium.org/2784003002/diff/80001/content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc#newcode347 content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc:347: FlushIORunLoop(); I'm not convinced ...
3 years, 7 months ago (2017-05-16 20:55:13 UTC) #28
Ken Rockot(use gerrit already)
The mojom stuff all looks great, but I think the waiting-for-stuff-to-happen behavior definitely needs to ...
3 years, 7 months ago (2017-05-16 21:06:32 UTC) #29
dmurph
Thanks for the comments! PTAL - I removed all of the thread flushing and join ...
3 years, 7 months ago (2017-05-16 23:06:44 UTC) #31
Ken Rockot(use gerrit already)
https://codereview.chromium.org/2784003002/diff/100001/content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc File content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc (right): https://codereview.chromium.org/2784003002/diff/100001/content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc#newcode164 content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc:164: void TearDown() override { Thanks, this seems better! One ...
3 years, 7 months ago (2017-05-16 23:12:47 UTC) #33
dmurph
On 2017/05/16 23:12:47, Ken Rockot wrote: > https://codereview.chromium.org/2784003002/diff/100001/content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc > File content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc (right): > > https://codereview.chromium.org/2784003002/diff/100001/content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc#newcode164 ...
3 years, 7 months ago (2017-05-16 23:21:59 UTC) #34
Ken Rockot(use gerrit already)
On 2017/05/16 at 23:21:59, dmurph wrote: > On 2017/05/16 23:12:47, Ken Rockot wrote: > > ...
3 years, 7 months ago (2017-05-16 23:34:04 UTC) #35
dmurph
Reilly, can you PTAL?
3 years, 7 months ago (2017-05-17 00:16:06 UTC) #38
Reilly Grant (use Gerrit)
lgtm
3 years, 7 months ago (2017-05-17 00:29:25 UTC) #39
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2784003002/100001
3 years, 7 months ago (2017-05-17 16:45:03 UTC) #41
commit-bot: I haz the power
3 years, 7 months ago (2017-05-17 20:59:46 UTC) #44
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/a4c5acae0c20cb76eb34f2703ab1...

Powered by Google App Engine
This is Rietveld 408576698