Add content browser test for <optgroup> with 'display: none'

Add content browser test for <optgroup> with 'display: none'

This patch adds the ExternalPopupMenu content browser 
tests for the changes in https://codereview.chromium.org/415343003/. This 
will be applicable on Mac port, as it uses 
ExternalPopupMenu to show dropdown menu.

BUG=398051
R=keishi@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289045

[spartha, 2014-08-05 16:46:17]

PTAL!

[keishi, 2014-08-06 03:10:56]

Thanks for adding a test! LGTM

https://codereview.chromium.org/447503002/diff/1/content/renderer/external_po...
File content/renderer/external_popup_menu_browsertest.cc (right):

https://codereview.chromium.org/447503002/diff/1/content/renderer/external_po...
content/renderer/external_popup_menu_browsertest.cc:181:
script.append(base::ASCIIToUTF16(".value"));
Maybe if you used selectedIndex, you won't need to add
ExecuteJavaScriptAndReturnStringValue

[spartha, 2014-08-06 05:29:32]

https://codereview.chromium.org/447503002/diff/1/content/renderer/external_po...
File content/renderer/external_popup_menu_browsertest.cc (right):

https://codereview.chromium.org/447503002/diff/1/content/renderer/external_po...
content/renderer/external_popup_menu_browsertest.cc:181:
script.append(base::ASCIIToUTF16(".value"));
Since we are skipping an item because of "display: none", we cannot rely on
selectedIndex. Instead we need to check against the selected value.
On 2014/08/06 03:10:55, keishi wrote:
> Maybe if you used selectedIndex, you won't need to add
> ExecuteJavaScriptAndReturnStringValue

[keishi, 2014-08-06 06:43:27]

https://codereview.chromium.org/447503002/diff/1/content/renderer/external_po...
File content/renderer/external_popup_menu_browsertest.cc (right):

https://codereview.chromium.org/447503002/diff/1/content/renderer/external_po...
content/renderer/external_popup_menu_browsertest.cc:181:
script.append(base::ASCIIToUTF16(".value"));
On 2014/08/06 05:29:32, spartha wrote:
> Since we are skipping an item because of "display: none", we cannot rely on
> selectedIndex. Instead we need to check against the selected value.
> On 2014/08/06 03:10:55, keishi wrote:
> > Maybe if you used selectedIndex, you won't need to add
> > ExecuteJavaScriptAndReturnStringValue
> 

I'm not sure.
After calling "view()->OnSelectPopupMenuItem(1);", mySelect.selectedIndex should
be 2 after your patch.
Before your patch, I think mySelect.selectedIndex would be 0 (because
HTMLSelectElement::listToOptionIndex will return 0)
Isn't that enough to prove it is working?

[spartha, 2014-08-06 06:54:46]

On 2014/08/06 06:43:27, keishi wrote:
>
https://codereview.chromium.org/447503002/diff/1/content/renderer/external_po...
> File content/renderer/external_popup_menu_browsertest.cc (right):
> 
>
https://codereview.chromium.org/447503002/diff/1/content/renderer/external_po...
> content/renderer/external_popup_menu_browsertest.cc:181:
> script.append(base::ASCIIToUTF16(".value"));
> On 2014/08/06 05:29:32, spartha wrote:
> > Since we are skipping an item because of "display: none", we cannot rely on
> > selectedIndex. Instead we need to check against the selected value.
> > On 2014/08/06 03:10:55, keishi wrote:
> > > Maybe if you used selectedIndex, you won't need to add
> > > ExecuteJavaScriptAndReturnStringValue
> > 
> 
> I'm not sure.
> After calling "view()->OnSelectPopupMenuItem(1);", mySelect.selectedIndex
should
> be 2 after your patch.
> Before your patch, I think mySelect.selectedIndex would be 0 (because
> HTMLSelectElement::listToOptionIndex will return 0)
> Isn't that enough to prove it is working?

Yeah. I totally missed that. <select> would have the right index. Thanks!

[spartha, 2014-08-06 11:09:40]

https://codereview.chromium.org/447503002/diff/1/content/renderer/external_po...
File content/renderer/external_popup_menu_browsertest.cc (right):

https://codereview.chromium.org/447503002/diff/1/content/renderer/external_po...
content/renderer/external_popup_menu_browsertest.cc:181:
script.append(base::ASCIIToUTF16(".value"));
On 2014/08/06 06:43:27, keishi wrote:
> On 2014/08/06 05:29:32, spartha wrote:
> > Since we are skipping an item because of "display: none", we cannot rely on
> > selectedIndex. Instead we need to check against the selected value.
> > On 2014/08/06 03:10:55, keishi wrote:
> > > Maybe if you used selectedIndex, you won't need to add
> > > ExecuteJavaScriptAndReturnStringValue
> > 
> 
> I'm not sure.
> After calling "view()->OnSelectPopupMenuItem(1);", mySelect.selectedIndex
should
> be 2 after your patch.
> Before your patch, I think mySelect.selectedIndex would be 0 (because
> HTMLSelectElement::listToOptionIndex will return 0)
> Isn't that enough to prove it is working?

