Bug 157072 – Modern IDB: Implement native IDBFactory.getAllDatabaseNames for WebInspector

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brady Eidson]

Modern IDB: Implement native IDBFactory.getAllDatabaseNames for WebInspector

This is for internal use only, not meant for bindings.

[Brady Eidson]

Created attachment 277546 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp:414:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.h:126:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.h:146:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/IDBFactory.cpp:162:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.cpp:85:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/IDBFactory.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 7 in 42 files


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

[Alex Christensen]

Comment on attachment 277546 [details]
Patch

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

> Source/WTF/wtf/HexNumber.h:117
> +    return (c >= '0' && c <= '9') || (c >= 'A' && c <= 'F') || (c >= 'a' && c <= 'f');

We can mask the characters then compare to reduce the number of operations done here.

> Source/WTF/wtf/HexNumber.h:120
> +inline char hexDigit(int i)

"unchecked" here would match uncheckedHexDigitValue

> Source/WTF/wtf/HexNumber.h:125
> +    return (i >= 10) ? i - 10 + 'A' : i += '0'; 

+=?  This only needs to be +.  Probably optimized out, though.

> Source/WebCore/platform/FileSystem.cpp:150
> +        if (i + 2 < length)

>=

> Source/WebCore/platform/FileSystem.cpp:168
> +        if (i + 5 < length)

>=

[Alex Christensen]

Comment on attachment 277546 [details]
Patch

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

> Source/WebCore/platform/posix/FileSystemPOSIX.cpp:260
> +String lastComponentOfPathIgnoringTrailingSlash(const String& path)

This is not implemented in GTK.  Move to FileSystem.cpp.

[Brady Eidson]

Created attachment 277548 [details]
Patch for EWS + landing

[WebKit Commit Bot]

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


ERROR: Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp:414:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.h:126:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.h:146:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/IDBFactory.cpp:162:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.cpp:85:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/Modules/indexeddb/IDBFactory.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 7 in 42 files


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

[WebKit Commit Bot]

Comment on attachment 277548 [details]
Patch for EWS + landing

Clearing flags on attachment: 277548

Committed r200163: <http://trac.webkit.org/changeset/200163>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Brady Eidson]

Welp, EFL doesn't use FileSystemGlib, apparently... *sigh*

Working on a fix.

[Brady Eidson]

EFL build fix attempted in http://trac.webkit.org/changeset/200168

[Konstantin Tokarev]

I think that instead of lastComponentOfPathIgnoringTrailingSlash() introduction it would be better to add a possibility to get list of relative paths from listDirectory().

Right now, listDirectory() implementations (posix and glib) iterate through file names in a directory and convert them to absolute. After that, you extract that original file name from absolute path with lastComponentOfPathIgnoringTrailingSlash().

Looks like the easiest way is to add a flag to listDirectory to control if absolute or relative paths are requested.

If you like this proposal I can prepare the patch.

[Alex Christensen]

(In reply to comment #11)
> If you like this proposal I can prepare the patch.
If you think you can make this cleaner then it never hurts to prepare a patch.  Once we see an implementation, the worst we could do is reject it.  I would believe this could be done cleaner, though.

[Brady Eidson]

(In reply to comment #12)
> (In reply to comment #11)
> > If you like this proposal I can prepare the patch.
> If you think you can make this cleaner then it never hurts to prepare a
> patch.  Once we see an implementation, the worst we could do is reject it. 
> I would believe this could be done cleaner, though.

Just please do it in a new bug.

It's harmful to discuss future plans in an already-resolved bug.