Bug 106135 – IndexedDB: Provide LevelDB with IDBEnv instead of Env::Default

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[David Grogan]

IndexedDB: Provide LevelDB with IDBEnv instead of Env::Default

[David Grogan]

Created attachment 181382 [details]
Patch

[WebKit Review Bot]

Attachment 181382 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 181382 [details]
Patch

Attachment 181382 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15739178

[Peter Beverloo (cr-android ews)]

Comment on attachment 181382 [details]
Patch

Attachment 181382 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15732218

[David Grogan]

Webkit's chromium deps will have to be rolled when https://codereview.chromium.org/11776009/ lands.

[David Grogan]

Created attachment 181724 [details]
Patch

[David Grogan]

Josh, could you take a look at this?

[Joshua Bell]

lgtm but implies work for other ports to ensure their leveldb copy exports IDBEnv.

CC'ing some folks working on GTK.

[David Grogan]

Tony, could you review this?

If it makes it to r+ it'll be the 75th patch of mine that you've granted review. Appreciated.

[WebKit Review Bot]

Comment on attachment 181724 [details]
Patch

Clearing flags on attachment: 181724

Committed r139143: <http://trac.webkit.org/changeset/139143>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Dimitri Glazkov (Google)]

Could this patch be causing these failures on try bots?

http://build.chromium.org/p/tryserver.chromium/builders/linux_aura/builds/7888/steps/content_browsertests/logs/stdio

[Dimitri Glazkov (Google)]

I am pretty sure the patch is the culprit. Please let me know if you have a fix. This is blocking the WebKit roll.

[Martin Robinson]

I recently bumped against this change when getting IndexedDatabase working for the GTK+ port. If this change stays in, I think it makes sense to wrap it with #if PLATFORM(CHROMIUM), since env_idb.h seems specific to Chromium.

[Dimitri Glazkov (Google)]

Reverted r139143 for reason:

Broke Chromium content_browsertests.

Committed r139208: <http://trac.webkit.org/changeset/139208>

[David Grogan]

Created attachment 182385 [details]
Patch

[David Grogan]

Comment on attachment 182385 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182385&action=review

The problems found by the DCHECK on the bots are addressed in https://codereview.chromium.org/11860015/.

I couldn't get the DCHECK to trigger locally but debugging and testing via trybot leads me to believe that all should be well after that CL lands and is rolled into webkit's chromium DEPS.

> Source/WebCore/platform/leveldb/LevelDBDatabase.cpp:130
> +    options.env = leveldb::IDBEnv();

This is the only difference from the original patch that got reverted.

[David Grogan]

(In reply to comment #14)
> I recently bumped against this change when getting IndexedDatabase working for the GTK+ port. If this change stays in, I think it makes sense to wrap it with #if PLATFORM(CHROMIUM), since env_idb.h seems specific to Chromium.

Agreed. Send out a patch once you've gotten this working locally. It would probably make sense to have a new method LevelDBDatabase::getEnv whose implementation contains the #ifs.

[Tony Chang]

Comment on attachment 182385 [details]
Patch

OK, make sure to roll Source/WebKit/chromium/DEPS before landing this.

[WebKit Review Bot]

Comment on attachment 182385 [details]
Patch

Clearing flags on attachment: 182385

Committed r139556: <http://trac.webkit.org/changeset/139556>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.