Done.

[spartha, 2014-08-08 05:34:32]

On 2014/08/06 11:09:40, spartha wrote:
>
https://codereview.chromium.org/447503002/diff/1/content/renderer/external_po...
> File content/renderer/external_popup_menu_browsertest.cc (right):
> 
>
https://codereview.chromium.org/447503002/diff/1/content/renderer/external_po...
> content/renderer/external_popup_menu_browsertest.cc:181:
> script.append(base::ASCIIToUTF16(".value"));
> On 2014/08/06 06:43:27, keishi wrote:
> > On 2014/08/06 05:29:32, spartha wrote:
> > > Since we are skipping an item because of "display: none", we cannot rely
on
> > > selectedIndex. Instead we need to check against the selected value.
> > > On 2014/08/06 03:10:55, keishi wrote:
> > > > Maybe if you used selectedIndex, you won't need to add
> > > > ExecuteJavaScriptAndReturnStringValue
> > > 
> > 
> > I'm not sure.
> > After calling "view()->OnSelectPopupMenuItem(1);", mySelect.selectedIndex
> should
> > be 2 after your patch.
> > Before your patch, I think mySelect.selectedIndex would be 0 (because
> > HTMLSelectElement::listToOptionIndex will return 0)
> > Isn't that enough to prove it is working?
> 
> Done.

@Darin, @Ben Need an owner's lgtm! Thanks

[spartha, 2014-08-08 20:00:59]

On 2014/08/08 05:34:32, spartha wrote:
> On 2014/08/06 11:09:40, spartha wrote:
> >
>
https://codereview.chromium.org/447503002/diff/1/content/renderer/external_po...
> > File content/renderer/external_popup_menu_browsertest.cc (right):
> > 
> >
>
https://codereview.chromium.org/447503002/diff/1/content/renderer/external_po...
> > content/renderer/external_popup_menu_browsertest.cc:181:
> > script.append(base::ASCIIToUTF16(".value"));
> > On 2014/08/06 06:43:27, keishi wrote:
> > > On 2014/08/06 05:29:32, spartha wrote:
> > > > Since we are skipping an item because of "display: none", we cannot rely
> on
> > > > selectedIndex. Instead we need to check against the selected value.
> > > > On 2014/08/06 03:10:55, keishi wrote:
> > > > > Maybe if you used selectedIndex, you won't need to add
> > > > > ExecuteJavaScriptAndReturnStringValue
> > > > 
> > > 
> > > I'm not sure.
> > > After calling "view()->OnSelectPopupMenuItem(1);", mySelect.selectedIndex
> > should
> > > be 2 after your patch.
> > > Before your patch, I think mySelect.selectedIndex would be 0 (because
> > > HTMLSelectElement::listToOptionIndex will return 0)
> > > Isn't that enough to prove it is working?
> > 
> > Done.
> 
> @Darin, @Ben Need an owner's lgtm! Thanks

Gentle ping!!

[spartha, 2014-08-09 19:24:24]

The CQ bit was checked by sudarshan.p@samsung.com

[commit-bot: I haz the power, 2014-08-09 19:25:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sudarshan.p@samsung.com/447503002/20001

[commit-bot: I haz the power, 2014-08-09 21:16:49]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[commit-bot: I haz the power, 2014-08-09 21:19:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-09 21:19:37]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[spartha, 2014-08-11 17:05:40]

On 2014/08/09 21:19:37, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

owner's lgtm needed to get this in. Someone PTAL!!

[jochen (gone - plz use gerrit), 2014-08-12 14:37:55]

lgtm

https://codereview.chromium.org/447503002/diff/20001/content/renderer/externa...
File content/renderer/external_popup_menu_browsertest.cc (right):

https://codereview.chromium.org/447503002/diff/20001/content/renderer/externa...
content/renderer/external_popup_menu_browsertest.cc:182: // ,skipping the item
with 'display: none'
nit. , on end of previous line

[spartha, 2014-08-12 14:45:35]

The CQ bit was checked by sudarshan.p@samsung.com

[spartha, 2014-08-12 14:46:34]

Thanks!!

https://codereview.chromium.org/447503002/diff/20001/content/renderer/externa...
File content/renderer/external_popup_menu_browsertest.cc (right):

https://codereview.chromium.org/447503002/diff/20001/content/renderer/externa...
content/renderer/external_popup_menu_browsertest.cc:182: // ,skipping the item
with 'display: none'
On 2014/08/12 14:37:55, jochen (slow - soon OOO) wrote:
> nit. , on end of previous line

Done.

[commit-bot: I haz the power, 2014-08-12 14:46:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sudarshan.p@samsung.com/447503002/40001

[commit-bot: I haz the power, 2014-08-12 19:17:19]

Change committed as 289045