Bug 106457 – [Chromium] Remove idbFactory from WebKitPlatformSupport

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

[Mark Pilgrim (Google)]

[Chromium] Remove idbFactory from WebKitPlatformSupport

[Mark Pilgrim (Google)]

Created attachment 181924 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[WebKit Review Bot]

Comment on attachment 181924 [details]
Patch

Attachment 181924 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15771537

[Mark Pilgrim (Google)]

Looks like we need to wait for some Chromium changes to sync upstream first. Removing review flag until that happens.

[Peter Beverloo (cr-android ews)]

Comment on attachment 181924 [details]
Patch

Attachment 181924 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15754787

[Adam Barth]

You probably need to increase the chromium_rev in this file:

http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/DEPS#L35

Usually we do that in a separate patch.

[Mark Pilgrim (Google)]

Created attachment 182052 [details]
Patch

[WebKit Review Bot]

Comment on attachment 182052 [details]
Patch

Attachment 182052 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15775613

[Peter Beverloo (cr-android ews)]

Comment on attachment 182052 [details]
Patch

Attachment 182052 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15775618

[Mark Pilgrim (Google)]

Created attachment 182228 [details]
Patch

[Mark Pilgrim (Google)]

Comment on attachment 182228 [details]
Patch

Indeed it was a DEPS problem, and it appears that someone else has resolved it in the meantime. http://trac.webkit.org/changeset/139347/trunk/Source/WebKit/chromium/DEPS

So, resubmitting same patch.

[noel gordon]

This appears to be crashing a truck load of storage tests on cr-linux bot.

[Mark Pilgrim (Google)]

Created attachment 186642 [details]
Patch

[WebKit Review Bot]

Comment on attachment 186642 [details]
Patch

Clearing flags on attachment: 186642

Committed r141896: <http://trac.webkit.org/changeset/141896>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by bug 108956

[Mark Pilgrim (Google)]

Created attachment 189141 [details]
Patch

[Mark Pilgrim (Google)]

Comment on attachment 189141 [details]
Patch

Same patch, applied to ToT. Now that https://codereview.chromium.org/12230054 has landed, this no longer crashes LayoutTests/http/tests/security/*indexeddb*

[WebKit Review Bot]

Comment on attachment 189141 [details]
Patch

Clearing flags on attachment: 189141

Committed r143382: <http://trac.webkit.org/changeset/143382>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Adrienne Walker]

Reverted r143382 for reason:

Breaks Chromium win/mac canary compilation

Committed r143403: <http://trac.webkit.org/changeset/143403>

[Adrienne Walker]

http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac%20Builder/builds/40413/steps/compile/logs/stdio

In file included from ../../webkit/tools/test_shell/test_shell_mac.mm:41:
../../webkit/tools/test_shell/test_shell_webkit_init.h:76:34:error: 'idbFactory' marked 'override' but does not override any member functions
  virtual WebKit::WebIDBFactory* idbFactory() OVERRIDE;

[Adam Barth]

We shouldn't ever use OVERRIDE when subclassing the WebKit API.  It just leads to this sort of pain...

[Mark Pilgrim (Google)]

(In reply to comment #23)
> We shouldn't ever use OVERRIDE when subclassing the WebKit API.  It just leads to this sort of pain...

I removed a bunch of them a few weeks ago, but this one was in test_shell and I missed it last time. Anyway, https://codereview.chromium.org/12320002/ will fix it then I'll re-submit this patch.

[Mark Pilgrim (Google)]

Comment on attachment 189141 [details]
Patch

Fixed downstream issue, roll led DEPS. Can we just resubmit this attachment or do I need to reupload the same patch again?

[Adam Barth]

Comment on attachment 189141 [details]
Patch

We can just resubmit.

[WebKit Review Bot]

Comment on attachment 189141 [details]
Patch

Clearing flags on attachment: 189141

Committed r143473: <http://trac.webkit.org/changeset/143473>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.