Keyboard Shortkey for ctrl+insert is not working.

Keyboard Shortkey for ctrl+insert is not working.

Earlier only ctrl+C is handled, I have added key for Ctrl+Insert also
So if their is any selection then it will get copied like that happen
in case of Ctrl+C, That is selected text get copied to clipboard.

BUG=390837
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183575

[Deepak, 2014-10-05 13:14:47]

deepak.m1@samsung.com changed reviewers:
+ aelias@chromium.org, vitalybuka@chromium.org

[Deepak, 2014-10-05 13:14:48]

PTAL

[aelias_OOO_until_Jul13, 2014-10-06 18:39:21]

Can you find a way to write a test for this in WebPluginContainerTest.cpp?

[Vitaly Buka (NO REVIEWS), 2014-10-06 19:05:41]

lgtm

[Deepak, 2014-10-07 04:09:01]

On 2014/10/06 18:39:21, aelias wrote:
> Can you find a way to write a test for this in WebPluginContainerTest.cpp?

@aelias
I referred WebPluginContainerTest.cpp file for taking reference to write test
case.
But I am not able to find any test case related to keyboard event. 
As this change have same functionality as |CTRL+C|. 
It would be ideal to write test case for |CTRL + INSERT| at same place 
where test case for |CTRL+C| has been written.
I am not able to find any test case for |CTRL+C| also. 
Please guide how can I proceed, in case we are looking to add test case for
same.

[aelias_OOO_until_Jul13, 2014-10-07 19:07:35]

It seems there isn't an existing test for Ctrl-C, so can you write a new test
that covers both?

[Deepak, 2014-10-08 15:28:22]

On 2014/10/07 19:07:35, aelias wrote:
> It seems there isn't an existing test for Ctrl-C, so can you write a new test
> that covers both?

I understand that Ctrl-C and Ctrl-Insert have similar test case.
One hand I feel, it involves more maintenance code than actual fix.

I tried following approach:

I am creating the friend function in WebPluginContainerImpl class. As we have
handleKeyboardEvent() private.
I am calling that this friend function from my testcase by passing the pointer
of WebPluginContainerImpl class and KeyboardEvent pointer.
From that friend function I am calling handleKeyboardEvent().

But my test case is getting failed. The sample function is as:

TEST_F(WebPluginContainerTest, Copykeytest)
{
//-------------------Already existed
code--------------------------------------------------
   
URLTestHelpers::registerMockedURLFromBaseURL(WebString::fromUTF8(m_baseURL.c_str()),
WebString::fromUTF8("plugin_container.html"));
    FrameTestHelpers::WebViewHelper webViewHelper;
    WebView* webView = webViewHelper.initializeAndLoad(m_baseURL +
"plugin_container.html", true, new TestPluginWebFrameClient());
    ASSERT(webView);
    webView->settings()->setPluginsEnabled(true);
    webView->resize(WebSize(300, 300));
    webView->layout();
    FrameTestHelpers::runPendingTasks();

    WebElement pluginContainerOneElement =
webView->mainFrame()->document().getElementById(WebString::fromUTF8("translated-plugin"));
//------------------------------------------------------------------------
    RefPtrWillBeRawPtr<KeyboardEvent> keyEvent = KeyboardEvent::create();   //
NEW CODE CHANGED
   
keyEvent->initKeyboardEvent(EventTypeNames::keydown,true,true,0,"U+0003",false,true,false,false,false);
// NEW CODE CHANGED
    
   
handleKeyboardEventTest((WebPluginContainerImpl*)(pluginContainerOneElement.pluginContainer()),
keyEvent.get()); // NEW CODE CHANGED
    EXPECT_EQ(WebString("x"),
Platform::current()->clipboard()->readPlainText(WebClipboard::Buffer()));
}

I am not getting reference of initKeyboardEvent() creation. most of the
references are in the .js files.
Can you please guide me for event creation part,if his approach seems fine to
you, so that I can give a try for this.

Thanks

[Deepak, 2014-10-09 15:21:34]

