Bug 122884 – Flesh out the DatabaseProcess (and launch it!)

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brady Eidson]

Flesh out the DatabaseProcess (and launch it!)

This is followup to https://bugs.webkit.org/show_bug.cgi?id=122849

[Brady Eidson]

Created attachment 214341 [details]
Patch v1

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/English.lproj/Localizable.strings', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/DatabaseProcess/DatabaseProcess.cpp', u'Source/WebKit2/DatabaseProcess/DatabaseProcess.h', u'Source/WebKit2/DatabaseProcess/DatabaseProcess.messages.in', u'Source/WebKit2/DatabaseProcess/DatabaseToWebProcessConnection.cpp', u'Source/WebKit2/DatabaseProcess/DatabaseToWebProcessConnection.h', u'Source/WebKit2/DerivedSources.make', u'Source/WebKit2/UIProcess/Databases/DatabaseProcessProxy.cpp', u'Source/WebKit2/UIProcess/Databases/DatabaseProcessProxy.h', u'Source/WebKit2/UIProcess/Databases/DatabaseProcessProxy.messages.in', u'Source/WebKit2/UIProcess/WebContext.cpp', u'Source/WebKit2/UIProcess/WebContext.h', u'Source/WebKit2/UIProcess/WebProcessProxy.cpp', u'Source/WebKit2/UIProcess/WebProcessProxy.h', u'Source/WebKit2/UIProcess/WebProcessProxy.messages.in', u'Source/WebKit2/WebKit2.xcodeproj/project.pbxproj', u'Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBFactoryBackend.cpp', u'Source/WebKit2/WebProcess/Databases/WebToDatabaseProcessConnection.cpp', u'Source/WebKit2/WebProcess/Databases/WebToDatabaseProcessConnection.h', u'Source/WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp', u'Source/WebKit2/WebProcess/WebProcess.cpp', u'Source/WebKit2/WebProcess/WebProcess.h']" exit_code: 1
Source/WebKit2/DatabaseProcess/DatabaseProcess.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/DatabaseProcess/DatabaseToWebProcessConnection.h:54:  Could not find a newline character at the end of the file.  [whitespace/ending_newline] [5]
Source/WebKit2/DatabaseProcess/DatabaseToWebProcessConnection.cpp:71:  Could not find a newline character at the end of the file.  [whitespace/ending_newline] [5]
Total errors found: 3 in 25 files


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

[Brady Eidson]

(In reply to comment #2)
> Attachment 214341 [details] did not pass style-queue:
> Source/WebKit2/DatabaseProcess/DatabaseProcess.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
> Source/WebKit2/DatabaseProcess/DatabaseToWebProcessConnection.h:54:  Could not find a newline character at the end of the file.  [whitespace/ending_newline] [5]
> Source/WebKit2/DatabaseProcess/DatabaseToWebProcessConnection.cpp:71:  Could not find a newline character at the end of the file.  [whitespace/ending_newline] [5]

All fixed locally.

[EFL EWS Bot]

Comment on attachment 214341 [details]
Patch v1 

Attachment 214341 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/4106193

[Tim Horton]

Comment on attachment 214341 [details]
Patch v1 

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

Seems reasonable to me.

> Source/WebCore/English.lproj/Localizable.strings:-39
> -/* Visible name of the shared worker process. The argument is the application name. */
> -"%@ Shared Worker" = "%@ Shared Worker";
> -

Why don't we need this one anymore? Why doesn't the changelog say why not?

> Source/WebKit2/UIProcess/Databases/DatabaseProcessProxy.cpp:93
> +    // Grab the first pending connection reply.

Not sure this is a useful comment.

> Source/WebKit2/UIProcess/WebContext.cpp:423
> +    ASSERT(m_databaseProcess);

This seems unnecessary after an ensure()! At least, that's what ensure means :) And otherwise you're just gonna crash below, so you don't need this.

> Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBFactoryBackend.cpp:56
> +    // FIXME: Access the connection to the DatabaseProcess to make sure it has been created to assisted

To assisted?

[Brady Eidson]

(In reply to comment #5)
> (From update of attachment 214341 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=214341&action=review
> 
> Seems reasonable to me.
> 
> > Source/WebCore/English.lproj/Localizable.strings:-39
> > -/* Visible name of the shared worker process. The argument is the application name. */
> > -"%@ Shared Worker" = "%@ Shared Worker";
> > -
> 
> Why don't we need this one anymore? Why doesn't the changelog say why not?
>

I ran the update-webkit-localizable-strings script and it noticed that the Shared Worker string had been removed.  This is because the Shared Worker process was removed.  And when it was removed, the patch authors didn't remove the localizable string.

This happens all the time (an obsolete localizable string being removed when a new one is added) and I don't think we call it out in change logs.
 
> > Source/WebKit2/UIProcess/Databases/DatabaseProcessProxy.cpp:93
> > +    // Grab the first pending connection reply.
> 
> Not sure this is a useful comment.

Somewhat agreed, yet it follows the exact pattern from NetworkProcessProxy and PluginProcessProxy (which include the comment).  Not sure I want to make this new one different.

> > Source/WebKit2/UIProcess/WebContext.cpp:423
> > +    ASSERT(m_databaseProcess);
> 
> This seems unnecessary after an ensure()! At least, that's what ensure means :) And otherwise you're just gonna crash below, so you don't need this.

Ugh.  Yet another difference from the other two following the pattern.  Okay, will remove.

> 
> > Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBFactoryBackend.cpp:56
> > +    // FIXME: Access the connection to the DatabaseProcess to make sure it has been created to assisted
> 
> To assisted?

s/assisted/assist/   fixed.

[Brady Eidson]

(In reply to comment #4)
> (From update of attachment 214341 [details])
> Attachment 214341 [details] did not pass efl-wk2-ews (efl-wk2):
> Output: http://webkit-queues.appspot.com/results/4106193

Weird that these failures are new, actually...  but I nuked the argument names for now.

[Brady Eidson]

http://trac.webkit.org/changeset/157524