Bug fix: Fix wrong assumption in ImageResource when adding observer

Bug fix: Fix wrong assumption in ImageResource when adding observer

When the response is not multipart, if |m_data| exists, |m_image| must
be created. This is assured that |updateImage()| is called when
|appendData()| is called.

On the other hand, when the response is multipart, |updateImage()| is
not called in |appendData()|, which means |m_image| might not be created
even when |m_data| exists. This is intentional since creating a |m_image|
on receiving data might destroy an existing image in a previous part.

This CL fixes the wrong DCHECK in addObserver and didAddClient in
ImageResource.

BUG=630983
TEST=webkit_unit_tests --gtest_filter=ImageResourceTest.MultipartImage

Committed: https://crrev.com/d1e1dd413401a3434537e12f1f12637e7c8bf2fb
Cr-Commit-Position: refs/heads/master@{#408092}

[hajimehoshi, 2016-07-26 07:52:15]

hajimehoshi@chromium.org changed reviewers:
+ hiroshige@chromium.org, mkwst@chromium.org

[hajimehoshi, 2016-07-26 07:52:15]

PTAL

[hajimehoshi, 2016-07-26 07:52:54]

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

[commit-bot: I haz the power, 2016-07-26 07:53:10]

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-07-26 08:59:17]

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

[commit-bot: I haz the power, 2016-07-26 08:59:17]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[hiroshige, 2016-07-27 08:13:40]

https://codereview.chromium.org/2182023002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/fetch/ImageResourceTest.cpp (right):

https://codereview.chromium.org/2182023002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/ImageResourceTest.cpp:254:
ASSERT_TRUE(client->notifyFinishedCalled());
Could you confirm that |client2| is also notified, i.e.
> ASSERT_EQ(client2->imageChangedCount(), 1);
> ASSERT_TRUE(client2->notifyFinishedCalled());
?

[hajimehoshi, 2016-07-27 08:26:30]

Thank you!

https://codereview.chromium.org/2182023002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/fetch/ImageResourceTest.cpp (right):

https://codereview.chromium.org/2182023002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/ImageResourceTest.cpp:254:
ASSERT_TRUE(client->notifyFinishedCalled());
On 2016/07/27 08:13:40, hiroshige wrote:
> Could you confirm that |client2| is also notified, i.e.
> > ASSERT_EQ(client2->imageChangedCount(), 1);
> > ASSERT_TRUE(client2->notifyFinishedCalled());
> ?

Done.

[hajimehoshi, 2016-07-27 08:27:23]

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

[commit-bot: I haz the power, 2016-07-27 08:27:42]

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

[Mike West, 2016-07-27 08:37:54]

LGTM

[hajimehoshi, 2016-07-27 08:40:39]

The CQ bit was unchecked by hajimehoshi@chromium.org

[hajimehoshi, 2016-07-27 08:40:39]

The CQ bit was checked by hajimehoshi@chromium.org

[commit-bot: I haz the power, 2016-07-27 08:40:44]

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-07-27 11:00:04]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-27 11:01:52]

Description was changed from

==========
Bug fix: Fix wrong assumption in ImageResource when adding observer

When the response is not multipart, if |m_data| exists, |m_image| must
be created. This is assured that |updateImage()| is called when
|appendData()| is called.

On the other hand, when the response is multipart, |updateImage()| is
not called in |appendData()|, which means |m_image| might not be created
even when |m_data| exists. This is intentional since creating a |m_image|
on receiving data might destroy an existing image in a previous part.

This CL fixes the wrong DCHECK in addObserver and didAddClient in
ImageResource.

BUG=630983
TEST=webkit_unit_tests --gtest_filter=ImageResourceTest.MultipartImage
==========

to

==========
Bug fix: Fix wrong assumption in ImageResource when adding observer

When the response is not multipart, if |m_data| exists, |m_image| must
be created. This is assured that |updateImage()| is called when
|appendData()| is called.

On the other hand, when the response is multipart, |updateImage()| is
not called in |appendData()|, which means |m_image| might not be created
even when |m_data| exists. This is intentional since creating a |m_image|
on receiving data might destroy an existing image in a previous part.

This CL fixes the wrong DCHECK in addObserver and didAddClient in
ImageResource.

BUG=630983
TEST=webkit_unit_tests --gtest_filter=ImageResourceTest.MultipartImage

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

[commit-bot: I haz the power, 2016-07-27 11:01:53]

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

[hiroshige, 2016-07-27 11:14:34]

lgtm