[ImageResource 0b] Decouple ImageResourceObserver::imageNotifyFinished() from notifyFinished()

Decouple ImageResourceObserver::imageNotifyFinished() from notifyFinished()

Previously, imageNotifyFinished() is coupled with ResourceClient::notifyFinished().
In order to decouple ImageResorceObserver from resource loading and make it
purely image-related observer, this CL make imageNotifyFinished() to be
called after ImageResourceObserver::imageChanged(), not notifyFinished().

BUG=667641
Committed: https://crrev.com/d8fa1cd54a5df1a17fb9359a5b9cac00293517aa
Cr-Commit-Position: refs/heads/master@{#435235}

[hiroshige, 2016-11-01 17:17:17]

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

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

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

[hiroshige, 2016-11-01 17:20:20]

Description was changed from

==========
Decouple ImageResourceObserver::imageNotifyFinished() from resource loading

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

to

==========
Decouple ImageResourceObserver::imageNotifyFinished() from resource loading

This CL conceptually reverts https://codereview.chromium.org/1843533005/.

Previously, imageNotifyFinished() is coupled with
ResourceClient::notifyFinished().
In order to decouple ImageResorceObserver from resource loading and make it
purely image-related observer, this CL make imageNotifyFinished() to be called
after ImageResourceObserver::imageChanged(), not notifyFinished().

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

[commit-bot: I haz the power, 2016-11-01 18:40:01]

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

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

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[hiroshige, 2016-11-09 07:11:09]

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

[commit-bot: I haz the power, 2016-11-09 07:11:22]

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-09 10:40:06]

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

[commit-bot: I haz the power, 2016-11-09 10:40:07]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-11-09 11:36:18]

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

[commit-bot: I haz the power, 2016-11-09 11:36:34]

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-09 13:26:30]

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

[commit-bot: I haz the power, 2016-11-09 13:26:31]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-11-22 06:53:30]

Description was changed from

==========
Decouple ImageResourceObserver::imageNotifyFinished() from resource loading

This CL conceptually reverts https://codereview.chromium.org/1843533005/.

Previously, imageNotifyFinished() is coupled with
ResourceClient::notifyFinished().
In order to decouple ImageResorceObserver from resource loading and make it
purely image-related observer, this CL make imageNotifyFinished() to be called
after ImageResourceObserver::imageChanged(), not notifyFinished().

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

to

==========
Decouple ImageResourceObserver::imageNotifyFinished() from resource loading

This CL conceptually reverts https://codereview.chromium.org/1843533005/.

Previously, imageNotifyFinished() is coupled with
ResourceClient::notifyFinished().
In order to decouple ImageResorceObserver from resource loading and make it
purely image-related observer, this CL make imageNotifyFinished() to be called
after ImageResourceObserver::imageChanged(), not notifyFinished().

BUG=667641
==========

[hiroshige, 2016-11-22 07:57:47]

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

[commit-bot: I haz the power, 2016-11-22 07:57:57]

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-22 09:29:53]

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

[commit-bot: I haz the power, 2016-11-22 09:29:54]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-11-22 10:27:55]

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

[commit-bot: I haz the power, 2016-11-22 10:28:10]

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

[hiroshige, 2016-11-22 11:52:35]

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

[commit-bot: I haz the power, 2016-11-22 11:52:47]

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

[hiroshige, 2016-11-22 11:58:58]

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

[commit-bot: I haz the power, 2016-11-22 11:59:07]

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-22 14:58:14]

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

[commit-bot: I haz the power, 2016-11-22 14:58:15]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-11-22 18:46:16]

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

[commit-bot: I haz the power, 2016-11-22 18:46:41]

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-22 19:13:15]

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

[commit-bot: I haz the power, 2016-11-22 19:13:16]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[hiroshige, 2016-11-25 04:58:02]

Description was changed from

==========
Decouple ImageResourceObserver::imageNotifyFinished() from resource loading

This CL conceptually reverts https://codereview.chromium.org/1843533005/.

Previously, imageNotifyFinished() is coupled with
ResourceClient::notifyFinished().
In order to decouple ImageResorceObserver from resource loading and make it
purely image-related observer, this CL make imageNotifyFinished() to be called
after ImageResourceObserver::imageChanged(), not notifyFinished().

