Bug 153735 - Move properties that use custom bindings to the prototype
Summary: Move properties that use custom bindings to the prototype
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar, WebExposed
Depends on:
Blocks:
 
Reported: 2016-01-31 21:19 PST by Chris Dumez
Modified: 2016-02-01 11:42 PST (History)
8 users (show)

See Also:


Attachments
Patch (15.57 KB, patch)
2016-01-31 21:23 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (15.57 KB, patch)
2016-01-31 21:24 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-yosemite (920.28 KB, application/zip)
2016-01-31 22:15 PST, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (1.08 MB, application/zip)
2016-01-31 22:19 PST, Build Bot
no flags Details
Archive of layout-test-results from ews116 for mac-yosemite (880.54 KB, application/zip)
2016-01-31 22:27 PST, Build Bot
no flags Details
Patch (61.60 KB, patch)
2016-02-01 09:43 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (61.64 KB, patch)
2016-02-01 11:41 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-01-31 21:19:40 PST
Move properties that use custom bindings to the prototype. Whether a property's bindings code is generated or custom-written should not impact where the location of the property.
Comment 1 Chris Dumez 2016-01-31 21:23:42 PST
Created attachment 270370 [details]
Patch
Comment 2 Chris Dumez 2016-01-31 21:24:56 PST
Created attachment 270371 [details]
Patch
Comment 3 Build Bot 2016-01-31 22:15:56 PST
Comment on attachment 270371 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/html/dom/interfaces.html
http/tests/security/cross-frame-access-enumeration.html
imported/w3c/web-platform-tests/dom/interfaces.html
Comment 4 Build Bot 2016-01-31 22:15:59 PST
Created attachment 270373 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 5 Build Bot 2016-01-31 22:19:40 PST
Comment on attachment 270371 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/html/dom/interfaces.html
imported/w3c/web-platform-tests/dom/interfaces.html
Comment 6 Build Bot 2016-01-31 22:19:43 PST
Created attachment 270374 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 7 Build Bot 2016-01-31 22:26:58 PST
Comment on attachment 270371 [details]
Patch

Attachment 270371 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/766492

New failing tests:
http/tests/security/cross-frame-access-enumeration.html
Comment 8 Build Bot 2016-01-31 22:27:02 PST
Created attachment 270375 [details]
Archive of layout-test-results from ews116 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 9 Darin Adler 2016-02-01 08:53:24 PST
Comment on attachment 270371 [details]
Patch

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

> Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp:79
> +    if (JSHTMLDocument::info()->staticPropHashTable) {
> +        if (const HashTableValue* entry = JSHTMLDocument::info()->staticPropHashTable->entry(propertyName)) {
> +            slot.setCacheableCustom(thisObject, entry->attributes(), entry->propertyGetter());
> +            return true;
> +        }
>      }

I’d write:

    if (auto* table = JSHTMLDocument::info()->staticPropHashTable) {
        if (auto* entry = table->entry(propertyName)) {
            slot.setCacheableCustom(thisObject, entry->attributes(), entry->propertyGetter());
            return true;
        }
    }

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:-708
> -    return 1 if HasCustomGetter($attribute->signature->extendedAttributes);
> -    return 1 if HasCustomSetter($attribute->signature->extendedAttributes);

Need to regenerate bindings tests expectations.

> LayoutTests/imported/w3c/ChangeLog:10
> +        * web-platform-tests/XMLHttpRequest/interfaces-expected.txt:

Looks like there are progressions in a couple other tests too.
Comment 10 Chris Dumez 2016-02-01 09:03:57 PST
Comment on attachment 270371 [details]
Patch

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

>> Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp:79
>>      }
> 
> I’d write:
> 
>     if (auto* table = JSHTMLDocument::info()->staticPropHashTable) {
>         if (auto* entry = table->entry(propertyName)) {
>             slot.setCacheableCustom(thisObject, entry->attributes(), entry->propertyGetter());
>             return true;
>         }
>     }

Right, I hesitated. I'll make the change.

>> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:-708
>> -    return 1 if HasCustomSetter($attribute->signature->extendedAttributes);
> 
> Need to regenerate bindings tests expectations.

I keep forgetting, will do.

>> LayoutTests/imported/w3c/ChangeLog:10
>> +        * web-platform-tests/XMLHttpRequest/interfaces-expected.txt:
> 
> Looks like there are progressions in a couple other tests too.

Yes. It surprised me that I did not see more progressions at the time. Now I realize I used a debug build to test and these interfaces.html tests are skipped in Debug builds because they are slow :(
I'll rebase the others.
Comment 11 Chris Dumez 2016-02-01 09:43:46 PST
Created attachment 270394 [details]
Patch
Comment 12 Radar WebKit Bug Importer 2016-02-01 09:44:28 PST
<rdar://problem/24438956>
Comment 13 Radar WebKit Bug Importer 2016-02-01 09:44:29 PST
<rdar://problem/24438955>
Comment 14 WebKit Commit Bot 2016-02-01 10:36:38 PST
Comment on attachment 270394 [details]
Patch

Rejecting attachment 270394 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 270394, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
/git.webkit.org/WebKit
   eac0aff..d73cdd5  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 195966 = eac0afff8b3de43c1b8ca44cff8397b283c0b6f9
r195967 = d73cdd50547addd61f8cd309d5e4beff4222ae44
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/768703
Comment 15 Chris Dumez 2016-02-01 11:41:31 PST
Created attachment 270408 [details]
Patch
Comment 16 Chris Dumez 2016-02-01 11:42:19 PST
Comment on attachment 270408 [details]
Patch

Clearing flags on attachment: 270408

Committed r195969: <http://trac.webkit.org/changeset/195969>
Comment 17 Chris Dumez 2016-02-01 11:42:24 PST
All reviewed patches have been landed.  Closing bug.