[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 #56 from Dane Walllinga <dgwallinga at chromium.org>  2013-02-26 15:05:13 PST ---
(From update of attachment 190344)
View in context: https://bugs.webkit.org/attachment.cgi?id=190344&action=review

>> 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;
> ...

ah, good old deMorgan

>> 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?

done

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

done

>> Source/WebCore/html/HTMLFormElement.cpp:33
>> +#include "EditorClient.h"
> 
> This doesn't look needed now.

good catch

>> 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.

done

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