[Webkit-unassigned] [Bug 110375] Add client callbacks to notify of changes of associated from controls

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=110375





--- Comment #50 from Elliott Sprehn <esprehn at chromium.org>  2013-02-26 13:02:25 PST ---
(From update of attachment 190344)
View in context: https://bugs.webkit.org/attachment.cgi?id=190344&action=review

Bunch of style things, but this looks fantastic.

> Source/WebCore/dom/Document.cpp:6054
> +    if (this->frame() && this->frame()->page()->chrome()->client()->shouldNotifyOnFormChanges()) {

These two methods should early return, and you don't need the this-> prefix.

if (!frame() || !frame()->...)
  return;
...

> Source/WebCore/dom/Document.h:1577
> +    void didAssociateFormControlsTimerFired(Timer<Document>*);

We don't usually intersperse methods and data members. Could you move the method up higher in the def with other methods?

> Source/WebCore/dom/Document.h:1581
> +    

extra ws.

> Source/WebCore/html/HTMLFormElement.cpp:33
> +#include "EditorClient.h"

This doesn't look needed now.

> Source/WebKit/chromium/src/ChromeClientImpl.cpp:1176
> +    if (m_webView->autofillClient()) {

Early return is usually nicer than wrapping an entire method body in an if statement.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.