Browser tests for OOPIF support for drag-n-drop.

Initial browser tests for OOPIF support for drag-n-drop.

This CL adds a single test - dragging and dropping a piece of text from
outside of the browser into a cross-site frame.  The test expectedly
fails for now in --site-per-process mode (the CL adds an exception
in site-per-process.content_browsertests.filter).

DOMMessageQueue
===============

The new test verifies whether appropriate javascript events have fired.
This is done by DOMMessageQueue, which has been tweaked slightly to
target only a specified WebContents and detect renderer crashes.

The modified DOMMessageQueue makes the old DOMOperationObserver
obsolete, so the CL deletes the old class and switches its users
to use DOMMessageQueue instead.

DomAutomationController::SendMsg was refusing to serialize and send
javascript objects.  This restriction has been lifted (any object
type can be sent, as long as it can be serialized to JSON).

Simulation of drag-and-drop
===========================

The CL introduces a DragAndDropSimulator that helps with simulating
drag-and-drop events for WebContents.  The simulator supports only
the minimum functionality required to enable the first OOPIF test:

- Currently the simulator only supports drag-enter (dragging something
  from outside the browser into WebContenst) and drop events.

- Currently the simulator only works when WebContents is built on top
  of aura UI abstraction/platform.

Test pages
==========

The CL provides a main test page (content/test/data/drag_and_drop/page.html)
which enables:

- Easy locating of subframes (which are positioned at fixed coordinates).

- Easy replacing of subframe contents, which are specified by URL query.

  - This helps with testing various drag-and-drop scenarios (e.g. using
    different subframes for testing drop with drop_target.html OR for
    testing drag-start with image_source.html).

  - This also helps exercise various OOPIF scenarios (e.g. using
    subframes that are same-site or cross-site).

BUG=647249
Committed: https://crrev.com/f1cf6923ce29e2995e7deefa654d299fd5512d28
Cr-Commit-Position: refs/heads/master@{#429472}

[Łukasz Anforowicz, 2016-10-31 19:54:58]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-31 19:55:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-31 20:03:02]

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

[commit-bot: I haz the power, 2016-10-31 20:03:03]

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

[Łukasz Anforowicz, 2016-10-31 20:28:18]

Description was changed from

==========
Browser tests for OOPIF support for drag-n-drop.

BUG=647249
==========

to

==========
Initial browser tests for OOPIF support for drag-n-drop.

This CL adds a single test - dragging and dropping a piece of text from
outside of the browser into a cross-site frame.  The test expectedly
fails for now in --site-per-process mode (the CL adds an exception
in site-per-process.content_browsertests.filter).


DOMAutomationWaiter
===================

The new test verifies whether appropriate javascript events have fired.
This is done by DOMAutomationWaiter, which is not exposed by
content/public/test/browser_test_utils.h.  This class is based on
the old DOMOperationObserver class that used to be hidden in an
anonymous namespace inside browser_test_utils.cc.

To let DOMAutomationWaiter monitor a single frame (rather than a whole
WebContents), it has been changed to directly handle IPC messages
(where the exact sender / RenderFrameHost is known), rather than
relying on broad NOTIFICATION_DOM_OPERATION_RESPONSE notifications
(where WebContents is used as the notification source).

In addition to the changes above, some small refactorings were done:

- JSON parsing has been folded/hidden into DOMAutomationWaiter
  (it used to be done by users of this class)

- Rather than listen to WCO::RenderProcessGone, the new class listens
  to WCO::RenderFrameDeleted (which handles frame removal as well
  as a crash of a renderer process, but is resilient to process swaps).

- did_respond_ boolean field has been removed - it is sufficient
  to check if response_ field has been populated.

- DomAutomationController::SendMsg was refusing to serialize and send
  javascript objects.  This restriction has been lifted (any object
  type can be sent, as long as it can be serialized to JSON).

- ShouldStopWaiting method has been introduced, to enable subclasses
  to ignore some responses (e.g. when waiting for response indicating
  a particular event).


