Bug 106457 - [Chromium] Remove idbFactory from WebKitPlatformSupport
Summary: [Chromium] Remove idbFactory from WebKitPlatformSupport
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Pilgrim (Google)
URL:
Keywords:
Depends on: 107132 108956
Blocks: 82948 110262
  Show dependency treegraph
 
Reported: 2013-01-09 07:35 PST by Mark Pilgrim (Google)
Modified: 2013-02-20 10:23 PST (History)
13 users (show)

See Also:


Attachments
Patch (2.66 KB, patch)
2013-01-09 07:37 PST, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
Patch (2.62 KB, patch)
2013-01-09 20:06 PST, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
Patch (2.59 KB, patch)
2013-01-10 17:29 PST, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
Patch (2.53 KB, patch)
2013-02-05 09:18 PST, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
Patch (2.56 KB, patch)
2013-02-19 12:30 PST, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Pilgrim (Google) 2013-01-09 07:35:32 PST
[Chromium] Remove idbFactory from WebKitPlatformSupport
Comment 1 Mark Pilgrim (Google) 2013-01-09 07:37:58 PST
Created attachment 181924 [details]
Patch
Comment 2 WebKit Review Bot 2013-01-09 07:40:13 PST
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 WebKit Review Bot 2013-01-09 07:44:32 PST
Comment on attachment 181924 [details]
Patch

Attachment 181924 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15771537
Comment 4 Mark Pilgrim (Google) 2013-01-09 07:55:51 PST
Looks like we need to wait for some Chromium changes to sync upstream first. Removing review flag until that happens.
Comment 5 Peter Beverloo (cr-android ews) 2013-01-09 08:19:04 PST
Comment on attachment 181924 [details]
Patch

Attachment 181924 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15754787
Comment 6 Adam Barth 2013-01-09 12:38:24 PST
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.
Comment 7 Mark Pilgrim (Google) 2013-01-09 20:06:32 PST
Created attachment 182052 [details]
Patch
Comment 8 WebKit Review Bot 2013-01-09 20:33:51 PST
Comment on attachment 182052 [details]
Patch

Attachment 182052 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15775613
Comment 9 Peter Beverloo (cr-android ews) 2013-01-09 20:52:46 PST
Comment on attachment 182052 [details]
Patch

Attachment 182052 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15775618
Comment 10 Mark Pilgrim (Google) 2013-01-10 17:29:18 PST
Created attachment 182228 [details]
Patch
Comment 11 Mark Pilgrim (Google) 2013-01-10 17:30:05 PST
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.
Comment 12 noel gordon 2013-01-10 23:37:49 PST
This appears to be crashing a truck load of storage tests on cr-linux bot.
Comment 13 Mark Pilgrim (Google) 2013-02-05 09:18:15 PST
Created attachment 186642 [details]
Patch
Comment 14 WebKit Review Bot 2013-02-05 09:44:22 PST
Comment on attachment 186642 [details]
Patch

Clearing flags on attachment: 186642

Committed r141896: <http://trac.webkit.org/changeset/141896>
Comment 15 WebKit Review Bot 2013-02-05 09:44:27 PST
All reviewed patches have been landed.  Closing bug.
Comment 16 WebKit Review Bot 2013-02-05 10:28:36 PST
Re-opened since this is blocked by bug 108956
Comment 17 Mark Pilgrim (Google) 2013-02-19 12:30:29 PST
Created attachment 189141 [details]
Patch
Comment 18 Mark Pilgrim (Google) 2013-02-19 12:31:44 PST
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*
Comment 19 WebKit Review Bot 2013-02-19 14:04:28 PST
Comment on attachment 189141 [details]
Patch

Clearing flags on attachment: 189141

Committed r143382: <http://trac.webkit.org/changeset/143382>
Comment 20 WebKit Review Bot 2013-02-19 14:04:34 PST
All reviewed patches have been landed.  Closing bug.
Comment 21 Adrienne Walker 2013-02-19 16:22:06 PST
Reverted r143382 for reason:

Breaks Chromium win/mac canary compilation

Committed r143403: <http://trac.webkit.org/changeset/143403>
Comment 22 Adrienne Walker 2013-02-19 16:23:30 PST
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;
Comment 23 Adam Barth 2013-02-19 20:44:10 PST
We shouldn't ever use OVERRIDE when subclassing the WebKit API.  It just leads to this sort of pain...
Comment 24 Mark Pilgrim (Google) 2013-02-20 06:00:10 PST
(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.
Comment 25 Mark Pilgrim (Google) 2013-02-20 09:28:25 PST
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?
Comment 26 Adam Barth 2013-02-20 09:36:31 PST
Comment on attachment 189141 [details]
Patch

We can just resubmit.
Comment 27 WebKit Review Bot 2013-02-20 10:23:21 PST
Comment on attachment 189141 [details]
Patch

Clearing flags on attachment: 189141

Committed r143473: <http://trac.webkit.org/changeset/143473>
Comment 28 WebKit Review Bot 2013-02-20 10:23:26 PST
All reviewed patches have been landed.  Closing bug.