HTMLFormControlElement::dispatchBlurEvent() and HTMLFormControlElement::dispatchFocusEvent() has redundant NULL-check. We can remove it to simplify the code.
Created attachment 51413 [details] a small change for cleanup
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; }
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-
Created attachment 51477 [details] v1; removed the chagne on HTMLCanvasElemet
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.
Comment on attachment 51477 [details] v1; removed the chagne on HTMLCanvasElemet Clearing flags on attachment: 51477 Committed r56463: <http://trac.webkit.org/changeset/56463>
All reviewed patches have been landed. Closing bug.