Bug 157072

Summary: Modern IDB: Implement native IDBFactory.getAllDatabaseNames for WebInspector
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebCore Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, alecflett, annulen, benjamin, cdumez, cmarcelo, commit-queue, joepeck, jsbell, timothy
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 149117, 154686    
Attachments:
Description Flags
Patch
achristensen: review+
Patch for EWS + landing none

Description Brady Eidson 2016-04-27 09:13:43 PDT
Modern IDB: Implement native IDBFactory.getAllDatabaseNames for WebInspector

This is for internal use only, not meant for bindings.
Comment 1 Brady Eidson 2016-04-27 16:09:05 PDT
Created attachment 277546 [details]
Patch
Comment 2 WebKit Commit Bot 2016-04-27 16:11:13 PDT
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.
Comment 3 Alex Christensen 2016-04-27 16:36:08 PDT
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)

>=
Comment 4 Alex Christensen 2016-04-27 16:38:55 PDT
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.
Comment 5 Brady Eidson 2016-04-27 16:55:52 PDT
Created attachment 277548 [details]
Patch for EWS + landing
Comment 6 WebKit Commit Bot 2016-04-27 16:59:15 PDT
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.
Comment 7 WebKit Commit Bot 2016-04-27 18:16:01 PDT
Comment on attachment 277548 [details]
Patch for EWS + landing

Clearing flags on attachment: 277548

Committed r200163: <http://trac.webkit.org/changeset/200163>
Comment 8 WebKit Commit Bot 2016-04-27 18:16:06 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Brady Eidson 2016-04-27 20:24:52 PDT
Welp, EFL doesn't use FileSystemGlib, apparently... *sigh*

Working on a fix.
Comment 10 Brady Eidson 2016-04-27 20:33:37 PDT
EFL build fix attempted in http://trac.webkit.org/changeset/200168
Comment 11 Konstantin Tokarev 2016-05-27 14:36:15 PDT
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.
Comment 12 Alex Christensen 2016-05-31 11:06:53 PDT
(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.
Comment 13 Brady Eidson 2016-05-31 11:29:44 PDT
(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.