Simulation of drag-and-drop
===========================

The CL introduces a DragAndDropSimulator that helps with simulating
drag-and-drop events for WebContents.  The simulator supports only
the minimum functionality required to enable the first OOPIF test:

- Currently the simulator only supports drag-enter (dragging something
  from outside the browser into WebContenst) and drop events.

- Currently the simulator only works when WebContents is built on top
  of aura UI abstraction/platform.


Test pages
==========

The CL provides a main test page (content/test/data/drag_and_drop/page.html)
which enables:

- Easy targeting of subframes (positioned at fixed coordinates)

- Easy replacing of subframe contents, which are specified by URL query.

  - This helps with testing various drag-and-drop scenarios (e.g. using
    different subframes for testing drop with drop_target.html OR for
    testing drag-start with image_source.html).

  - This also helps exercise various OOPIF scenarios (e.g. using
    subframes that are same-site or cross-site).


BUG=647249
==========

[Łukasz Anforowicz, 2016-10-31 20:54:52]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-31 20:55:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-31 21:06:31]

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

[commit-bot: I haz the power, 2016-10-31 21:06:32]

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

[Łukasz Anforowicz, 2016-10-31 23:59:40]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-01 00:00:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-01 01:13:38]

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

[commit-bot: I haz the power, 2016-11-01 01:13:39]

Dry run: This issue passed the CQ dry run.

[Łukasz Anforowicz, 2016-11-01 17:52:24]

Description was changed from

==========
Initial browser tests for OOPIF support for drag-n-drop.

This CL adds a single test - dragging and dropping a piece of text from
outside of the browser into a cross-site frame.  The test expectedly
fails for now in --site-per-process mode (the CL adds an exception
in site-per-process.content_browsertests.filter).


DOMAutomationWaiter
===================

The new test verifies whether appropriate javascript events have fired.
This is done by DOMAutomationWaiter, which is not exposed by
content/public/test/browser_test_utils.h.  This class is based on
the old DOMOperationObserver class that used to be hidden in an
anonymous namespace inside browser_test_utils.cc.

To let DOMAutomationWaiter monitor a single frame (rather than a whole
WebContents), it has been changed to directly handle IPC messages
(where the exact sender / RenderFrameHost is known), rather than
relying on broad NOTIFICATION_DOM_OPERATION_RESPONSE notifications
(where WebContents is used as the notification source).

In addition to the changes above, some small refactorings were done:

- JSON parsing has been folded/hidden into DOMAutomationWaiter
  (it used to be done by users of this class)

- Rather than listen to WCO::RenderProcessGone, the new class listens
  to WCO::RenderFrameDeleted (which handles frame removal as well
  as a crash of a renderer process, but is resilient to process swaps).

- did_respond_ boolean field has been removed - it is sufficient
  to check if response_ field has been populated.

- DomAutomationController::SendMsg was refusing to serialize and send
  javascript objects.  This restriction has been lifted (any object
  type can be sent, as long as it can be serialized to JSON).

- ShouldStopWaiting method has been introduced, to enable subclasses
  to ignore some responses (e.g. when waiting for response indicating
  a particular event).


Simulation of drag-and-drop
===========================

The CL introduces a DragAndDropSimulator that helps with simulating
drag-and-drop events for WebContents.  The simulator supports only
the minimum functionality required to enable the first OOPIF test:

- Currently the simulator only supports drag-enter (dragging something
  from outside the browser into WebContenst) and drop events.

- Currently the simulator only works when WebContents is built on top
  of aura UI abstraction/platform.


Test pages
==========

The CL provides a main test page (content/test/data/drag_and_drop/page.html)
which enables:

- Easy targeting of subframes (positioned at fixed coordinates)

- Easy replacing of subframe contents, which are specified by URL query.

  - This helps with testing various drag-and-drop scenarios (e.g. using
    different subframes for testing drop with drop_target.html OR for
    testing drag-start with image_source.html).

  - This also helps exercise various OOPIF scenarios (e.g. using
    subframes that are same-site or cross-site).