BUG=667641
==========

to

==========
Decouple ImageResourceObserver::imageNotifyFinished() from notifyFinished()

This CL conceptually reverts https://codereview.chromium.org/1843533005/.

Previously, imageNotifyFinished() is coupled with
ResourceClient::notifyFinished().
In order to decouple ImageResorceObserver from resource loading and make it
purely image-related observer, this CL make imageNotifyFinished() to be
called after ImageResourceObserver::imageChanged(), not notifyFinished().

BUG=667641
==========

[hiroshige, 2016-11-25 04:58:02]

hiroshige@chromium.org changed reviewers:
+ yhirano@chromium.org

[hiroshige, 2016-11-25 04:58:06]

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

[commit-bot: I haz the power, 2016-11-25 04:58:15]

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-25 06:43:34]

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

[commit-bot: I haz the power, 2016-11-25 06:43:35]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-11-25 10:09:43]

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

[commit-bot: I haz the power, 2016-11-25 10:09:57]

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-25 11:27:35]

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

[commit-bot: I haz the power, 2016-11-25 11:27:36]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-11-29 07:08:28]

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

[commit-bot: I haz the power, 2016-11-29 07:08:53]

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-29 08:32:15]

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

[commit-bot: I haz the power, 2016-11-29 08:32:16]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-11-29 09:15:34]

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

[commit-bot: I haz the power, 2016-11-29 09:15:51]

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-29 10:34:05]

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

[commit-bot: I haz the power, 2016-11-29 10:34:07]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-11-29 10:47:07]

Description was changed from

==========
Decouple ImageResourceObserver::imageNotifyFinished() from notifyFinished()

This CL conceptually reverts https://codereview.chromium.org/1843533005/.

Previously, imageNotifyFinished() is coupled with
ResourceClient::notifyFinished().
In order to decouple ImageResorceObserver from resource loading and make it
purely image-related observer, this CL make imageNotifyFinished() to be
called after ImageResourceObserver::imageChanged(), not notifyFinished().

BUG=667641
==========

to

==========
Decouple ImageResourceObserver::imageNotifyFinished() from notifyFinished()

Previously, imageNotifyFinished() is coupled with
ResourceClient::notifyFinished().
In order to decouple ImageResorceObserver from resource loading and make it
purely image-related observer, this CL make imageNotifyFinished() to be
called after ImageResourceObserver::imageChanged(), not notifyFinished().

BUG=667641
==========

[hiroshige, 2016-11-29 10:47:18]

PTAL.

[kouhei (in TOK), 2016-11-30 03:48:21]

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

[kouhei (in TOK), 2016-11-30 03:48:22]

https://codereview.chromium.org/2468883003/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ImageResource.h (right):

