Make ImageLoader have at most one pending error event on EventSender

Make ImageLoader have at most one pending error event on EventSender

Relanding the CHECK() that asserts
ImageLoader::DispatchPendingErrorEvent() should always called when
has_pending_error_event_ is true.

Previously, we sometimes schedule multiple error events on EventSender:
1. DispatchErrorEvent() is called.
2. DispatchErrorEvent() is called.
3. DispatchPendingErrorEvent() is called (corresponding to Step 1)
   and actual processing is done.
4. DispatchPendingErrorEvent() is called (corresponding to Step 2)
   but ignored because has_pending_error_event_ is already false.

This CL makes DispatchErrorEvent() cancel previous scheduled error
events if any, ensures there can be at most scheduled error event on
EventSender, and makes the assertion hold.

The scenario above will be processed as follows, and thus this CL
shouldn't change any observable behavior.
1. DispatchErrorEvent() is called.
2. DispatchErrorEvent() is called.
   The error event scheduled by Step 1 is cancelled.
3. DispatchPendingErrorEvent() is called (corresponding to Step 2)
   and actual processing is done.

[1] https://codereview.chromium.org/2859383002/
[2] https://codereview.chromium.org/2884773002/

BUG=624697, 719759, 722500
Review-Url: https://codereview.chromium.org/2893993002
Cr-Commit-Position: refs/heads/master@{#473214}
Committed: https://chromium.googlesource.com/chromium/src/+/84fb9e0a3e168b81e538a5ff8ce79e3690396a10

[hiroshige, 2017-05-18 19:16:23]

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

[commit-bot: I haz the power, 2017-05-18 19:17:52]

Dry run: CQ is trying da patch.

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

[hiroshige, 2017-05-18 19:20:17]

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

[commit-bot: I haz the power, 2017-05-18 19:21:06]

Dry run: CQ is trying da patch.

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

[hiroshige, 2017-05-18 19:27:18]

Description was changed from

==========
Make ImageLoader have at most one pending error event on EventSender

BUG=
==========

to

==========
Make ImageLoader have at most one pending error event on EventSender

Relanding the CHECK() that asserts
ImageLoader::DispatchPendingErrorEvent() should always called when
has_pending_error_event_ is true.

Previously, we sometimes schedule multiple error events on EventSender:
1. DispatchErrorEvent() is called.
2. DispatchErrorEvent() is called.
3. DispatchPendingErrorEvent() is called (corresponding to Step 1)
   and actual processing is done.
4. DispatchPendingErrorEvent() is called (corresponding to Step 2)
   but ignored because has_pending_error_event_ is already false.

This CL makes DispatchErrorEvent() cancel previous scheduled error
events if any, ensures there can be at most scheduled error event on
EventSender, and makes the assertion hold.

The scenario above will be processed as follows, and thus this CL
shouldn't change any observable behavior.
1. DispatchErrorEvent() is called.
2. DispatchErrorEvent() is called.
   The event scheduled by Step 1 is cancelled.
3. DispatchPendingErrorEvent() is called (corresponding to Step 2)
   and actual processing is done.

[1] https://codereview.chromium.org/2859383002/
[2] https://codereview.chromium.org/2884773002/

BUG=624697, 719759, 722500
==========

[hiroshige, 2017-05-18 19:27:18]

hiroshige@chromium.org changed reviewers:
+ japhet@chromium.org, kinuko@chromium.org, kouhei@chromium.org

[hiroshige, 2017-05-18 19:27:41]

Description was changed from

==========
Make ImageLoader have at most one pending error event on EventSender

Relanding the CHECK() that asserts
ImageLoader::DispatchPendingErrorEvent() should always called when
has_pending_error_event_ is true.

Previously, we sometimes schedule multiple error events on EventSender:
1. DispatchErrorEvent() is called.
2. DispatchErrorEvent() is called.
3. DispatchPendingErrorEvent() is called (corresponding to Step 1)
   and actual processing is done.
4. DispatchPendingErrorEvent() is called (corresponding to Step 2)
   but ignored because has_pending_error_event_ is already false.

This CL makes DispatchErrorEvent() cancel previous scheduled error
events if any, ensures there can be at most scheduled error event on
EventSender, and makes the assertion hold.

The scenario above will be processed as follows, and thus this CL
shouldn't change any observable behavior.
1. DispatchErrorEvent() is called.
2. DispatchErrorEvent() is called.
   The event scheduled by Step 1 is cancelled.
3. DispatchPendingErrorEvent() is called (corresponding to Step 2)
   and actual processing is done.

[1] https://codereview.chromium.org/2859383002/
[2] https://codereview.chromium.org/2884773002/

BUG=624697, 719759, 722500
==========

to

==========
Make ImageLoader have at most one pending error event on EventSender

Relanding the CHECK() that asserts
ImageLoader::DispatchPendingErrorEvent() should always called when
has_pending_error_event_ is true.

Previously, we sometimes schedule multiple error events on EventSender:
1. DispatchErrorEvent() is called.
2. DispatchErrorEvent() is called.
3. DispatchPendingErrorEvent() is called (corresponding to Step 1)
   and actual processing is done.
4. DispatchPendingErrorEvent() is called (corresponding to Step 2)
   but ignored because has_pending_error_event_ is already false.

This CL makes DispatchErrorEvent() cancel previous scheduled error
events if any, ensures there can be at most scheduled error event on
EventSender, and makes the assertion hold.

The scenario above will be processed as follows, and thus this CL
shouldn't change any observable behavior.
1. DispatchErrorEvent() is called.
2. DispatchErrorEvent() is called.
   The error event scheduled by Step 1 is cancelled.
3. DispatchPendingErrorEvent() is called (corresponding to Step 2)
   and actual processing is done.

[1] https://codereview.chromium.org/2859383002/
[2] https://codereview.chromium.org/2884773002/

BUG=624697, 719759, 722500
==========

[hiroshige, 2017-05-18 19:31:27]

PTAL.

I think we can make this cleaner by refactoring DoUpdateFromElement(), but I
expect it would require behavior changes (because there're other bugs around
there).
I'd like EventSender efforts and other ImageLoader bug fixes not to block each
other.

[commit-bot: I haz the power, 2017-05-18 21:35:54]

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

[commit-bot: I haz the power, 2017-05-18 21:35:56]

Dry run: This issue passed the CQ dry run.

[kouhei (in TOK), 2017-05-19 06:15:14]

lgtm

[hiroshige, 2017-05-19 16:46:34]

The CQ bit was checked by hiroshige@chromium.org

[commit-bot: I haz the power, 2017-05-19 16:47:22]

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-05-19 16:53:23]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495212394591420,
"parent_rev": "b087b7731c2efa0c446d4b52ea0839487cce4e1d", "commit_rev":
"84fb9e0a3e168b81e538a5ff8ce79e3690396a10"}

