Fix network service hanging with empty bodies.

Fix network service hanging with empty bodies.

This fixes WebContentsBindingSetBrowserTest.CloseOnFrameDeletion with --enable-network-service.

Also add a ScopedAllowSyncCall for the host resolver sync IPC. And update the filter with a new failing test.

BUG=729848
Review-Url: https://codereview.chromium.org/2919413003
Cr-Commit-Position: refs/heads/master@{#477205}
Committed: https://chromium.googlesource.com/chromium/src/+/7de671968094071ef51a5a03bf98b72bd1d4d586

[jam, 2017-06-05 20:44:00]

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

[commit-bot: I haz the power, 2017-06-05 20:44:29]

Dry run: CQ is trying da patch.

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

[jam, 2017-06-05 20:56:17]

jam@chromium.org changed reviewers:
+ yzshen@chromium.org

[yzshen1, 2017-06-05 21:07:05]

lgtm

https://codereview.chromium.org/2919413003/diff/1/mojo/public/cpp/bindings/sy...
File mojo/public/cpp/bindings/sync_call_restrictions.h (right):

https://codereview.chromium.org/2919413003/diff/1/mojo/public/cpp/bindings/sy...
mojo/public/cpp/bindings/sync_call_restrictions.h:68: // Test-only.
Does it make sense to move line 68-69 into the "BEGIN/END ALLOWED USAGE"
section?

[mmenke, 2017-06-05 21:08:54]

Should we have a unit test for this?  Relying on some random browser test to
test this specific case seems like a bad idea to me (I'd be less concerned if
there were a browser test specifically for the empty body case), and it
shouldn't be too hard to add one.

[jam, 2017-06-05 22:50:55]

On 2017/06/05 21:08:54, mmenke wrote:
> Should we have a unit test for this?  Relying on some random browser test to
> test this specific case seems like a bad idea to me (I'd be less concerned if
> there were a browser test specifically for the empty body case), and it
> shouldn't be too hard to add one.

Good question, I had debated this and err'd on the side of not adding duplicate
tests. but your question convinced me that my judgement was wrong so I'll add a
reduced test case.

[jam, 2017-06-05 23:41:51]

The CQ bit was checked by jam@chromium.org

[jam, 2017-06-05 23:41:52]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org
Link to the patchset: https://codereview.chromium.org/2919413003/#ps20001
(title: "review comments")

[commit-bot: I haz the power, 2017-06-05 23:42:32]

CQ is trying da patch.

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

[jam, 2017-06-06 00:15:47]

https://codereview.chromium.org/2919413003/diff/1/mojo/public/cpp/bindings/sy...
File mojo/public/cpp/bindings/sync_call_restrictions.h (right):

https://codereview.chromium.org/2919413003/diff/1/mojo/public/cpp/bindings/sy...
mojo/public/cpp/bindings/sync_call_restrictions.h:68: // Test-only.
On 2017/06/05 21:07:04, yzshen1 wrote:
> Does it make sense to move line 68-69 into the "BEGIN/END ALLOWED USAGE"
> section?

Done.

[jam, 2017-06-06 00:40:58]

Description was changed from

==========
Fix network service hanging with empty bodies.

This fixes WebContentsBindingSetBrowserTest.CloseOnFrameDeletion with
--enable-network-service.

Also add a ScopedAllowSyncCall for the host resolver sync IPC. And update the
filter with a new failing test.

BUG=598073
==========

to

==========
Fix network service hanging with empty bodies.

This fixes WebContentsBindingSetBrowserTest.CloseOnFrameDeletion with
--enable-network-service.

Also add a ScopedAllowSyncCall for the host resolver sync IPC. And update the
filter with a new failing test.

BUG=729848
==========

[commit-bot: I haz the power, 2017-06-06 01:11:07]

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

[commit-bot: I haz the power, 2017-06-06 01:11:08]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[jam, 2017-06-06 03:39:01]

The CQ bit was checked by jam@chromium.org

[commit-bot: I haz the power, 2017-06-06 03:39:17]

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, 2017-06-06 05:45:57]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1496720341281800,
"parent_rev": "c438f78ca087f2b992bbf76beb25043ab0c3c0bd", "commit_rev":
"210bd7602bfc901c9465c6661af796873dbd7f64"}

[commit-bot: I haz the power, 2017-06-06 05:46:12]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1496720341281800,
"parent_rev": "865c5b53cdb67074a3c6d177dee052b46022bde6", "commit_rev":
"7de671968094071ef51a5a03bf98b72bd1d4d586"}

[commit-bot: I haz the power, 2017-06-06 05:46:24]

Description was changed from

==========
Fix network service hanging with empty bodies.

This fixes WebContentsBindingSetBrowserTest.CloseOnFrameDeletion with
--enable-network-service.

Also add a ScopedAllowSyncCall for the host resolver sync IPC. And update the
filter with a new failing test.

BUG=729848
==========

to

==========
Fix network service hanging with empty bodies.

This fixes WebContentsBindingSetBrowserTest.CloseOnFrameDeletion with
--enable-network-service.

Also add a ScopedAllowSyncCall for the host resolver sync IPC. And update the
filter with a new failing test.

BUG=729848

Review-Url: https://codereview.chromium.org/2919413003
Cr-Commit-Position: refs/heads/master@{#477205}
Committed:
https://chromium.googlesource.com/chromium/src/+/7de671968094071ef51a5a03bf98...
==========

[commit-bot: I haz the power, 2017-06-06 05:46:26]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/7de671968094071ef51a5a03bf98...