Bug 36487 – Refactoring: HTMLFormControlElement should not have redundant null check

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Hajime Morrita]

HTMLFormControlElement::dispatchBlurEvent() and HTMLFormControlElement::dispatchFocusEvent() has redundant NULL-check.
We can remove it to simplify the code.

[Hajime Morrita]

Created attachment 51413 [details]
a small change for cleanup

[Alexey Proskuryakov]

It looks like this patch introduces a redundant null check. Or am I just confused?

Page* Document::page() const
{
    return m_frame ? m_frame->page() : 0;    
}

[Darin Adler]

Comment on attachment 51413 [details]
a small change for cleanup

The changes to HTMLFormControlElement::dispatchFocusEvent and HTMLFormControlElement::dispatchBlurEvent are OK. Not a great improvement, but nice to use the Document::page helper function to save a little code.

The changes to HTMLCanvasElement::convertLogicalToDevice and HTMLCanvasElement::convertLogicalToDevice are different though. Those are adding a null check of Page*. Presumably, this is a bug fix. Before, there would be a crash if the page was zero. That means we need to construct a test case to demonstrate that crash, and that the crash is fixed.

review-

[Hajime Morrita]

Created attachment 51477 [details]
v1; removed the chagne on HTMLCanvasElemet

[Hajime Morrita]

Thank you for reviewing!

> The changes to HTMLCanvasElement::convertLogicalToDevice and
> HTMLCanvasElement::convertLogicalToDevice are different though. Those are
> adding a null check of Page*. Presumably, this is a bug fix. Before, there
> would be a crash if the page was zero. That means we need to construct a test
> case to demonstrate that crash, and that the crash is fixed.
Agreed. But I could not figure out the test case for crash,
so I remove the change on HTMLCanvasElement.

@ap
Thank you reviewing too.
You are right. I did attempt to simplify the null-check code, 
but It actually introduced a redundant null check at runtime.

Regards.

[WebKit Commit Bot]

Comment on attachment 51477 [details]
v1; removed the chagne on HTMLCanvasElemet

Clearing flags on attachment: 51477

Committed r56463: <http://trac.webkit.org/changeset/56463>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.