BrowserPluginHostTest: Rewrite 3 focus related tests and 1 visibility test to browser_tests.

BrowserPluginHostTest: Rewrite 3 focus related tests and 1 visibility test to browser_tests.

This CL also removes 	BrowserPluginHostTest.HiddenBeforeNavigation, that was disabled under TSAN v2.

BUG=368077, 370240
Test=None, internal only change.

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

[lazyboy, 2014-05-16 01:48:08]

Correct upload this time.

[Fady Samuel, 2014-05-20 15:37:19]

https://codereview.chromium.org/281833003/diff/60001/chrome/test/data/extensi...
File chrome/test/data/extensions/platform_apps/web_view/focus/embedder.js
(right):

https://codereview.chromium.org/281833003/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/platform_apps/web_view/focus/embedder.js:142:
function testFocusBeforeNavigation() {
There's a lot of code here and it's a lot easier for me to review this code if
there is a good description of what it does. What is the flow of control here?

https://codereview.chromium.org/281833003/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/platform_apps/web_view/focus/embedder.js:195:
function testFocusTracksEmbedder() {
Description of what this test does please.

https://codereview.chromium.org/281833003/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/platform_apps/web_view/focus/embedder.js:231:
function testAdvanceFocus() {
There's a lot of code here and it's a lot easier for me to review this code if
there is a good description of what it does. What is the flow of control here?

[lazyboy, 2014-05-20 16:46:51]

https://codereview.chromium.org/281833003/diff/60001/chrome/test/data/extensi...
File chrome/test/data/extensions/platform_apps/web_view/focus/embedder.js
(right):

https://codereview.chromium.org/281833003/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/platform_apps/web_view/focus/embedder.js:142:
function testFocusBeforeNavigation() {
On 2014/05/20 15:37:20, Fady Samuel wrote:
> There's a lot of code here and it's a lot easier for me to review this code if
> there is a good description of what it does. What is the flow of control here?

Done.

https://codereview.chromium.org/281833003/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/platform_apps/web_view/focus/embedder.js:195:
function testFocusTracksEmbedder() {
On 2014/05/20 15:37:20, Fady Samuel wrote:
> Description of what this test does please.

Done.

https://codereview.chromium.org/281833003/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/platform_apps/web_view/focus/embedder.js:231:
function testAdvanceFocus() {
On 2014/05/20 15:37:20, Fady Samuel wrote:
> There's a lot of code here and it's a lot easier for me to review this code if
> there is a good description of what it does. What is the flow of control here?

Done.

[lazyboy, 2014-05-21 16:27:28]

Ping

[Fady Samuel, 2014-05-21 18:02:36]

https://codereview.chromium.org/281833003/diff/100001/chrome/browser/apps/web...
File chrome/browser/apps/web_view_interactive_browsertest.cc (right):

https://codereview.chromium.org/281833003/diff/100001/chrome/browser/apps/web...
chrome/browser/apps/web_view_interactive_browsertest.cc:553:
EXPECT_TRUE(content::ExecuteScript(
Why do we need this?

https://codereview.chromium.org/281833003/diff/100001/chrome/browser/apps/web...
chrome/browser/apps/web_view_interactive_browsertest.cc:561: //
BrowserPluginGuest::OnTakeFocus.
Let's not mention BrowserPlugin details in this patch.

https://codereview.chromium.org/281833003/diff/100001/chrome/browser/apps/web...
chrome/browser/apps/web_view_interactive_browsertest.cc:565: // The browser
plugin will take keyboard focus but it will not
Let's not mention BrowserPlugin in this patch.

https://codereview.chromium.org/281833003/diff/100001/chrome/test/data/extens...
File chrome/test/data/extensions/platform_apps/web_view/focus/inject_focus.js
(right):

https://codereview.chromium.org/281833003/diff/100001/chrome/test/data/extens...
chrome/test/data/extensions/platform_apps/web_view/focus/inject_focus.js:32:
inputElement.addEventListener('focus', function(e) {
Let's remove the listener after it gets called so we don't get weird behavior if
it gets called multiple times?

https://codereview.chromium.org/281833003/diff/100001/chrome/test/data/extens...
chrome/test/data/extensions/platform_apps/web_view/focus/inject_focus.js:36:
inputElement.addEventListener('blur', function(e) {
Let's remove the listener after it gets called so we don't get weird behavior if
it gets called multiple times?

[lazyboy, 2014-05-21 19:19:07]

patchset #7,
Forgot to clean up TestBrowserPluginGuest, done.

https://codereview.chromium.org/281833003/diff/100001/chrome/browser/apps/web...
File chrome/browser/apps/web_view_interactive_browsertest.cc (right):

https://codereview.chromium.org/281833003/diff/100001/chrome/browser/apps/web...
chrome/browser/apps/web_view_interactive_browsertest.cc:553:
EXPECT_TRUE(content::ExecuteScript(
On 2014/05/21 18:02:37, Fady Samuel wrote:
> Why do we need this?

Not needed, removed.

https://codereview.chromium.org/281833003/diff/100001/chrome/browser/apps/web...
chrome/browser/apps/web_view_interactive_browsertest.cc:561: //
BrowserPluginGuest::OnTakeFocus.
On 2014/05/21 18:02:37, Fady Samuel wrote:
> Let's not mention BrowserPlugin details in this patch.

Done.

https://codereview.chromium.org/281833003/diff/100001/chrome/browser/apps/web...
chrome/browser/apps/web_view_interactive_browsertest.cc:565: // The browser
plugin will take keyboard focus but it will not
On 2014/05/21 18:02:37, Fady Samuel wrote:
> Let's not mention BrowserPlugin in this patch.

Done.

https://codereview.chromium.org/281833003/diff/100001/chrome/test/data/extens...
File chrome/test/data/extensions/platform_apps/web_view/focus/inject_focus.js
(right):

https://codereview.chromium.org/281833003/diff/100001/chrome/test/data/extens...
chrome/test/data/extensions/platform_apps/web_view/focus/inject_focus.js:32:
inputElement.addEventListener('focus', function(e) {
On 2014/05/21 18:02:37, Fady Samuel wrote:
> Let's remove the listener after it gets called so we don't get weird behavior
if
> it gets called multiple times?

Wo do rely on multiple call to this listener.
There's a count check in embedder.js that would prevent the weird behavior.

https://codereview.chromium.org/281833003/diff/100001/chrome/test/data/extens...
chrome/test/data/extensions/platform_apps/web_view/focus/inject_focus.js:36:
inputElement.addEventListener('blur', function(e) {
On 2014/05/21 18:02:37, Fady Samuel wrote:
> Let's remove the listener after it gets called so we don't get weird behavior
if
> it gets called multiple times?

Done.

[Fady Samuel, 2014-05-21 19:53:32]

lgtm

[lazyboy, 2014-05-21 20:36:58]

The CQ bit was checked by lazyboy@chromium.org

[commit-bot: I haz the power, 2014-05-21 20:39:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/lazyboy@chromium.org/281833003/120001

[commit-bot: I haz the power, 2014-05-22 03:43:24]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)

[commit-bot: I haz the power, 2014-05-22 10:17:58]

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

[commit-bot: I haz the power, 2014-05-22 10:17:59]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)

[lazyboy, 2014-05-22 10:41:05]

The CQ bit was checked by lazyboy@chromium.org

[commit-bot: I haz the power, 2014-05-22 10:42:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/lazyboy@chromium.org/281833003/120001

[commit-bot: I haz the power, 2014-05-22 11:03:24]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)

[commit-bot: I haz the power, 2014-05-22 11:09:52]

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

[commit-bot: I haz the power, 2014-05-22 11:09:53]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)

[lazyboy, 2014-05-22 14:23:55]

The CQ bit was checked by lazyboy@chromium.org

[commit-bot: I haz the power, 2014-05-22 14:25:46]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/lazyboy@chromium.org/281833003/120001

[commit-bot: I haz the power, 2014-05-22 14:32:50]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)

[commit-bot: I haz the power, 2014-05-22 14:38:28]

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

[commit-bot: I haz the power, 2014-05-22 14:38:28]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)

[lazyboy, 2014-05-22 15:41:09]

The CQ bit was checked by lazyboy@chromium.org

[commit-bot: I haz the power, 2014-05-22 15:42:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/lazyboy@chromium.org/281833003/140001

[commit-bot: I haz the power, 2014-05-22 19:32:17]

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

[commit-bot: I haz the power, 2014-05-22 19:32:18]

Failed to apply patch for content/browser/browser_plugin/browser_plugin_guest.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/browser/browser_plugin/browser_plugin_guest.h
  Hunk #1 FAILED at 322.
  Hunk #2 FAILED at 395.
  2 out of 2 hunks FAILED -- saving rejects to file
content/browser/browser_plugin/browser_plugin_guest.h.rej

Patch:       content/browser/browser_plugin/browser_plugin_guest.h
Index: content/browser/browser_plugin/browser_plugin_guest.h
diff --git a/content/browser/browser_plugin/browser_plugin_guest.h
b/content/browser/browser_plugin/browser_plugin_guest.h
index
b3466c0d143f9341c503817f877b81c55b14df98..3f595e4450d6c696bf4230c764e9581bb45a9880
100644
--- a/content/browser/browser_plugin/browser_plugin_guest.h
+++ b/content/browser/browser_plugin/browser_plugin_guest.h
@@ -322,11 +322,9 @@ class CONTENT_EXPORT BrowserPluginGuest
   void OnNavigateGuest(int instance_id, const std::string& src);
   void OnPluginDestroyed(int instance_id);
   // Resizes the guest's web contents.
-  // Overridden in tests.
-  virtual void OnResizeGuest(
+  void OnResizeGuest(
       int instance_id, const BrowserPluginHostMsg_ResizeGuest_Params& params);
-  // Overridden in tests.
-  virtual void OnSetFocus(int instance_id, bool focused);
+  void OnSetFocus(int instance_id, bool focused);
   // Sets the name of the guest so that other guests in the same partition can
   // access it.
   void OnSetName(int instance_id, const std::string& name);
@@ -395,8 +393,7 @@ class CONTENT_EXPORT BrowserPluginGuest
   void OnShowPopup(const ViewHostMsg_ShowPopup_Params& params);
 #endif
   void OnShowWidget(int route_id, const gfx::Rect& initial_pos);
-  // Overridden in tests.
-  virtual void OnTakeFocus(bool reverse);
+  void OnTakeFocus(bool reverse);
   void OnUpdateFrameName(int frame_id,
                          bool is_top_level,
                          const std::string& name);

[lazyboy, 2014-05-22 19:46:48]

The CQ bit was checked by lazyboy@chromium.org

[commit-bot: I haz the power, 2014-05-22 19:47:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/lazyboy@chromium.org/281833003/160001

[commit-bot: I haz the power, 2014-05-23 01:38:55]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-23 07:17:19]

Change committed as 272427