Fix a problem that text prefs is not hooked up with CrosSettings.

Fix a problem that text prefs are not hooked up with CrosSettings.

And bump up CppVariant::ToStringVector limit from 100 to 60000 so that we can pass more arguments through chrome.send.

BUG=chromium-os:6827
TEST=Verify proxy setting values entered are sent to CrosSettingsProviderProxy.

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

[zel, 2010-09-17 19:52:50]

LGTM

[kuan_google.com, 2010-09-17 20:00:01]

xiyuan,
thanks very much for helping to fix this!!
but, as we've discussed, running on linux box crashes in
CrosSettingsProviderProxy, so pls let me fix that first b4 u land this
cl..:)
thanks again!

On Fri, Sep 17, 2010 at 12:52 PM, <zelidrag@chromium.org> wrote:

> LGTM
>
>
> http://codereview.chromium.org/3386009/show
>

[kuan, 2010-09-18 01:21:36]

i found a really strange bug, probably because i don't know html/js well enough.
if u add the type=text line before text=url line, Get is not called with the
pacurl input field.
if u add the type=textline after text=url line, Get is not called with the socks
host and port fields.
some other fields may also hv missed the Get call, but i was focusing on those
that i mentioned.

[xiyuan, 2010-09-20 19:10:03]

The problem found by kuan is caused by chrome.sent seems to have a 100-element
limitation of the args list. Thus, prefs are only fetched/observed for the first
99.

I have updated to patch to the fix the problem: use string to pass back the
prefs list instead of array to avoid that chrome.send limitation.

Please take another look. Thanks.

[arv (Not doing code reviews), 2010-09-20 19:14:55]

Can you remove the send limitation instead?

erik


On Mon, Sep 20, 2010 at 12:10,  <xiyuan@chromium.org> wrote:
> The problem found by kuan is caused by chrome.sent seems to have a
> 100-element
> limitation of the args list. Thus, prefs are only fetched/observed for the
> first
> 99.
>
> I have updated to patch to the fix the problem: use string to pass back the
> prefs list instead of array to avoid that chrome.send limitation.
>
> Please take another look. Thanks.
>
> http://codereview.chromium.org/3386009/show
>

[xiyuan, 2010-09-20 20:26:17]

The limitation is introduced in CppVariant::ToStringVector when
DOMUIBindings::send converts CppArgumentList args into a Value.

Inside CppVariant::ToStringVector line 231 - 232, we have this:
      // For sanity, only allow 100 items.
      length = std::min(100, length);

Should I just remove the limitation there?

On 2010/09/20 19:14:55, arv wrote:
> Can you remove the send limitation instead?
> 
> erik
> 
> 
> On Mon, Sep 20, 2010 at 12:10,  <mailto:xiyuan@chromium.org> wrote:
> > The problem found by kuan is caused by chrome.sent seems to have a
> > 100-element
> > limitation of the args list. Thus, prefs are only fetched/observed for the
> > first
> > 99.
> >
> > I have updated to patch to the fix the problem: use string to pass back the
> > prefs list instead of array to avoid that chrome.send limitation.
> >
> > Please take another look. Thanks.
> >
> > http://codereview.chromium.org/3386009/show
> >
>

[arv (Not doing code reviews), 2010-09-20 20:31:10]

I think that makes sense but please talk to the original authors as well.

erik



On Mon, Sep 20, 2010 at 13:26,  <xiyuan@chromium.org> wrote:
> The limitation is introduced in CppVariant::ToStringVector when
> DOMUIBindings::send converts CppArgumentList args into a Value.
>
> Inside CppVariant::ToStringVector line 231 - 232, we have this:
>      // For sanity, only allow 100 items.
>      length = std::min(100, length);
>
> Should I just remove the limitation there?
>
> On 2010/09/20 19:14:55, arv wrote:
>>
>> Can you remove the send limitation instead?
>
>> erik
>
>
>> On Mon, Sep 20, 2010 at 12:10,  <mailto:xiyuan@chromium.org> wrote:
>> > The problem found by kuan is caused by chrome.sent seems to have a
>> > 100-element
>> > limitation of the args list. Thus, prefs are only fetched/observed for
>> > the
>> > first
>> > 99.
>> >
>> > I have updated to patch to the fix the problem: use string to pass back
>> > the
>> > prefs list instead of array to avoid that chrome.send limitation.
>> >
>> > Please take another look. Thanks.
>> >
>> > http://codereview.chromium.org/3386009/show
>> >
>
>
>
>
> http://codereview.chromium.org/3386009/show
>

[xiyuan, 2010-09-20 20:37:58]

+sky, darin

darin, sky asked me to check with you that if it is okay to remove the 100
limitation in CppVariant::ToStringVector.

We hit this in the DOMUI based options page where we try to use chrome.send to
send back an argument list that has more than 100 elements.

Please comment. Thanks.

[sky, 2010-09-20 20:45:03]

Mike might also know.

  -Scott

On Mon, Sep 20, 2010 at 1:37 PM,  <xiyuan@chromium.org> wrote:
> +sky, darin
>
> darin, sky asked me to check with you that if it is okay to remove the 100
> limitation in CppVariant::ToStringVector.
>
> We hit this in the DOMUI based options page where we try to use chrome.send
> to
> send back an argument list that has more than 100 elements.
>
> Please comment. Thanks.
>
>
> http://codereview.chromium.org/3386009/show
>

[xiyuan, 2010-09-21 16:34:34]

+dglazkov

Talked to Dimitry, the CppVarint code is originally for test_shell and not
really intends to use for DOMUI. As a temp solution for now, we will increase
the limit.