[webkit-reviews] review requested: [Bug 44950] [WML] Add create functions to WML : [Attachment 66643] Patch

[bugzilla-daemon at webkit.org]

Gyuyoung Kim <gyuyoung.kim at samsung.com> has asked  for review:
Bug 44950: [WML]  Add create functions to WML
https://bugs.webkit.org/show_bug.cgi?id=44950

Attachment 66643: Patch
https://bugs.webkit.org/attachment.cgi?id=66643&action=review

------- Additional Comments from Gyuyoung Kim <gyuyoung.kim at samsung.com>
>> > WebCore/wml/WMLFormControlElement.h:33
>> >	  WMLFormControlElement(const QualifiedName&, Document*);
>> should be private now.

Ok, I move create(...) to private.

>> Also don't we miss PassRefPtr<WMLFormControlElement> create(..) in the cpp
now?
WMLFormControlElement.cpp already has the create function as below,

  38  PassRefPtr<WMLFormControlElement> WMLFormControlElement::create(const
QualifiedName& tagName,   Document* document)
  39 {
  40	 return adoptRef(new WMLFormControlElement(tagName, document));
  41 }


>> > WebCore/wml/WMLIntrinsicEvent.h:37
>> > +	  static PassRefPtr<WMLIntrinsicEvent> create(const QualifiedName&,
Document*);
>> Dito. Where is this function located? Did you miss it? I didn't see a
WMLIntrinsicEvent(const >> QualifiedName&, Document*) call before. Do we need
this here at all?

WMLIntrinsicEvent.cpp already has create() as below. However, there is no
definition for the create(). So, I define the create(...) in
WMLIntrinsicEvent.h.

 51 PassRefPtr<WMLIntrinsicEvent> WMLIntrinsicEvent::create(const
QualifiedName& tagName, Document* document)
 52 {
 53	return adoptRef(new WMLIntrinsicEvent(tagName, document));
 54 }

In addition, the create() return a WMLIntrinsicEvent instance. But, there is
not define the "new WMLIntrinsicEvent(tagName, document)". So, I add the
construct function as well.

 +WMLIntrinsicEvent::WMLIntrinsicEvent(const QualifiedName& tagName, Document*
document)
 61 +	 : m_taskElement(createTaskElement(document))
 62 +{
 63 +}
 64 +

BTW, there is additional wrong code in WMLDocument.h. I fix it together with
this patch.

 33	 static PassRefPtr<WMLDocument> create(Frame* frame, const KURL& url)
 34	 {
 35 -	     return adoptRef(adoptRef(new WMLDocument(frame, url))));
 36 +	     return adoptRef(new WMLDocument(frame, url));


And, there is a coding style error in WMLIntrinsicEvent.h. I fix it as well.
 
 70 --- a/WebCore/wml/WMLIntrinsicEvent.h
 71 +++ b/WebCore/wml/WMLIntrinsicEvent.h
 72 @@ -22,18 +22,20 @@
 73  #define WMLIntrinsicEvent_h
 74 
 75  #if ENABLE(WML)
 76 +#include "WMLTaskElement.h"
 77 +
 78  #include <wtf/PassRefPtr.h>
 79  #include <wtf/RefCounted.h>
 80  #include <wtf/RefPtr.h>
 81 
 82 -#include "WMLTaskElement.h"

Now, I should go to bed. I will reply your comments tomorrow. :) See you.