https://codereview.chromium.org/2468883003/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ImageResource.h:194: void
notifyObservers(bool isNotifyingFinish,
Please change bool -> enum

[hiroshige, 2016-11-30 05:19:41]

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

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

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

[hiroshige, 2016-11-30 05:21:40]

https://codereview.chromium.org/2468883003/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ImageResource.h (right):

https://codereview.chromium.org/2468883003/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ImageResource.h:194: void
notifyObservers(bool isNotifyingFinish,
On 2016/11/30 03:48:22, kouhei wrote:
> Please change bool -> enum

Done.

[kouhei (in TOK), 2016-11-30 05:31:24]

lgtm

[yhirano, 2016-11-30 05:42:57]

Now imageNotifyFinished is preceded by imageChanged. How about merging two
notifications by adding a parameter?

[hiroshige, 2016-11-30 05:49:28]

On 2016/11/30 05:42:57, yhirano wrote:
> Now imageNotifyFinished is preceded by imageChanged. How about merging two
> notifications by adding a parameter?

I think it's a good thing to do.
However, I'd like to postpone it to after I split ImageResource due to the long
chain of rebasing.

[hiroshige, 2016-11-30 05:55:48]

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

[commit-bot: I haz the power, 2016-11-30 05:56:37]

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

[yhirano, 2016-11-30 06:53:15]

On 2016/11/30 05:49:28, hiroshige wrote:
> On 2016/11/30 05:42:57, yhirano wrote:
> > Now imageNotifyFinished is preceded by imageChanged. How about merging two
> > notifications by adding a parameter?
> 
> I think it's a good thing to do.
> However, I'd like to postpone it to after I split ImageResource due to the
long
> chain of rebasing.

Hmm, OK, then please put a TODO comment on ImageResourceObserver.h.

[yhirano, 2016-11-30 07:00:32]

https://codereview.chromium.org/2468883003/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ImageResource.cpp (right):

https://codereview.chromium.org/2468883003/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ImageResource.cpp:696: // We notify
clients/observers of finish here, and they will not be
This comment should be fixed.

[hiroshige, 2016-11-30 07:00:34]

On 2016/11/30 06:53:15, yhirano wrote:
> On 2016/11/30 05:49:28, hiroshige wrote:
> > On 2016/11/30 05:42:57, yhirano wrote:
> > > Now imageNotifyFinished is preceded by imageChanged. How about merging two
> > > notifications by adding a parameter?
> > 
> > I think it's a good thing to do.
> > However, I'd like to postpone it to after I split ImageResource due to the
> long
> > chain of rebasing.
> 
> Hmm, OK, then please put a TODO comment on ImageResourceObserver.h.

Done.

[hiroshige, 2016-11-30 07:04:24]

https://codereview.chromium.org/2468883003/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ImageResource.cpp (right):

https://codereview.chromium.org/2468883003/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ImageResource.cpp:696: // We notify
clients/observers of finish here, and they will not be
On 2016/11/30 07:00:32, yhirano wrote:
> This comment should be fixed.

Done.

[yhirano, 2016-11-30 07:06:05]

lgtm

[hiroshige, 2016-11-30 11:18:44]

The CQ bit was checked by hiroshige@chromium.org

[hiroshige, 2016-11-30 11:18:44]

The patchset sent to the CQ was uploaded after l-g-t-m from kouhei@chromium.org
Link to the patchset: https://codereview.chromium.org/2468883003/#ps300001
(title: "comment")

[commit-bot: I haz the power, 2016-11-30 11:18:59]

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-30 12:58:13]

CQ is committing da patch.

Bot data: {"patchset_id": 300001, "attempt_start_ts": 1480504724569210,
"parent_rev": "facb9b28b958c1a353cf1968049c21f13d83ef30", "commit_rev":
"e156c6ee0cd0186246a3ecfb956939a167d5c7b1"}

[commit-bot: I haz the power, 2016-11-30 12:58:35]

Description was changed from

==========
Decouple ImageResourceObserver::imageNotifyFinished() from notifyFinished()

Previously, imageNotifyFinished() is coupled with
ResourceClient::notifyFinished().
In order to decouple ImageResorceObserver from resource loading and make it
purely image-related observer, this CL make imageNotifyFinished() to be
called after ImageResourceObserver::imageChanged(), not notifyFinished().

BUG=667641
==========

to

==========
Decouple ImageResourceObserver::imageNotifyFinished() from notifyFinished()

Previously, imageNotifyFinished() is coupled with
ResourceClient::notifyFinished().
In order to decouple ImageResorceObserver from resource loading and make it
purely image-related observer, this CL make imageNotifyFinished() to be
called after ImageResourceObserver::imageChanged(), not notifyFinished().

BUG=667641
==========

[commit-bot: I haz the power, 2016-11-30 12:58:37]

Committed patchset #16 (id:300001)

[commit-bot: I haz the power, 2016-11-30 13:00:59]

Description was changed from

==========
Decouple ImageResourceObserver::imageNotifyFinished() from notifyFinished()

Previously, imageNotifyFinished() is coupled with
ResourceClient::notifyFinished().
In order to decouple ImageResorceObserver from resource loading and make it
purely image-related observer, this CL make imageNotifyFinished() to be
called after ImageResourceObserver::imageChanged(), not notifyFinished().

BUG=667641
==========

to

==========
Decouple ImageResourceObserver::imageNotifyFinished() from notifyFinished()

Previously, imageNotifyFinished() is coupled with
ResourceClient::notifyFinished().
In order to decouple ImageResorceObserver from resource loading and make it
purely image-related observer, this CL make imageNotifyFinished() to be
called after ImageResourceObserver::imageChanged(), not notifyFinished().

BUG=667641

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

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

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