Bug 133606

Summary: Structure should initialize its previousID in its constructor
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, fpizlo, ggaren, mhahnenberg, mmirman, msaboff, oliver, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
the patch
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2
none
same patch. mhahnenberg: review+

Description Mark Lam 2014-06-07 08:21:11 PDT
Currently, the Structure constructor that takes a previous structure will initialize its previousID to point to the previous structure's previousID.  This is incorrect.  However, the caller of the Structure::create() factory method (which instantiated the Structure) will later call setPreviousID() to set the previousID to the correct previous structure.  This makes the code confusing to read and more error prone in that the structure relies on client code to fix its invalid previousID.

This patch fixes this by making the Structure constructor initialize previousID correctly.
Comment 1 Mark Lam 2014-06-07 08:30:26 PDT
Created attachment 232661 [details]
the patch

This patch has passed the jsc tests and the layout tests.
Comment 2 Build Bot 2014-06-07 09:48:16 PDT
Comment on attachment 232661 [details]
the patch

Attachment 232661 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6442432270958592

New failing tests:
media/W3C/video/networkState/networkState_during_loadstart.html
Comment 3 Build Bot 2014-06-07 09:48:19 PDT
Created attachment 232663 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 4 Mark Lam 2014-06-07 18:06:54 PDT
(In reply to comment #2)
> (From update of attachment 232661 [details])
> Attachment 232661 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-queues.appspot.com/results/6442432270958592
> 
> New failing tests:
> media/W3C/video/networkState/networkState_during_loadstart.html

I think this failure is unrelated to this patch.  I've re-run the layout tests without incident.  Will try to trigger the ews one more time to see if this repeats.
Comment 5 Mark Lam 2014-06-07 18:08:25 PDT
Created attachment 232672 [details]
same patch.

Uploading the same identical patch again so that I can try another crack at the EWS bots.
Comment 6 Mark Hahnenberg 2014-06-09 10:51:06 PDT
Comment on attachment 232672 [details]
same patch.

r=me assuming the windows build failure was spurious.
Comment 7 Mark Lam 2014-06-09 11:01:50 PDT
Comment on attachment 232672 [details]
same patch.

Windows build failure is spurious since this patch is identical to the previous, and the previous one built without any issues.  I will land this manually after I svn up.
Comment 8 Mark Lam 2014-06-09 11:08:44 PDT
Thanks for the review.  Landed in r169695: <http://trac.webkit.org/r169695>.