[commit-bot: I haz the power, 2017-05-19 16:53:35]

Description was changed from

==========
Make ImageLoader have at most one pending error event on EventSender

Relanding the CHECK() that asserts
ImageLoader::DispatchPendingErrorEvent() should always called when
has_pending_error_event_ is true.

Previously, we sometimes schedule multiple error events on EventSender:
1. DispatchErrorEvent() is called.
2. DispatchErrorEvent() is called.
3. DispatchPendingErrorEvent() is called (corresponding to Step 1)
   and actual processing is done.
4. DispatchPendingErrorEvent() is called (corresponding to Step 2)
   but ignored because has_pending_error_event_ is already false.

This CL makes DispatchErrorEvent() cancel previous scheduled error
events if any, ensures there can be at most scheduled error event on
EventSender, and makes the assertion hold.

The scenario above will be processed as follows, and thus this CL
shouldn't change any observable behavior.
1. DispatchErrorEvent() is called.
2. DispatchErrorEvent() is called.
   The error event scheduled by Step 1 is cancelled.
3. DispatchPendingErrorEvent() is called (corresponding to Step 2)
   and actual processing is done.

[1] https://codereview.chromium.org/2859383002/
[2] https://codereview.chromium.org/2884773002/

BUG=624697, 719759, 722500
==========

to

==========
Make ImageLoader have at most one pending error event on EventSender

Relanding the CHECK() that asserts
ImageLoader::DispatchPendingErrorEvent() should always called when
has_pending_error_event_ is true.

Previously, we sometimes schedule multiple error events on EventSender:
1. DispatchErrorEvent() is called.
2. DispatchErrorEvent() is called.
3. DispatchPendingErrorEvent() is called (corresponding to Step 1)
   and actual processing is done.
4. DispatchPendingErrorEvent() is called (corresponding to Step 2)
   but ignored because has_pending_error_event_ is already false.

This CL makes DispatchErrorEvent() cancel previous scheduled error
events if any, ensures there can be at most scheduled error event on
EventSender, and makes the assertion hold.

The scenario above will be processed as follows, and thus this CL
shouldn't change any observable behavior.
1. DispatchErrorEvent() is called.
2. DispatchErrorEvent() is called.
   The error event scheduled by Step 1 is cancelled.
3. DispatchPendingErrorEvent() is called (corresponding to Step 2)
   and actual processing is done.

[1] https://codereview.chromium.org/2859383002/
[2] https://codereview.chromium.org/2884773002/

BUG=624697, 719759, 722500

Review-Url: https://codereview.chromium.org/2893993002
Cr-Commit-Position: refs/heads/master@{#473214}
Committed:
https://chromium.googlesource.com/chromium/src/+/84fb9e0a3e168b81e538a5ff8ce7...
==========

[commit-bot: I haz the power, 2017-05-19 16:53:38]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/84fb9e0a3e168b81e538a5ff8ce7...