AutoFill forms. We do this by responding to a message from WebKit which send...

AutoFill forms.  We do this by responding to a message from WebKit which sends the (name, label) key requested to fill out a specific field.  The RenderView finds the form to AutoFill using the FormManager and requests the AutoFillManager to fill in the data for the form.  At the end, the FormManager uses this data to fill out the fields in the form on the web page.

BUG=18201
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=39533

[James Hawkins, 2010-02-19 18:46:11]

ping

[jcampan, 2010-02-19 22:02:26]

http://codereview.chromium.org/651002/diff/1046/76
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/651002/diff/1046/76#newcode2590
chrome/renderer/render_view.cc:2590: // TODO(jhawkins): Make these use the
FormManager.
Should you remove this TODO?

http://codereview.chromium.org/651002/diff/1046/75
File chrome/renderer/render_view.h (right):

http://codereview.chromium.org/651002/diff/1046/75#newcode1120
chrome/renderer/render_view.h:1120: scoped_ptr<FormManager> form_manager_;
Why not:
FormManager form_manager_;

[James Hawkins, 2010-02-19 22:11:47]

http://codereview.chromium.org/651002/diff/1046/76
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/651002/diff/1046/76#newcode2590
chrome/renderer/render_view.cc:2590: // TODO(jhawkins): Make these use the
FormManager.
On 2010/02/19 22:02:26, jcampan wrote:
> Should you remove this TODO?

No, it's not quite done yet.  The calls to SendForms and SendPasswordForms
should request the forms from the FormManager.  This will happen soon, but
FormManager speaks in FormData terms while the other two basically roll their
own version of FormData.  I'm going to coalesce all versions into a unified
FormData.

http://codereview.chromium.org/651002/diff/1046/75
File chrome/renderer/render_view.h (right):

http://codereview.chromium.org/651002/diff/1046/75#newcode1120
chrome/renderer/render_view.h:1120: scoped_ptr<FormManager> form_manager_;
On 2010/02/19 22:02:26, jcampan wrote:
> Why not:
> FormManager form_manager_;

To save space in the RenderView object?  I'm never sure whether to use the
object or a pointer to the object in the class.  Your thoughts on this would be
valued.

[jcampan, 2010-02-19 22:24:02]

On Fri, Feb 19, 2010 at 2:11 PM,  <jhawkins@chromium.org> wrote:
>
> http://codereview.chromium.org/651002/diff/1046/76
> File chrome/renderer/render_view.cc (right):
>
> http://codereview.chromium.org/651002/diff/1046/76#newcode2590
> chrome/renderer/render_view.cc:2590: // TODO(jhawkins): Make these use
> the FormManager.
> On 2010/02/19 22:02:26, jcampan wrote:
>>
>> Should you remove this TODO?
>
> No, it's not quite done yet.  The calls to SendForms and
> SendPasswordForms should request the forms from the FormManager.  This
> will happen soon, but FormManager speaks in FormData terms while the
> other two basically roll their own version of FormData.  I'm going to
> coalesce all versions into a unified FormData.
>
> http://codereview.chromium.org/651002/diff/1046/75
> File chrome/renderer/render_view.h (right):
>
> http://codereview.chromium.org/651002/diff/1046/75#newcode1120
> chrome/renderer/render_view.h:1120: scoped_ptr<FormManager>
> form_manager_;
> On 2010/02/19 22:02:26, jcampan wrote:
>>
>> Why not:
>> FormManager form_manager_;
>
> To save space in the RenderView object?  I'm never sure whether to use
> the object or a pointer to the object in the class.  Your thoughts on
> this would be valued.
>
Since you are always creating the FormManager in the RenderView
constructor, I don't see any reason to use a scoped_ptr.
I think making the object part of the class is preferable to using a
pointer, as its life-cycle is straight-forward and you do not run the
risk of having NULL pointers.

Jay

[James Hawkins, 2010-02-19 22:32:50]

http://codereview.chromium.org/651002/diff/1046/75
File chrome/renderer/render_view.h (right):

http://codereview.chromium.org/651002/diff/1046/75#newcode1120
chrome/renderer/render_view.h:1120: scoped_ptr<FormManager> form_manager_;
On 2010/02/19 22:11:48, James Hawkins wrote:
> On 2010/02/19 22:02:26, jcampan wrote:
> > Why not:
> > FormManager form_manager_;
> 
> To save space in the RenderView object?  I'm never sure whether to use the
> object or a pointer to the object in the class.  Your thoughts on this would
be
> valued.

sgtm, I've made this change.

[jcampan, 2010-02-19 22:57:08]

LGTM