BUG=647249
==========

to

==========
Initial browser tests for OOPIF support for drag-n-drop.

This CL adds a single test - dragging and dropping a piece of text from
outside of the browser into a cross-site frame.  The test expectedly
fails for now in --site-per-process mode (the CL adds an exception
in site-per-process.content_browsertests.filter).


DOMAutomationWaiter
===================

The new test verifies whether appropriate javascript events have fired.
This is done by DOMAutomationWaiter, which is not exposed by
content/public/test/browser_test_utils.h.  This class is based on
the old DOMOperationObserver class that used to be hidden in an
anonymous namespace inside browser_test_utils.cc.

In addition to the changes above, some small refactorings were done:

- JSON parsing has been folded/hidden into DOMAutomationWaiter
  (it used to be done by users of this class)

- did_respond_ boolean field has been removed - it is sufficient
  to check if response_ field has been populated.

- DomAutomationController::SendMsg was refusing to serialize and send
  javascript objects.  This restriction has been lifted (any object
  type can be sent, as long as it can be serialized to JSON).

- ShouldStopWaiting method has been introduced, to enable subclasses
  to ignore some responses (e.g. when waiting for response indicating
  a particular event).


Simulation of drag-and-drop
===========================

The CL introduces a DragAndDropSimulator that helps with simulating
drag-and-drop events for WebContents.  The simulator supports only
the minimum functionality required to enable the first OOPIF test:

- Currently the simulator only supports drag-enter (dragging something
  from outside the browser into WebContenst) and drop events.

- Currently the simulator only works when WebContents is built on top
  of aura UI abstraction/platform.


Test pages
==========

The CL provides a main test page (content/test/data/drag_and_drop/page.html)
which enables:

- Easy locating of subframes (which are positioned at fixed coordinates).

- Easy replacing of subframe contents, which are specified by URL query.

  - This helps with testing various drag-and-drop scenarios (e.g. using
    different subframes for testing drop with drop_target.html OR for
    testing drag-start with image_source.html).

  - This also helps exercise various OOPIF scenarios (e.g. using
    subframes that are same-site or cross-site).


BUG=647249
==========

[ncarter (slow), 2016-11-01 19:50:51]

nick@chromium.org changed reviewers:
+ nick@chromium.org

[ncarter (slow), 2016-11-01 19:50:53]

https://codereview.chromium.org/2453693003/diff/120001/content/browser/web_co...
File content/browser/web_contents/drag_and_drop_browsertest.cc (right):

https://codereview.chromium.org/2453693003/diff/120001/content/browser/web_co...
content/browser/web_contents/drag_and_drop_browsertest.cc:162: class
DOMDragEventWaiter : public DOMAutomationWaiter {
I think this would be cleaner if this used composition instead of inheritance.

https://codereview.chromium.org/2453693003/diff/120001/content/browser/web_co...
content/browser/web_contents/drag_and_drop_browsertest.cc:197: bool
ShouldStopWaiting(const base::Value& value) override {
Couldn't you just do this, with no changes to DOMMessageQueue?

DOMMessageQueue message_queue;
// ...
std::string message;
ASSERT_TRUE(message_queue.WaitForMessage(&message));
ASSERT_EQ(message,
  "{\"event_type\": \"foo\", \"window_name\": \"bar\"}");

https://codereview.chromium.org/2453693003/diff/120001/content/public/test/br...
File content/public/test/browser_test_utils.h (right):

https://codereview.chromium.org/2453693003/diff/120001/content/public/test/br...
content/public/test/browser_test_utils.h:195: class DOMAutomationWaiter : public
NotificationObserver,
Can we do this as an enhancement to the existing DOMMessageQueue? They seem to
be offering equivalent functionality, with slightly different surfacing.

 - ctor that targets a particular WebContents (or RFH even) is a nice
enhancement.
 - The handling of crashes is a huge improvement.
 - As written, it's not possible to use DOMAutomationWaiter to obtain more than
one message (this matters in cases like this:
https://cs.chromium.org/chromium/src/content/browser/site_per_process_browser...
when you're waiting for two events that could arrive in either order.)
 - not clear how test-friendly base::Value is.

 - Maybe DOMMessageQueue could have a variant like this, which is how most
places use it in practice:

WaitForStringEqualing(std::string expected) {
  while (this->WaitForMessage(&message)) {
    if (message == "\"" + expected + "\"")
      return;
  }
}

https://codereview.chromium.org/2453693003/diff/120001/content/public/test/br...
content/public/test/browser_test_utils.h:223: // WebContentsObserver overrides,
, -> .

[Łukasz Anforowicz, 2016-11-01 20:38:53]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-01 20:39:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-01 21:06:37]

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

[commit-bot: I haz the power, 2016-11-01 21:06:38]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Łukasz Anforowicz, 2016-11-01 22:16:51]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-01 22:17:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Łukasz Anforowicz, 2016-11-01 23:43:42]

