Bug 103554 - IndexedDB: Remove duplicate toWireString() and createFromWire() methods in JSC SerializedScriptValue
Summary: IndexedDB: Remove duplicate toWireString() and createFromWire() methods in JS...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 103484
  Show dependency treegraph
 
Reported: 2012-11-28 13:03 PST by Michael Pruett
Modified: 2012-11-28 22:22 PST (History)
4 users (show)

See Also:


Attachments
Patch (7.18 KB, patch)
2012-11-28 15:14 PST, Michael Pruett
haraken: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Patch (13.17 KB, patch)
2012-11-28 17:37 PST, Michael Pruett
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (13.05 KB, patch)
2012-11-28 21:57 PST, Michael Pruett
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Pruett 2012-11-28 13:03:23 PST
In r135022, duplicate createFromWire() and toWireString() methods were added to the JSC version of SerializedScriptValue. This class already has methods of those same names with quite different implementations guarded by ENABLED(INDEXED_DATABASE). In order to allow the JSC SerializedScriptValue to compile when ENABLE(INDEXED_DATABASE) is turned on, these competing definitions should be resolved.
Comment 1 Michael Pruett 2012-11-28 15:14:32 PST
Created attachment 176587 [details]
Patch

This patch fixes compilation of the JSC version of SerializedScriptValue.cpp when ENABLE(INDEXED_DATABASE) is turned on.
Comment 2 Build Bot 2012-11-28 15:48:30 PST
Comment on attachment 176587 [details]
Patch

Attachment 176587 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15027587
Comment 3 Kentaro Hara 2012-11-28 15:59:02 PST
Looks good to me, though you need to expose symbols to other files to fix the build problems (see http://trac.webkit.org/wiki/ExportingSymbols).

jsbell, alecflett: would you take a look?
Comment 4 Build Bot 2012-11-28 16:00:48 PST
Comment on attachment 176587 [details]
Patch

Attachment 176587 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15028593
Comment 5 Joshua Bell 2012-11-28 16:08:09 PST
(In reply to comment #3)
> jsbell, alecflett: would you take a look?

LGTM.
Comment 6 Kentaro Hara 2012-11-28 16:10:32 PST
Comment on attachment 176587 [details]
Patch

Please fix symbol export issues before landing.
Comment 7 Michael Pruett 2012-11-28 17:37:58 PST
Created attachment 176612 [details]
Patch

I've added SerializedScriptValue's toWireString() and createFromWire() to the Windows and Mac symbol export files.
Comment 8 Kentaro Hara 2012-11-28 21:42:07 PST
Comment on attachment 176612 [details]
Patch

ok, now bots look happy.
Comment 9 WebKit Review Bot 2012-11-28 21:46:24 PST
Comment on attachment 176612 [details]
Patch

Rejecting attachment 176612 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

/mnt/git/webkit-commit-queue/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/15013741
Comment 10 Kentaro Hara 2012-11-28 21:47:32 PST
Comment on attachment 176612 [details]
Patch

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

> ChangeLog:6
> +        Reviewed by NOBODY (OOPS!).

Please manually add my name here.

> Source/WebCore/ChangeLog:6
> +        Reviewed by NOBODY (OOPS!).

and here.
Comment 11 Michael Pruett 2012-11-28 21:57:48 PST
Created attachment 176647 [details]
Patch

I have added Kentaro Hara as the reviewer for this patch in the ChangeLog messages.
Comment 12 WebKit Review Bot 2012-11-28 22:22:09 PST
Comment on attachment 176647 [details]
Patch

Clearing flags on attachment: 176647

Committed r136097: <http://trac.webkit.org/changeset/136097>
Comment 13 WebKit Review Bot 2012-11-28 22:22:12 PST
All reviewed patches have been landed.  Closing bug.