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

[bugzilla-daemon at webkit.org]

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





--- Comment #91 from Dane Walllinga <dgwallinga at chromium.org>  2013-03-20 12:24:04 PST ---
(From update of attachment 192285)
View in context: https://bugs.webkit.org/attachment.cgi?id=192285&action=review

>> Source/WebCore/dom/Document.cpp:6077
>> +    if (!frame() || !frame()->page()->chrome()->client()->shouldNotifyOnFormChanges())
> 
> Do we not need to null-check page here?

You tell me. Seems to work fine if we don't, but it's easy enough to add a check. How far down the line would I need to check for non-nullness?

>> Source/WebCore/dom/Document.cpp:6084
>> +void Document::didAssociateFormControlsTimerFired(Timer<Document>*)
> 
> Usually we ASSERT_USED that the argument we get is the timer we expect (in this case m_didAssociateFormControlsTimer).

Do you mean ASSERT_UNUSED?

>> Source/WebCore/dom/Document.cpp:6088
>> +    frame()->page()->chrome()->client()->didAssociateFormControls(m_associatedFormControls);
> 
> or here?

done

>> Source/WebCore/dom/Document.cpp:6089
>> +    m_associatedFormControls.clear();
> 
> I would recommend swapping this set into a local variable before calling didAssociateFormControls in case the client callback ends up calling didAssociateFormControl.  Re-entrancy bugs can be nasty.

done

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