Bug 35763 – Remove the now-redundant Settings fields for the Database

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Eric U.]

Bug 35310 finished adding the new interface; I then removed the last use of the old interface from Chromium.  It's now save to remove the Database fields from Settings and WebSettings.

[Eric U.]

Created attachment 50049 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 50049 [details]
Patch

Patch fails to apply:

Failed to run "['/mnt/git/webkit-chromium-ews/WebKitTools/Scripts/svn-apply', '--force']" exit_code: 1
patching file WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebCore/WebCore.base.exp
patching file WebCore/page/Settings.cpp
Hunk #1 succeeded at 73 (offset 1 line).
Hunk #2 FAILED at 261.
1 out of 2 hunks FAILED -- saving rejects to file WebCore/page/Settings.cpp.rej

Ok.  Why do we have a separate way of managing database settings?

[Eric U.]

I'll update to fix the patch; my code was a few days old.

The Settings object isn't available to Workers.  In order to get the Database accessible to Workers, the enable bit has to live somewhere that they can get at it.  WebSockets implemented their enable bit this way [as a static class member], so I followed that model.

[Eric U.]

Created attachment 50060 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 50060 [details]
Patch

Clearing flags on attachment: 50060

Committed r55666: <http://trac.webkit.org/changeset/55666>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Daniel Bates]

This change broke the Windows bots. Looks like the corresponding Windows changes were not landed.

[Daniel Bates]

Applied corresponding change to file WebView.cpp to fix the Windows build.

Committed fix in r55667 <http://trac.webkit.org/changeset/55667>.

[Eric U.]

Created attachment 50265 [details]
Fix the build fix.

That build fix patch fixed the compile, but failed to restore the path by which the database enable preference can actually enable the database API.  This puts the right call in.

[Eric U.]

Reopening for the new fix patch.

[David Levin]

Comment on attachment 50265 [details]
Fix the build fix.

> Index: WebKit/win/ChangeLog
> [phone]  Eric Uhrhane  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        The build fix for my patch on bug #35763 wasn't quite right--it removed
> +        the call entirely, instead of replacing it with the new API.  This adds
> +        the call to Database::setIsAvailable.

Please add a bug link here.

> Index: WebKit/win/WebView.cpp
> +    hr = prefsPrivate->databasesEnabled(&enabled);
> +    if (FAILED(hr))
> +        return hr;
> +    Database::setIsAvailable(enabled);
> +

This needs an appropriate if ENABLE guard.

[Eric U.]

Created attachment 50342 [details]
Patch

[Eric U.]

(In reply to comment #11)
> (From update of attachment 50265 [details])
> > Index: WebKit/win/ChangeLog
> > [phone]  Eric Uhrhane  <[e-mail]>
> > +
> > +        Reviewed by NOBODY (OOPS!).
> > +
> > +        The build fix for my patch on bug #35763 wasn't quite right--it removed
> > +        the call entirely, instead of replacing it with the new API.  This adds
> > +        the call to Database::setIsAvailable.
> 
> Please add a bug link here.

Done.

> > Index: WebKit/win/WebView.cpp
> > +    hr = prefsPrivate->databasesEnabled(&enabled);
> > +    if (FAILED(hr))
> > +        return hr;
> > +    Database::setIsAvailable(enabled);
> > +
> 
> This needs an appropriate if ENABLE guard.

Done, and I now have a post-it on my Mac's monitor reminding me to check for that every time.

[WebKit Commit Bot]

Comment on attachment 50342 [details]
Patch

Clearing flags on attachment: 50342

Committed r55809: <http://trac.webkit.org/changeset/55809>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.