Add UI test for click-to-play.

Add UI test for click-to-play.

BUG=57277
TEST=NPAPIVisiblePluginTester.ClickToPlay:ClickToPlayPluginTest.*

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

[Bernhard Bauer, 2010-10-20 16:03:47]

please review.

[stuartmorgan, 2010-10-20 17:15:29]

Please add the new plugin tests to the Mac valgrind suppression list (NPAPI
tests don't currently run correctly on the Mac under valgrind) as part of this
CL.

Otherwise LGTM. Is it worth adding a test that actually does a simulated click
on the plugin though, rather than calling the function directly? (It won't work
on the Mac yet, but we'll implement clicks there at some point, hopefully soon.)

[Paweł Hajdan Jr., 2010-10-20 17:20:22]

Drive-by with an automation comment.

http://codereview.chromium.org/3539002/diff/19001/20001
File chrome/browser/automation/automation_provider.cc (right):

http://codereview.chromium.org/3539002/diff/19001/20001#newcode632
chrome/browser/automation/automation_provider.cc:632: void
AutomationProvider::LoadBlockedPlugins(int tab_handle, bool* success) {
This should rather live in TestingAutomationProvider. Also, you should add it to
the IPC message handler map.

[Bernhard Bauer, 2010-10-20 17:39:30]

On Wed, Oct 20, 2010 at 19:15,  <stuartmorgan@chromium.org> wrote:
> Please add the new plugin tests to the Mac valgrind suppression list (NPAPI
> tests don't currently run correctly on the Mac under valgrind) as part of
> this
> CL.
>
> Otherwise LGTM. Is it worth adding a test that actually does a simulated
> click
> on the plugin though, rather than calling the function directly? (It won't
> work
> on the Mac yet, but we'll implement clicks there at some point, hopefully
> soon.)

I don't think that would get us a lot. The function is called from
Javascript when the user clicks on the blocked plug-in UI, and I
haven't had any problems with that, just with actually loading the
plug-in and cleaning up the placeholder.

>
> http://codereview.chromium.org/3539002/show
>

[Bernhard Bauer, 2010-10-20 17:42:18]

On Wed, Oct 20, 2010 at 19:20,  <phajdan.jr@chromium.org> wrote:
> Drive-by with an automation comment.
>
>
> http://codereview.chromium.org/3539002/diff/19001/20001
> File chrome/browser/automation/automation_provider.cc (right):
>
> http://codereview.chromium.org/3539002/diff/19001/20001#newcode632
> chrome/browser/automation/automation_provider.cc:632: void
> AutomationProvider::LoadBlockedPlugins(int tab_handle, bool* success) {
> This should rather live in TestingAutomationProvider. Also, you should
> add it to the IPC message handler map.

Pardon the question, what's the difference between
TestingAutomationProvider and vanilla AutomationProvider? And which
IPC message handler map do you mean besides
AutomationProvider::OnMessageReceived (or TestingAutomationProvider,
if I move it)?

>
> http://codereview.chromium.org/3539002/show
>

[Paweł Hajdan Jr., 2010-10-20 19:00:57]

On Wed, Oct 20, 2010 at 19:41, Bernhard Bauer <bauerb@chromium.org> wrote:

> Pardon the question, what's the difference between
> TestingAutomationProvider and vanilla AutomationProvider?


TestingAutomationProvider is used by tests. AutomationProvider is also used
by ChromeFrame, and ideally there should be no testing code inside it.


> And which IPC message handler map do you mean besides
> AutomationProvider::OnMessageReceived (or TestingAutomationProvider,
> if I move it)?


I meant OnMessageReceived. For some reason it was not in the Rietveld diff,
or I have missed it.

[Bernhard Bauer, 2010-10-22 08:35:05]

On 2010/10/20 19:00:57, Paweł Hajdan Jr. wrote:
> On Wed, Oct 20, 2010 at 19:41, Bernhard Bauer <mailto:bauerb@chromium.org>
wrote:
> 
> > Pardon the question, what's the difference between
> > TestingAutomationProvider and vanilla AutomationProvider?
> 
> 
> TestingAutomationProvider is used by tests. AutomationProvider is also used
> by ChromeFrame, and ideally there should be no testing code inside it.

OK. LoadBlockedPlugins is not purely testing code (it's also used by the "Run
all plug-ins this time" button in the blocked plug-in bubble), but if we should
ever need it in ChromeFrame automation, we can move it back.

> 
> > And which IPC message handler map do you mean besides
> > AutomationProvider::OnMessageReceived (or TestingAutomationProvider,
> > if I move it)?
> 
> 
> I meant OnMessageReceived. For some reason it was not in the Rietveld diff,
> or I have missed it.
>

[Paweł Hajdan Jr., 2010-10-22 08:58:25]

Could you upload an updated patchset then?

http://codereview.chromium.org/3539002/diff/19001/20004
File chrome/test/automation/automation_messages_internal.h (right):

http://codereview.chromium.org/3539002/diff/19001/20004#newcode866
chrome/test/automation/automation_messages_internal.h:866:
IPC_SYNC_MESSAGE_ROUTED1_1(AutomationMsg_LoadBlockedPlugins,
This is going to break. Please add all new IPC messages at the bottom of this
file.

[Bernhard Bauer, 2010-10-22 09:29:18]

On 2010/10/22 08:58:25, Paweł Hajdan Jr. wrote:
> Could you upload an updated patchset then?

Already done. I just sent the answer too early ;)

> 
> http://codereview.chromium.org/3539002/diff/19001/20004
> File chrome/test/automation/automation_messages_internal.h (right):
> 
> http://codereview.chromium.org/3539002/diff/19001/20004#newcode866
> chrome/test/automation/automation_messages_internal.h:866:
> IPC_SYNC_MESSAGE_ROUTED1_1(AutomationMsg_LoadBlockedPlugins,
> This is going to break. Please add all new IPC messages at the bottom of this
> file.

What does that break? From casually looking at `git blame
chrome/test/automation/automation_messages_internal.h | grep AutomationMsg_`, it
seems that a bunch of these message were added out of order.

[Bernhard Bauer, 2010-10-22 13:21:08]

On 2010/10/22 08:58:25, Paweł Hajdan Jr. wrote:
> Could you upload an updated patchset then?
> 
> http://codereview.chromium.org/3539002/diff/19001/20004
> File chrome/test/automation/automation_messages_internal.h (right):
> 
> http://codereview.chromium.org/3539002/diff/19001/20004#newcode866
> chrome/test/automation/automation_messages_internal.h:866:
> IPC_SYNC_MESSAGE_ROUTED1_1(AutomationMsg_LoadBlockedPlugins,
> This is going to break. Please add all new IPC messages at the bottom of this
> file.

done.

[Paweł Hajdan Jr., 2010-10-25 12:11:11]

Code I commented in the drive-by LGTM. Thanks!