On 2014/10/08 15:28:22, Deepak wrote:
> On 2014/10/07 19:07:35, aelias wrote:
> > It seems there isn't an existing test for Ctrl-C, so can you write a new
test
> > that covers both?
> 
> I understand that Ctrl-C and Ctrl-Insert have similar test case.
> One hand I feel, it involves more maintenance code than actual fix.
> 
> I tried following approach:
> 
> I am creating the friend function in WebPluginContainerImpl class. As we have
> handleKeyboardEvent() private.
> I am calling that this friend function from my testcase by passing the pointer
> of WebPluginContainerImpl class and KeyboardEvent pointer.
> From that friend function I am calling handleKeyboardEvent().
> 
> But my test case is getting failed. The sample function is as:
> 
> TEST_F(WebPluginContainerTest, Copykeytest)
> {
> //-------------------Already existed
> code--------------------------------------------------
>    
>
URLTestHelpers::registerMockedURLFromBaseURL(WebString::fromUTF8(m_baseURL.c_str()),
> WebString::fromUTF8("plugin_container.html"));
>     FrameTestHelpers::WebViewHelper webViewHelper;
>     WebView* webView = webViewHelper.initializeAndLoad(m_baseURL +
> "plugin_container.html", true, new TestPluginWebFrameClient());
>     ASSERT(webView);
>     webView->settings()->setPluginsEnabled(true);
>     webView->resize(WebSize(300, 300));
>     webView->layout();
>     FrameTestHelpers::runPendingTasks();
> 
>     WebElement pluginContainerOneElement =
>
webView->mainFrame()->document().getElementById(WebString::fromUTF8("translated-plugin"));
> //------------------------------------------------------------------------
>     RefPtrWillBeRawPtr<KeyboardEvent> keyEvent = KeyboardEvent::create();   //
> NEW CODE CHANGED
>    
>
keyEvent->initKeyboardEvent(EventTypeNames::keydown,true,true,0,"U+0003",false,true,false,false,false);
> // NEW CODE CHANGED
>     
>    
>
handleKeyboardEventTest((WebPluginContainerImpl*)(pluginContainerOneElement.pluginContainer()),
> keyEvent.get()); // NEW CODE CHANGED
>     EXPECT_EQ(WebString("x"),
> Platform::current()->clipboard()->readPlainText(WebClipboard::Buffer()));
> }
> 
> I am not getting reference of initKeyboardEvent() creation. most of the
> references are in the .js files.
> Can you please guide me for event creation part,if his approach seems fine to
> you, so that I can give a try for this.
> 
> Thanks

I have implemented the test case for Ctrl-C as well as Ctrl-Insert.
I will update these after verification.
Thanks

[aelias_OOO_until_Jul13, 2014-10-09 18:24:04]

On 2014/10/09 at 15:21:34, deepak.m1 wrote:
> I have implemented the test case for Ctrl-C as well as Ctrl-Insert.
> I will update these after verification.
> Thanks

I don't see a patchset for that, please upload it.

[Deepak, 2014-10-10 03:57:08]

On 2014/10/09 18:24:04, aelias wrote:
> On 2014/10/09 at 15:21:34, deepak.m1 wrote:
> > I have implemented the test case for Ctrl-C as well as Ctrl-Insert.
> > I will update these after verification.
> > Thanks
> 
> I don't see a patchset for that, please upload it.

I have verified and uploaded testcase for |Ctrl-C| as well as |Ctrl-Insert|
cases.
Thanks.
PTAL

[aelias_OOO_until_Jul13, 2014-10-10 04:38:32]

https://codereview.chromium.org/629823002/diff/30001/Source/web/WebPluginCont...
File Source/web/WebPluginContainerImpl.h (right):

https://codereview.chromium.org/629823002/diff/30001/Source/web/WebPluginCont...
Source/web/WebPluginContainerImpl.h:171: friend void
handleCopyKeyboardEventsTest(WebPluginContainerImpl*, KeyboardEvent*);
Can you avoid this by calling handleEvent() instead?

[Deepak, 2014-10-10 06:38:14]

On 2014/10/10 04:38:32, aelias wrote:
>
https://codereview.chromium.org/629823002/diff/30001/Source/web/WebPluginCont...
> File Source/web/WebPluginContainerImpl.h (right):
> 
>
https://codereview.chromium.org/629823002/diff/30001/Source/web/WebPluginCont...
> Source/web/WebPluginContainerImpl.h:171: friend void
> handleCopyKeyboardEventsTest(WebPluginContainerImpl*, KeyboardEvent*);
> Can you avoid this by calling handleEvent() instead?

Thanks for review & suggestion.
I have done changes as suggested.
PTAL

[aelias_OOO_until_Jul13, 2014-10-10 18:32:01]

lgtm, thanks.

[Deepak, 2014-10-11 01:56:19]

The CQ bit was checked by deepak.m1@samsung.com

[commit-bot: I haz the power, 2014-10-11 01:56:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/629823002/140001

[commit-bot: I haz the power, 2014-10-11 02:25:03]

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

[commit-bot: I haz the power, 2014-10-11 02:25:04]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/26718)

[Deepak, 2014-10-11 05:49:04]

The CQ bit was checked by deepak.m1@samsung.com

[commit-bot: I haz the power, 2014-10-11 05:50:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/629823002/270001

[commit-bot: I haz the power, 2014-10-11 06:39:53]

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

[commit-bot: I haz the power, 2014-10-11 06:39:54]

Try jobs failed on following builders:
  mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)

[Deepak, 2014-10-11 07:24:03]

The CQ bit was checked by deepak.m1@samsung.com

[commit-bot: I haz the power, 2014-10-11 07:25:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/629823002/270001

[commit-bot: I haz the power, 2014-10-11 12:48:28]

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

[commit-bot: I haz the power, 2014-10-11 12:48:29]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/31315)

[Deepak, 2014-10-11 12:51:57]

The CQ bit was checked by deepak.m1@samsung.com

[commit-bot: I haz the power, 2014-10-11 12:53:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/629823002/270001

[commit-bot: I haz the power, 2014-10-11 15:48:01]

Committed patchset #5 (id:270001) as 183575