Revert of [BlobAsync] Fixed race between IPC messages and IO task queue

Revert of [BlobAsync] Fixed race between IPC messages and IO task queue (patchset #1 id:1 of https://codereview.chromium.org/1988293002/ )

Reason for revert:
The oortonline benchmark started failing to load blobs around the time this landed, so I'll do a speculative revert. Failure log:

https://build.chromium.org/p/chromium.perf/builders/Mac%20Retina%20Perf%20%285%29/builds/3008/steps/oortonline/logs/stdio/text

Original issue's description:
> [BlobAsync] Fixed race between IPC messages and IO task queue
>
> The message responses to our blob start message are dispatched on the IO
> thread but they're not added to the task queue. So a race can happen
> where OnMemoryRequest is called before we store the consolidation in our
> map.
>
> I moved the StartBuildingBlob message - which signals the browser to
> start requesting memory from the renderer - to our IO task which stores
> our blob data. This eliminates the race case with a small sacrifice of
> construction speed.
>
> In the future, I'd like to be able to still dispatch both IPC messages
> right away to keep the fast path, and correctly handle the race case.
>
> BUG=612358
>
> Committed: https://crrev.com/65a8fb95a8c71a7be8515f5e1c8125b04a477716
> Cr-Commit-Position: refs/heads/master@{#394549}

TBR=michaeln@chromium.org,dmurph@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=612358
Committed: https://crrev.com/723cf1bf74560eb41120c1bf94eaaa38115203d0
Cr-Commit-Position: refs/heads/master@{#394740}

[Sami, 2016-05-19 11:40:47]

Created Revert of [BlobAsync] Fixed race between IPC messages and IO task queue

[commit-bot: I haz the power, 2016-05-19 11:41:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1997623002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1997623002/1

[commit-bot: I haz the power, 2016-05-19 11:41:40]

Description was changed from

==========
Revert of [BlobAsync] Fixed race between IPC messages and IO task queue
(patchset #1 id:1 of https://codereview.chromium.org/1988293002/ )

Reason for revert:
The oortonline benchmark started failing to load blobs around the time this
landed, so I'll do a speculative revert. Failure log:

https://build.chromium.org/p/chromium.perf/builders/Mac%20Retina%20Perf%20%28...

Original issue's description:
> [BlobAsync] Fixed race between IPC messages and IO task queue
> 
> The message responses to our blob start message are dispatched on the IO
> thread but they're not added to the task queue. So a race can happen
> where OnMemoryRequest is called before we store the consolidation in our
> map.
> 
> I moved the StartBuildingBlob message - which signals the browser to
> start requesting memory from the renderer - to our IO task which stores
> our blob data. This eliminates the race case with a small sacrifice of
> construction speed.
> 
> In the future, I'd like to be able to still dispatch both IPC messages
> right away to keep the fast path, and correctly handle the race case.
> 
> BUG=612358
> 
> Committed: https://crrev.com/65a8fb95a8c71a7be8515f5e1c8125b04a477716
> Cr-Commit-Position: refs/heads/master@{#394549}

TBR=michaeln@chromium.org,dmurph@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=612358
==========

to

==========
Revert of [BlobAsync] Fixed race between IPC messages and IO task queue
(patchset #1 id:1 of https://codereview.chromium.org/1988293002/ )

Reason for revert:
The oortonline benchmark started failing to load blobs around the time this
landed, so I'll do a speculative revert. Failure log:

https://build.chromium.org/p/chromium.perf/builders/Mac%20Retina%20Perf%20%28...

Original issue's description:
> [BlobAsync] Fixed race between IPC messages and IO task queue
> 
> The message responses to our blob start message are dispatched on the IO
> thread but they're not added to the task queue. So a race can happen
> where OnMemoryRequest is called before we store the consolidation in our
> map.
> 
> I moved the StartBuildingBlob message - which signals the browser to
> start requesting memory from the renderer - to our IO task which stores
> our blob data. This eliminates the race case with a small sacrifice of
> construction speed.
> 
> In the future, I'd like to be able to still dispatch both IPC messages
> right away to keep the fast path, and correctly handle the race case.
> 
> BUG=612358
> 
> Committed: https://crrev.com/65a8fb95a8c71a7be8515f5e1c8125b04a477716
> Cr-Commit-Position: refs/heads/master@{#394549}

TBR=michaeln@chromium.org,dmurph@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=612358
==========

[commit-bot: I haz the power, 2016-05-19 11:41:41]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-05-19 11:43:19]

Description was changed from

==========
Revert of [BlobAsync] Fixed race between IPC messages and IO task queue
(patchset #1 id:1 of https://codereview.chromium.org/1988293002/ )

Reason for revert:
The oortonline benchmark started failing to load blobs around the time this
landed, so I'll do a speculative revert. Failure log:

https://build.chromium.org/p/chromium.perf/builders/Mac%20Retina%20Perf%20%28...

Original issue's description:
> [BlobAsync] Fixed race between IPC messages and IO task queue
> 
> The message responses to our blob start message are dispatched on the IO
> thread but they're not added to the task queue. So a race can happen
> where OnMemoryRequest is called before we store the consolidation in our
> map.
> 
> I moved the StartBuildingBlob message - which signals the browser to
> start requesting memory from the renderer - to our IO task which stores
> our blob data. This eliminates the race case with a small sacrifice of
> construction speed.
> 
> In the future, I'd like to be able to still dispatch both IPC messages
> right away to keep the fast path, and correctly handle the race case.
> 
> BUG=612358
> 
> Committed: https://crrev.com/65a8fb95a8c71a7be8515f5e1c8125b04a477716
> Cr-Commit-Position: refs/heads/master@{#394549}

TBR=michaeln@chromium.org,dmurph@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=612358
==========

to

==========
Revert of [BlobAsync] Fixed race between IPC messages and IO task queue
(patchset #1 id:1 of https://codereview.chromium.org/1988293002/ )

Reason for revert:
The oortonline benchmark started failing to load blobs around the time this
landed, so I'll do a speculative revert. Failure log:

https://build.chromium.org/p/chromium.perf/builders/Mac%20Retina%20Perf%20%28...

Original issue's description:
> [BlobAsync] Fixed race between IPC messages and IO task queue
>
> The message responses to our blob start message are dispatched on the IO
> thread but they're not added to the task queue. So a race can happen
> where OnMemoryRequest is called before we store the consolidation in our
> map.
>
> I moved the StartBuildingBlob message - which signals the browser to
> start requesting memory from the renderer - to our IO task which stores
> our blob data. This eliminates the race case with a small sacrifice of
> construction speed.
>
> In the future, I'd like to be able to still dispatch both IPC messages
> right away to keep the fast path, and correctly handle the race case.
>
> BUG=612358
>
> Committed: https://crrev.com/65a8fb95a8c71a7be8515f5e1c8125b04a477716
> Cr-Commit-Position: refs/heads/master@{#394549}

TBR=michaeln@chromium.org,dmurph@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=612358

Committed: https://crrev.com/723cf1bf74560eb41120c1bf94eaaa38115203d0
Cr-Commit-Position: refs/heads/master@{#394740}
==========

[commit-bot: I haz the power, 2016-05-19 11:43:20]

Patchset 1 (id:??) landed as
https://crrev.com/723cf1bf74560eb41120c1bf94eaaa38115203d0
Cr-Commit-Position: refs/heads/master@{#394740}

[Sami, 2016-05-20 11:12:02]

To give an update I think this worked -- here's the green build after the revert
landed:

https://build.chromium.org/p/chromium.perf/builders/Mac%20Retina%20Perf%20%28...

For some reason the blame list shows the revert in the previous build (which was
still red):

https://build.chromium.org/p/chromium.perf/builders/Mac%20Retina%20Perf%20%28...

but if the revision (54e8895f6205ab1486db3a15e66b8832f12aa541) is accurate then
the revert actually wasn't in that build yet.

[dmurph, 2016-05-23 18:03:11]

On 2016/05/20 at 11:12:02, skyostil wrote:
> To give an update I think this worked -- here's the green build after the
revert landed:
> 
>
https://build.chromium.org/p/chromium.perf/builders/Mac%20Retina%20Perf%20%28...
> 
> For some reason the blame list shows the revert in the previous build (which
was still red):
> 
>
https://build.chromium.org/p/chromium.perf/builders/Mac%20Retina%20Perf%20%28...
> 
> but if the revision (54e8895f6205ab1486db3a15e66b8832f12aa541) is accurate
then the revert actually wasn't in that build yet.

Yeah there was some flakiness in a past design with that benchmark, so I'm not
surprised that it started failing again. I don't know exactly why it was failing
though.