Thanks Nick.  I've replaced DOMAutomationWaiter with DOMMessageQueue - I think
the CL should be ready for another round of review please?

https://codereview.chromium.org/2453693003/diff/120001/content/browser/web_co...
File content/browser/web_contents/drag_and_drop_browsertest.cc (right):

https://codereview.chromium.org/2453693003/diff/120001/content/browser/web_co...
content/browser/web_contents/drag_and_drop_browsertest.cc:62: //     the browser
(without leaving OS drag&drop machinery in a weird state).
We'll have to work through the items above, but I think it is fine to land the
CL as-is (the test it enables already expose drag-and-drop OOPIF support issues
[for the hangouts scenario]) and work on other test scenarios afterwards (most
of the CL shouldn't change - the main question is about the right way to
implement DragAndDropSimulator).

https://codereview.chromium.org/2453693003/diff/120001/content/public/test/br...
File content/public/test/browser_test_utils.h (right):

https://codereview.chromium.org/2453693003/diff/120001/content/public/test/br...
content/public/test/browser_test_utils.h:195: class DOMAutomationWaiter : public
NotificationObserver,
On 2016/11/01 19:50:52, ncarter wrote:
> Can we do this as an enhancement to the existing DOMMessageQueue? They seem to
> be offering equivalent functionality, with slightly different surfacing.
> 
>  - ctor that targets a particular WebContents (or RFH even) is a nice
> enhancement.
>  - The handling of crashes is a huge improvement.
>  - As written, it's not possible to use DOMAutomationWaiter to obtain more
than
> one message (this matters in cases like this:
>
https://cs.chromium.org/chromium/src/content/browser/site_per_process_browser...
> when you're waiting for two events that could arrive in either order.)

Agreed that DOMAutomationWaiter can be scraped and replaced with
DOMMessageQueue.

>  - not clear how test-friendly base::Value is.

I am not sure.  FWIW, I switched to using testing::HasSubstr and/or
base::MatchPattern (instead of parsing JSON).

>  - Maybe DOMMessageQueue could have a variant like this, which is how most
> places use it in practice:
> 
> WaitForStringEqualing(std::string expected) {
>   while (this->WaitForMessage(&message)) {
>     if (message == "\"" + expected + "\"")
>       return;
>   }
> }

DOMDragEventWaiter needs pattern matching (rather than straight-up equality) so
I didn't add anything like this just yet.

https://codereview.chromium.org/2453693003/diff/120001/content/public/test/br...
content/public/test/browser_test_utils.h:220: virtual bool
ShouldStopWaiting(const base::Value& response);
Yesterday I read an article arguing for minimizing the number of member methods
in general (*) and I've started wondering whether the ShouldStopWaiting method
could be removed (and replaced by DOMDragEventWaiter in
drag_and_drop_browsertest.cc calling WaitAndGetResponse in a loop, until it gets
an expected response).

(*) http://www.gotw.ca/gotw/084.htm

https://codereview.chromium.org/2453693003/diff/120001/content/public/test/br...
content/public/test/browser_test_utils.h:223: // WebContentsObserver overrides,
On 2016/11/01 19:50:52, ncarter wrote:
> , -> .

Acknowledged.  (not applicable anymore after removal of DOMAutomationWaiter)

https://codereview.chromium.org/2453693003/diff/120001/content/test/BUILD.gn
File content/test/BUILD.gn (right):

https://codereview.chromium.org/2453693003/diff/120001/content/test/BUILD.gn#...
content/test/BUILD.gn:820: sources += [
"../browser/web_contents/drag_and_drop_browsertest.cc" ]
Hopefully aura-only only is okay for now.  aura-only means that we are ignoring
web_contents_view_android.cc and web_contents/web_drag_*_mac.mm

Also - for thinking about the proper level to do the drag-and-drop simulation,
it might be useful to note chrome/test/base/interactive_test_utils* which come
in multiple platform-specific flavors.  For now the new browser test can live in
the //content layer though, so landing the CL as-is seems okay.

https://codereview.chromium.org/2453693003/diff/120001/content/test/data/drag...
File content/test/data/drag_and_drop/event_monitoring.js (right):

https://codereview.chromium.org/2453693003/diff/120001/content/test/data/drag...
content/test/data/drag_and_drop/event_monitoring.js:17:
window.domAutomationController.setAutomationId(0);
The line above tells the renderer to enable window.domAutomationController.send
to send IPCs to the browser.  The exact value of the argument (0) is mostly
ignored (see DOMAutomationController::automation_id_) and only distinguishes
"armed" (!= MSG_ROUTING_NONE) and "disarmed / already responded" (==
MSG_ROUTING_NONE).  Blocking IPCs via setAutomationId is useful for a
fire-once-only ExecuteScriptAndExtractString (although for a request/response we
should instead send some kind of a nonce - with the current scheme unrelated
messages/responses can still get through).

As a follow-up I want to try removing the setAutomationId method altogether.  In
most cases it seems to be always followed by an almost immediate
window.domAutomationController.send call.

[Łukasz Anforowicz, 2016-11-01 23:44:45]

Description was changed from

==========
Initial browser tests for OOPIF support for drag-n-drop.

This CL adds a single test - dragging and dropping a piece of text from
outside of the browser into a cross-site frame.  The test expectedly
fails for now in --site-per-process mode (the CL adds an exception
in site-per-process.content_browsertests.filter).


DOMAutomationWaiter
===================

The new test verifies whether appropriate javascript events have fired.
This is done by DOMAutomationWaiter, which is not exposed by
content/public/test/browser_test_utils.h.  This class is based on
the old DOMOperationObserver class that used to be hidden in an
anonymous namespace inside browser_test_utils.cc.

In addition to the changes above, some small refactorings were done:

- JSON parsing has been folded/hidden into DOMAutomationWaiter
  (it used to be done by users of this class)

- did_respond_ boolean field has been removed - it is sufficient
  to check if response_ field has been populated.

- DomAutomationController::SendMsg was refusing to serialize and send
  javascript objects.  This restriction has been lifted (any object
  type can be sent, as long as it can be serialized to JSON).

- ShouldStopWaiting method has been introduced, to enable subclasses
  to ignore some responses (e.g. when waiting for response indicating
  a particular event).


Simulation of drag-and-drop
===========================

The CL introduces a DragAndDropSimulator that helps with simulating
drag-and-drop events for WebContents.  The simulator supports only
the minimum functionality required to enable the first OOPIF test:

- Currently the simulator only supports drag-enter (dragging something
  from outside the browser into WebContenst) and drop events.

- Currently the simulator only works when WebContents is built on top
  of aura UI abstraction/platform.


Test pages
==========

The CL provides a main test page (content/test/data/drag_and_drop/page.html)
which enables:

- Easy locating of subframes (which are positioned at fixed coordinates).

- Easy replacing of subframe contents, which are specified by URL query.

  - This helps with testing various drag-and-drop scenarios (e.g. using
    different subframes for testing drop with drop_target.html OR for
    testing drag-start with image_source.html).

  - This also helps exercise various OOPIF scenarios (e.g. using
    subframes that are same-site or cross-site).


BUG=647249
==========

to

==========
Initial browser tests for OOPIF support for drag-n-drop.

This CL adds a single test - dragging and dropping a piece of text from
outside of the browser into a cross-site frame.  The test expectedly
fails for now in --site-per-process mode (the CL adds an exception
in site-per-process.content_browsertests.filter).


DOMMessageQueue
===============

The new test verifies whether appropriate javascript events have fired.
This is done by DOMMessageQueue, which has been tweaked slightly to
target only a specified WebContents and detect renderer crashes.

The modified DOMMessageQueue makes the old DOMOperationObserver
obsolete, so the CL deletes the old class and switches its users
to use DOMMessageQueue instead.

DomAutomationController::SendMsg was refusing to serialize and send
javascript objects.  This restriction has been lifted (any object
type can be sent, as long as it can be serialized to JSON).


Simulation of drag-and-drop
===========================

The CL introduces a DragAndDropSimulator that helps with simulating
drag-and-drop events for WebContents.  The simulator supports only
the minimum functionality required to enable the first OOPIF test:

- Currently the simulator only supports drag-enter (dragging something
  from outside the browser into WebContenst) and drop events.

- Currently the simulator only works when WebContents is built on top
  of aura UI abstraction/platform.


Test pages
==========

The CL provides a main test page (content/test/data/drag_and_drop/page.html)
which enables:

- Easy locating of subframes (which are positioned at fixed coordinates).

- Easy replacing of subframe contents, which are specified by URL query.

  - This helps with testing various drag-and-drop scenarios (e.g. using
    different subframes for testing drop with drop_target.html OR for
    testing drag-start with image_source.html).

  - This also helps exercise various OOPIF scenarios (e.g. using
    subframes that are same-site or cross-site).


BUG=647249
==========

[commit-bot: I haz the power, 2016-11-01 23:57:52]

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

[commit-bot: I haz the power, 2016-11-01 23:57:53]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[alexmos, 2016-11-02 17:44:17]

alexmos@chromium.org changed reviewers:
+ alexmos@chromium.org

[alexmos, 2016-11-02 17:44:18]

Drive-by thought: I haven't looked at the test in detail, so not entirely sure
how it works, but should something like this be in interactive_ui_tests instead?
 Could a window focus change in the middle of the test cause the drag and drop
to be interrupted?  We've had issues with focus tests being flaky in
content_browsertests in the past (e.g., https://crbug.com/559273), and drag and
drop seems similarly fragile.

[ncarter (slow), 2016-11-02 19:11:45]

lgtm

https://codereview.chromium.org/2453693003/diff/160001/content/browser/web_co...
File content/browser/web_contents/drag_and_drop_browsertest.cc (right):

https://codereview.chromium.org/2453693003/diff/160001/content/browser/web_co...
content/browser/web_contents/drag_and_drop_browsertest.cc:69:
DragAndDropSimulator(WebContents* web_contents)
explicit

https://codereview.chromium.org/2453693003/diff/160001/content/browser/web_co...
content/browser/web_contents/drag_and_drop_browsertest.cc:270: RenderFrameHost*
GetFrameByName(const std::string& name_to_find) {
Can't you use FrameTree::FindByName()?

WebContentsImpl* web_contents =
        static_cast<WebContentsImpl*>(shell()->web_contents());
FrameTree* frame_tree = web_contents->GetFrameTree();
return frame_tree->FindByName(name_to_find);

https://codereview.chromium.org/2453693003/diff/160001/content/public/test/br...
File content/public/test/browser_test_utils.h (right):

https://codereview.chromium.org/2453693003/diff/160001/content/public/test/br...
content/public/test/browser_test_utils.h:448: void
RenderProcessGone(base::TerminationStatus status) override;
Usually I would leave these as public, since they're public in the base class,
and callable through that interface.

[Łukasz Anforowicz, 2016-11-02 21:03:33]

Thanks for reviewing.

On 2016/11/02 17:44:18, alexmos wrote:
> Drive-by thought: I haven't looked at the test in detail, so not entirely sure
> how it works, but should something like this be in interactive_ui_tests
instead?
>  Could a window focus change in the middle of the test cause the drag and drop
> to be interrupted?  We've had issues with focus tests being flaky in
> content_browsertests in the past (e.g., https://crbug.com/559273), and drag
and
> drop seems similarly fragile.

This is a good question.  I think current tests should be immune to these kinds
of issues - aura is used to find aura::client::DragDropDelegate but then the
drag-and-drop events are simulated by directly calling methods in the delegate
(WebContentsViewAura).  FWIW, I tried running the test with an artificial 10
seconds delay (posting MessageLoopRunner::QuitClosure with a delay) in the
middle of the test and couldn't disrupt the test with various mouse actions and
focus switches.

https://codereview.chromium.org/2453693003/diff/160001/content/browser/web_co...
File content/browser/web_contents/drag_and_drop_browsertest.cc (right):

https://codereview.chromium.org/2453693003/diff/160001/content/browser/web_co...
content/browser/web_contents/drag_and_drop_browsertest.cc:69:
DragAndDropSimulator(WebContents* web_contents)
On 2016/11/02 at 19:11:45, ncarter wrote:
> explicit

done.  (it's really sad that I almost always forget about this piece... :-/  I
did remember for the new constructor of DOMMessageQueue though, so maybe one day
I'll learn...)

https://codereview.chromium.org/2453693003/diff/160001/content/browser/web_co...
content/browser/web_contents/drag_and_drop_browsertest.cc:270: RenderFrameHost*
GetFrameByName(const std::string& name_to_find) {
On 2016/11/02 19:11:45, ncarter wrote:
> Can't you use FrameTree::FindByName()?
> 
> WebContentsImpl* web_contents =
>         static_cast<WebContentsImpl*>(shell()->web_contents());
> FrameTree* frame_tree = web_contents->GetFrameTree();
> return frame_tree->FindByName(name_to_find);

Ooops.  Done.  (I did look for something like this, but only in WebContents and
didn't think that I should have looked inside WebContentsImpl / FrameTree).

I wonder if I'll have to undo this though, when I eventually might have to
simulate some things via mouse (and therefore might have to move these
browsertests to //chrome layer and use only the public part of //content API).

https://codereview.chromium.org/2453693003/diff/160001/content/public/test/br...
File content/public/test/browser_test_utils.h (right):

https://codereview.chromium.org/2453693003/diff/160001/content/public/test/br...
content/public/test/browser_test_utils.h:448: void
RenderProcessGone(base::TerminationStatus status) override;
On 2016/11/02 19:11:45, ncarter wrote:
> Usually I would leave these as public, since they're public in the base class,
> and callable through that interface.

Done.  I thought that making them private would make them non-callable without
an extra cast (to the interface or to the base class), but I see that this is
indeed not the case... :-/  Oh, well...

[Łukasz Anforowicz, 2016-11-02 21:05:38]

The CQ bit was checked by lukasza@chromium.org

[Łukasz Anforowicz, 2016-11-02 21:05:38]

The patchset sent to the CQ was uploaded after l-g-t-m from nick@chromium.org
Link to the patchset: https://codereview.chromium.org/2453693003/#ps180001
(title: "Addressed CR feedback from nick@.")

[commit-bot: I haz the power, 2016-11-02 21:06:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-02 23:24:38]

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

[commit-bot: I haz the power, 2016-11-02 23:24:39]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Łukasz Anforowicz, 2016-11-02 23:32:16]

The CQ bit was checked by lukasza@chromium.org

[commit-bot: I haz the power, 2016-11-02 23:32:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-03 00:22:39]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-11-03 00:25:25]

Description was changed from

==========
Initial browser tests for OOPIF support for drag-n-drop.

This CL adds a single test - dragging and dropping a piece of text from
outside of the browser into a cross-site frame.  The test expectedly
fails for now in --site-per-process mode (the CL adds an exception
in site-per-process.content_browsertests.filter).


DOMMessageQueue
===============

The new test verifies whether appropriate javascript events have fired.
This is done by DOMMessageQueue, which has been tweaked slightly to
target only a specified WebContents and detect renderer crashes.

The modified DOMMessageQueue makes the old DOMOperationObserver
obsolete, so the CL deletes the old class and switches its users
to use DOMMessageQueue instead.

DomAutomationController::SendMsg was refusing to serialize and send
javascript objects.  This restriction has been lifted (any object
type can be sent, as long as it can be serialized to JSON).


Simulation of drag-and-drop
===========================

The CL introduces a DragAndDropSimulator that helps with simulating
drag-and-drop events for WebContents.  The simulator supports only
the minimum functionality required to enable the first OOPIF test:

- Currently the simulator only supports drag-enter (dragging something
  from outside the browser into WebContenst) and drop events.

- Currently the simulator only works when WebContents is built on top
  of aura UI abstraction/platform.


Test pages
==========

The CL provides a main test page (content/test/data/drag_and_drop/page.html)
which enables:

- Easy locating of subframes (which are positioned at fixed coordinates).

- Easy replacing of subframe contents, which are specified by URL query.

  - This helps with testing various drag-and-drop scenarios (e.g. using
    different subframes for testing drop with drop_target.html OR for
    testing drag-start with image_source.html).

  - This also helps exercise various OOPIF scenarios (e.g. using
    subframes that are same-site or cross-site).


BUG=647249
==========

to

==========
Initial browser tests for OOPIF support for drag-n-drop.

This CL adds a single test - dragging and dropping a piece of text from
outside of the browser into a cross-site frame.  The test expectedly
fails for now in --site-per-process mode (the CL adds an exception
in site-per-process.content_browsertests.filter).

DOMMessageQueue
===============

The new test verifies whether appropriate javascript events have fired.
This is done by DOMMessageQueue, which has been tweaked slightly to
target only a specified WebContents and detect renderer crashes.

The modified DOMMessageQueue makes the old DOMOperationObserver
obsolete, so the CL deletes the old class and switches its users
to use DOMMessageQueue instead.

DomAutomationController::SendMsg was refusing to serialize and send
javascript objects.  This restriction has been lifted (any object
type can be sent, as long as it can be serialized to JSON).

Simulation of drag-and-drop
===========================

The CL introduces a DragAndDropSimulator that helps with simulating
drag-and-drop events for WebContents.  The simulator supports only
the minimum functionality required to enable the first OOPIF test:

- Currently the simulator only supports drag-enter (dragging something
  from outside the browser into WebContenst) and drop events.

- Currently the simulator only works when WebContents is built on top
  of aura UI abstraction/platform.

Test pages
==========

The CL provides a main test page (content/test/data/drag_and_drop/page.html)
which enables:

- Easy locating of subframes (which are positioned at fixed coordinates).

- Easy replacing of subframe contents, which are specified by URL query.

  - This helps with testing various drag-and-drop scenarios (e.g. using
    different subframes for testing drop with drop_target.html OR for
    testing drag-start with image_source.html).

  - This also helps exercise various OOPIF scenarios (e.g. using
    subframes that are same-site or cross-site).

BUG=647249

Committed: https://crrev.com/f1cf6923ce29e2995e7deefa654d299fd5512d28
Cr-Commit-Position: refs/heads/master@{#429472}
==========

[commit-bot: I haz the power, 2016-11-03 00:25:26]

Patchset 10 (id:??) landed as
https://crrev.com/f1cf6923ce29e2995e7deefa654d299fd5512d28
Cr-Commit-Position: refs/heads/master@{#429472}