WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
154296
Modern IDB: WK2 IPC Scaffolding
https://bugs.webkit.org/show_bug.cgi?id=154296
Summary
Modern IDB: WK2 IPC Scaffolding
Brady Eidson
Reported
2016-02-16 11:09:49 PST
Modern IDB: WK2 IPC Scaffolding
Attachments
Patch v1
(46.68 KB, patch)
2016-02-16 11:13 PST
,
Brady Eidson
achristensen
: review+
Details
Formatted Diff
Diff
Patch for landing
(47.10 KB, patch)
2016-02-16 11:47 PST
,
Brady Eidson
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Brady Eidson
Comment 1
2016-02-16 11:13:32 PST
Created
attachment 271445
[details]
Patch v1
WebKit Commit Bot
Comment 2
2016-02-16 11:14:55 PST
Attachment 271445
[details]
did not pass style-queue: ERROR: Source/WebKit2/ChangeLog:10: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] ERROR: Source/WebKit2/WebProcess/Databases/WebDatabaseProvider.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKit2/WebProcess/Databases/WebToDatabaseProcessConnection.h:60: The parameter name "sessionID" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 3 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.
Alex Christensen
Comment 3
2016-02-16 11:24:51 PST
Comment on
attachment 271445
[details]
Patch v1 View in context:
https://bugs.webkit.org/attachment.cgi?id=271445&action=review
r=me
> Source/WebKit2/DatabaseProcess/DatabaseToWebProcessConnection.h:76 > + HashMap<uint64_t, RefPtr<WebIDBConnectionToClient>> m_webIDBConnections;
It might be nice to typedef uint64_t ServerConnectionIdentifer. There are a lot of uint64_t's in IDB.
> Source/WebKit2/DatabaseProcess/IndexedDB/WebIDBConnectionToClient.cpp:62 > + return *m_connectionToClient;
It would be nice if you could make m_connectionToClient a ref. Does it need to be initialized after relaxAdoptionRequirement?
> Source/WebKit2/WebProcess/Databases/WebDatabaseProvider.h:30 > +#include <WebCore/InProcessIDBServer.h> > #include <WebCore/DatabaseProvider.h>
Include order
> Source/WebKit2/WebProcess/Databases/WebToDatabaseProcessConnection.h:60 > + WebIDBConnectionToServer& idbConnectionToServerForSession(const WebCore::SessionID& sessionID);
Parameter name should be removed.
Brady Eidson
Comment 4
2016-02-16 11:40:29 PST
(In reply to
comment #3
)
> Comment on
attachment 271445
[details]
> Patch v1 > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=271445&action=review
> > r=me > > > Source/WebKit2/DatabaseProcess/DatabaseToWebProcessConnection.h:76 > > + HashMap<uint64_t, RefPtr<WebIDBConnectionToClient>> m_webIDBConnections; > > It might be nice to typedef uint64_t ServerConnectionIdentifer. There are a > lot of uint64_t's in IDB.
Unclear to me why a whole bunch of compatible typedefs would be better :(
> > > Source/WebKit2/DatabaseProcess/IndexedDB/WebIDBConnectionToClient.cpp:62 > > + return *m_connectionToClient; > > It would be nice if you could make m_connectionToClient a ref.
Can't.
> Does it need to be initialized after relaxAdoptionRequirement?
Yes
> > > Source/WebKit2/WebProcess/Databases/WebDatabaseProvider.h:30 > > +#include <WebCore/InProcessIDBServer.h> > > #include <WebCore/DatabaseProvider.h> > > Include order
Whoops!
> > > Source/WebKit2/WebProcess/Databases/WebToDatabaseProcessConnection.h:60 > > + WebIDBConnectionToServer& idbConnectionToServerForSession(const WebCore::SessionID& sessionID); > > Parameter name should be removed.
Sure!
Brady Eidson
Comment 5
2016-02-16 11:45:18 PST
gtk and elf are broken apparently because some headers from WebCore cannot be found... I don't know how to fix!
Brady Eidson
Comment 6
2016-02-16 11:45:59 PST
NM, figured out it out in the WebKit2 CMakeLists.txt
Brady Eidson
Comment 7
2016-02-16 11:47:19 PST
Created
attachment 271460
[details]
Patch for landing
WebKit Commit Bot
Comment 8
2016-02-16 12:58:54 PST
Comment on
attachment 271460
[details]
Patch for landing Clearing flags on attachment: 271460 Committed
r196651
: <
http://trac.webkit.org/changeset/196651
>
WebKit Commit Bot
Comment 9
2016-02-16 12:58:57 PST
All reviewed patches have been landed. Closing bug.
Csaba Osztrogonác
Comment 10
2016-02-16 14:03:11 PST
(In reply to
comment #8
)
> Comment on
attachment 271460
[details]
> Patch for landing > > Clearing flags on attachment: 271460 > > Committed
r196651
: <
http://trac.webkit.org/changeset/196651
>
It broke the Apple Mac cmake build.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug