Bug 164332 - Give IDBKey(Data) a WTF::Variant overhaul
Summary: Give IDBKey(Data) a WTF::Variant overhaul
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks: 160306
  Show dependency treegraph
 
Reported: 2016-11-02 12:39 PDT by Brady Eidson
Modified: 2016-11-02 16:08 PDT (History)
7 users (show)

See Also:


Attachments
Patch (20.57 KB, patch)
2016-11-02 14:39 PDT, Brady Eidson
achristensen: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-yosemite-wk2 (1.51 MB, application/zip)
2016-11-02 15:43 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2016-11-02 12:39:31 PDT
Give IDBKey(Data) a WTF::Variant overhaul
Comment 1 Brady Eidson 2016-11-02 14:39:32 PDT
Created attachment 293697 [details]
Patch
Comment 2 Alex Christensen 2016-11-02 14:50:20 PDT
Comment on attachment 293697 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/IDBKey.cpp:39
> +IDBKey::IDBKey()
> +    : m_type(KeyType::Invalid)
> +    , m_sizeEstimate(OverheadSize)
> +{
> +}

This should just use initializer lists in the header.
Comment 3 Andy Estes 2016-11-02 14:56:10 PDT
Comment on attachment 293697 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/IDBKey.cpp:89
> +        const auto& array = WTF::get<Vector<RefPtr<IDBKey>>>(m_value);
> +        const auto& otherArray = WTF::get<Vector<RefPtr<IDBKey>>>(other.m_value);

Vector<RefPtr<IDBKey>> could benefit from a type alias.

I would omit the const here, but I don't feel strongly about that. You added similar code below (in IDBKeyData.cpp) that omits const, so maybe at least be consistent.

> Source/WebCore/Modules/indexeddb/IDBKey.cpp:105
> +        double number = WTF::get<double>(m_value);
> +        double otherNumber = WTF::get<double>(other.m_value);

I'd use auto here.
Comment 4 Build Bot 2016-11-02 15:43:13 PDT
Comment on attachment 293697 [details]
Patch

Attachment 293697 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2452648

New failing tests:
svg/wicd/test-rightsizing-b.xhtml
Comment 5 Build Bot 2016-11-02 15:43:16 PDT
Created attachment 293702 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 6 Brady Eidson 2016-11-02 16:07:53 PDT
https://trac.webkit.org/changeset/208310
Comment 7 Brady Eidson 2016-11-02 16:08:53 PDT
(In reply to comment #4)
> Comment on attachment 293697 [details]
> Patch
> 
> Attachment 293697 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-queues.webkit.org/results/2452648
> 
> New failing tests:
> svg/wicd/test-rightsizing-b.xhtml

This failure has nothing to do with this patch.