Bug 136775

Summary: Generate toFooElementData() using ELEMENT_DATA_TYPE_CASTS
Product: WebKit Reporter: Gyuyoung Kim <gyuyoung.kim>
Component: WebKit Misc.Assignee: Gyuyoung Kim <gyuyoung.kim>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, commit-queue, darin, d-r, esprehn+autocc, fmalita, kangil.han, pdr, schenney, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 136773    
Attachments:
Description Flags
Patch none

Description Gyuyoung Kim 2014-09-12 00:51:43 PDT
SSIA
Comment 1 Gyuyoung Kim 2014-09-12 00:58:18 PDT
Created attachment 238019 [details]
Patch
Comment 2 Gyuyoung Kim 2014-09-13 07:02:41 PDT
CC'ing Darin
Comment 3 Darin Adler 2014-09-14 12:32:46 PDT
Comment on attachment 238019 [details]
Patch

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

> Source/WebCore/dom/ElementData.h:186
> +    TYPE_CASTS_BASE(ToValueTypeName, ElementData, elementData, pointerPredicate, referencePredicate)

We need to fix TYPE_CASTS_BASE. It’s kind of dumb that we need one expression for -> and one for . in every single invocation. We can use *(x) in the macro to dereference the pointer instead. We should go make that change globally to make every single use of the macro simpler.
Comment 4 Gyuyoung Kim 2014-09-14 20:03:12 PDT
(In reply to comment #3)
> (From update of attachment 238019 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=238019&action=review
> 
> > Source/WebCore/dom/ElementData.h:186
> > +    TYPE_CASTS_BASE(ToValueTypeName, ElementData, elementData, pointerPredicate, referencePredicate)
> 
> We need to fix TYPE_CASTS_BASE. It’s kind of dumb that we need one expression for -> and one for . in every single invocation. We can use *(x) in the macro to dereference the pointer instead. We should go make that change globally to make every single use of the macro simpler.

I agree your suggestion. It looks it would be nicer to file a new bug to handle this suggestion. I'm going to file a bug for this.
Comment 5 Gyuyoung Kim 2014-09-14 20:05:33 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 238019 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=238019&action=review
> > 
> > > Source/WebCore/dom/ElementData.h:186
> > > +    TYPE_CASTS_BASE(ToValueTypeName, ElementData, elementData, pointerPredicate, referencePredicate)
> > 
> > We need to fix TYPE_CASTS_BASE. It’s kind of dumb that we need one expression for -> and one for . in every single invocation. We can use *(x) in the macro to dereference the pointer instead. We should go make that change globally to make every single use of the macro simpler.
> 
> I agree your suggestion. It looks it would be nicer to file a new bug to handle this suggestion. I'm going to file a bug for this.

I file a Bug 136809. I'm gonna upload a patch for this.
Comment 6 WebKit Commit Bot 2014-09-14 20:40:27 PDT
Comment on attachment 238019 [details]
Patch

Clearing flags on attachment: 238019

Committed r173610: <http://trac.webkit.org/changeset/173610>
Comment 7 WebKit Commit Bot 2014-09-14 20:40:32 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Darin Adler 2014-09-14 22:13:49 PDT
(In reply to comment #5)
> I file a Bug 136809. I'm gonna upload a patch for this.

Bug 136809 is about .get(); doesn’t sound like the same thing to me. Maybe you filed a different bug about the -> vs. dot thing?
Comment 9 Gyuyoung Kim 2014-09-14 22:21:54 PDT
(In reply to comment #8)
> (In reply to comment #5)
> > I file a Bug 136809. I'm gonna upload a patch for this.
> 
> Bug 136809 is about .get(); doesn’t sound like the same thing to me. Maybe you filed a different bug about the -> vs. dot thing?

Darin, now I understand that you suggested to dereference a pointer in TYPE_CAST_BASE using *(x) use, and then we can adjust it to WebKit globally, right ?

If so, Bug 136809 will handle it. If bug title is a little strange, I will change the title when I prepare a patch for the bug.
Comment 10 Darin Adler 2014-09-14 22:23:35 PDT
Great, looking forward to seeing it.
Comment 11 Gyuyoung Kim 2014-09-14 22:24:39 PDT
(In reply to comment #10)
> Great, looking forward to seeing it.

Now I have very urgent local work, So, I will be able to prepare the patch after going back to my home. :) Please wait for my patch !
Comment 12 Darin Adler 2014-09-14 22:28:14 PDT
Absolutely. I will wait patiently. I may not have a chance to look at it for a while since I have more patch reviewing time on weekends.