New issue
Advanced search Search tips

Issue 668598 link

Starred by 2 users
Status: Fixed
Owner:
hirosh...@chromium.org
Closed: Nov 2016
Cc:
yhirano@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 667641



Sign in to add a comment

Do not notify ResourceClient/ImageResourceObserver of finish twice for multipart image

Project Member Reported by hirosh...@chromium.org, Nov 25 2016 
Currently, clients/observers can be notified of finish twice, when:
Case 1. The first part is loaded, AND
Case 2. When loading is really finished (i.e. finish() or error() is called).

This issue aims to drop the second call (Case 2) to
- Make the finish notifications to be always called at most once, and
- Simplify the logic for preparation for Issue 667641.
  (Issue 667641 can be done without this refactoring, but with more additional logic.)
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 29 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0292701c1facba4d2162eaff216ba10065ca482f

commit 0292701c1facba4d2162eaff216ba10065ca482f
Author: hiroshige <hiroshige@chromium.org>
Date: Tue Nov 29 10:26:19 2016

Do not rely on broken images in http/tests/multipart/ tests

Preparation for https://codereview.chromium.org/2513413008.

This CL introduces Internals::isLoading() and make the tests to use
isLoading() instead of size checks for detecting broken images.

BUG= 668598 

Review-Url: https://codereview.chromium.org/1840933002
Cr-Commit-Position: refs/heads/master@{#434947}

[delete] https://crrev.com/27891919ba0f1465b7e7eee68196eb51b1df8e01/third_party/WebKit/LayoutTests/http/tests/multipart/stop-loading-after-onload1-expected.txt
[modify] https://crrev.com/0292701c1facba4d2162eaff216ba10065ca482f/third_party/WebKit/LayoutTests/http/tests/multipart/stop-loading-after-onload1.html
[delete] https://crrev.com/27891919ba0f1465b7e7eee68196eb51b1df8e01/third_party/WebKit/LayoutTests/http/tests/multipart/stop-loading-after-onload2-expected.txt
[modify] https://crrev.com/0292701c1facba4d2162eaff216ba10065ca482f/third_party/WebKit/LayoutTests/http/tests/multipart/stop-loading-after-onload2.html
[delete] https://crrev.com/27891919ba0f1465b7e7eee68196eb51b1df8e01/third_party/WebKit/LayoutTests/http/tests/multipart/stop-loading-expected.txt
[modify] https://crrev.com/0292701c1facba4d2162eaff216ba10065ca482f/third_party/WebKit/LayoutTests/http/tests/multipart/stop-loading.html
[modify] https://crrev.com/0292701c1facba4d2162eaff216ba10065ca482f/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/0292701c1facba4d2162eaff216ba10065ca482f/third_party/WebKit/Source/core/testing/Internals.h
[modify] https://crrev.com/0292701c1facba4d2162eaff216ba10065ca482f/third_party/WebKit/Source/core/testing/Internals.idl
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 29 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/18a8d7074c9f0c8e2c3fff36b3a021cdd3a06477

commit 18a8d7074c9f0c8e2c3fff36b3a021cdd3a06477
Author: hiroshige <hiroshige@chromium.org>
Date: Tue Nov 29 10:39:07 2016

Do not notify ResourceClient/ImageResourceObserver of finish twice

Previously, for a multipart image,
- ResourceClient::notifyFinished() and
- ImageResourceObserver::imageNotifyFinished()
might be called twice:
1. When the first part is loaded, and
2. When the whole loading is finished.

This CL removes the second case to make callback/observer control consistent
and simpler, as preparation for https://codereview.chromium.org/2469873002/.

Behavior change:
Previously, multipart images are replaced with broken images when cancelled
after the first part is loaded.
After this CL, multipart images remains as-is in such cases.
https://codereview.chromium.org/1840933002 fixes layout tests for this change.

BUG= 668598 

Review-Url: https://codereview.chromium.org/2513413008
Cr-Commit-Position: refs/heads/master@{#434952}

[modify] https://crrev.com/18a8d7074c9f0c8e2c3fff36b3a021cdd3a06477/third_party/WebKit/Source/core/fetch/ImageResource.cpp
[modify] https://crrev.com/18a8d7074c9f0c8e2c3fff36b3a021cdd3a06477/third_party/WebKit/Source/core/fetch/ImageResource.h
[modify] https://crrev.com/18a8d7074c9f0c8e2c3fff36b3a021cdd3a06477/third_party/WebKit/Source/core/fetch/Resource.cpp
[modify] https://crrev.com/18a8d7074c9f0c8e2c3fff36b3a021cdd3a06477/third_party/WebKit/Source/core/fetch/Resource.h

Comment 3 by hirosh...@chromium.org, Nov 29 2016

Status: Fixed (was: Started)
►

Sign in to add a comment