Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(93)

Side by Side Diff: third_party/WebKit/Source/core/loader/resource/ImageResource.cpp

Issue 2797993007: Show image placeholders on dimension decode error (Closed)
Patch Set: Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 /* 1 /*
2 Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de) 2 Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
3 Copyright (C) 2001 Dirk Mueller (mueller@kde.org) 3 Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
4 Copyright (C) 2002 Waldo Bastian (bastian@kde.org) 4 Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
5 Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com) 5 Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
6 Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved. 6 Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
7 7
8 This library is free software; you can redistribute it and/or 8 This library is free software; you can redistribute it and/or
9 modify it under the terms of the GNU Library General Public 9 modify it under the terms of the GNU Library General Public
10 License as published by the Free Software Foundation; either 10 License as published by the Free Software Foundation; either
(...skipping 418 matching lines...) Expand 10 before | Expand all | Expand 10 after
429 m_placeholderOption = PlaceholderOption::DoNotReloadPlaceholder; 429 m_placeholderOption = PlaceholderOption::DoNotReloadPlaceholder;
430 } else { 430 } else {
431 m_placeholderOption = PlaceholderOption::ReloadPlaceholderOnDecodeError; 431 m_placeholderOption = PlaceholderOption::ReloadPlaceholderOnDecodeError;
432 } 432 }
433 } 433 }
434 } 434 }
435 435
436 bool ImageResource::shouldShowPlaceholder() const { 436 bool ImageResource::shouldShowPlaceholder() const {
437 switch (m_placeholderOption) { 437 switch (m_placeholderOption) {
438 case PlaceholderOption::ShowAndReloadPlaceholderAlways: 438 case PlaceholderOption::ShowAndReloadPlaceholderAlways:
439 case PlaceholderOption::ShowAndDoNotReloadPlaceholder:
439 return true; 440 return true;
440 case PlaceholderOption::ReloadPlaceholderOnDecodeError: 441 case PlaceholderOption::ReloadPlaceholderOnDecodeError:
441 case PlaceholderOption::DoNotReloadPlaceholder: 442 case PlaceholderOption::DoNotReloadPlaceholder:
442 return false; 443 return false;
443 } 444 }
444 NOTREACHED(); 445 NOTREACHED();
445 return false; 446 return false;
446 } 447 }
447 448
448 bool ImageResource::shouldReloadBrokenPlaceholder() const { 449 bool ImageResource::shouldReloadBrokenPlaceholder() const {
449 switch (m_placeholderOption) { 450 switch (m_placeholderOption) {
450 case PlaceholderOption::ShowAndReloadPlaceholderAlways: 451 case PlaceholderOption::ShowAndReloadPlaceholderAlways:
451 return errorOccurred(); 452 return errorOccurred();
452 case PlaceholderOption::ReloadPlaceholderOnDecodeError: 453 case PlaceholderOption::ReloadPlaceholderOnDecodeError:
453 return getStatus() == ResourceStatus::DecodeError; 454 return getStatus() == ResourceStatus::DecodeError;
455 case PlaceholderOption::ShowAndDoNotReloadPlaceholder:
454 case PlaceholderOption::DoNotReloadPlaceholder: 456 case PlaceholderOption::DoNotReloadPlaceholder:
455 return false; 457 return false;
456 } 458 }
457 NOTREACHED(); 459 NOTREACHED();
458 return false; 460 return false;
459 } 461 }
460 462
461 static bool isLoFiImage(const ImageResource& resource) { 463 static bool isLoFiImage(const ImageResource& resource) {
462 if (!(resource.resourceRequest().previewsState() & 464 if (!(resource.resourceRequest().previewsState() &
463 WebURLRequest::ServerLoFiOn)) { 465 WebURLRequest::ServerLoFiOn)) {
(...skipping 17 matching lines...) Expand all
481 483
482 // Prevent clients and observers from being notified of completion while the 484 // Prevent clients and observers from being notified of completion while the
483 // reload is being scheduled, so that e.g. canceling an existing load in 485 // reload is being scheduled, so that e.g. canceling an existing load in
484 // progress doesn't cause clients and observers to be notified of completion 486 // progress doesn't cause clients and observers to be notified of completion
485 // prematurely. 487 // prematurely.
486 DCHECK(!m_isSchedulingReload); 488 DCHECK(!m_isSchedulingReload);
487 m_isSchedulingReload = true; 489 m_isSchedulingReload = true;
488 490
489 setCachePolicyBypassingCache(); 491 setCachePolicyBypassingCache();
490 492
493 WebURLRequest::PreviewsState previewsState =
sclittle 2017/04/06 21:58:06 nit: maybe name this "previewsStateBeforeReload" o
Raj 2017/05/03 07:05:18 Renamed as old_previews_state as per new naming co
494 resourceRequest().previewsState();
495
491 setPreviewsStateNoTransform(); 496 setPreviewsStateNoTransform();
492 497
493 if (m_placeholderOption != PlaceholderOption::DoNotReloadPlaceholder) 498 if (m_placeholderOption != PlaceholderOption::DoNotReloadPlaceholder)
494 clearRangeRequestHeader(); 499 clearRangeRequestHeader();
495 m_placeholderOption = PlaceholderOption::DoNotReloadPlaceholder; 500
501 if (previewsState & WebURLRequest::ClientLoFiOn) {
sclittle 2017/04/06 21:58:06 Chrome should still show the full image if the use
Raj 2017/05/03 07:05:18 Thanks. Added this check.
502 m_placeholderOption = PlaceholderOption::ShowAndDoNotReloadPlaceholder;
503 } else {
504 m_placeholderOption = PlaceholderOption::DoNotReloadPlaceholder;
505 }
sclittle 2017/04/06 21:58:06 Could you add tests for the new behavior you've ad
496 506
497 if (isLoading()) { 507 if (isLoading()) {
498 loader()->cancel(); 508 loader()->cancel();
499 // Canceling the loader causes error() to be called, which in turn calls 509 // Canceling the loader causes error() to be called, which in turn calls
500 // clear() and notifyObservers(), so there's no need to call these again 510 // clear() and notifyObservers(), so there's no need to call these again
501 // here. 511 // here.
502 } else { 512 } else {
503 clearData(); 513 clearData();
504 setEncodedSize(0); 514 setEncodedSize(0);
505 updateImage(nullptr, ImageResourceContent::ClearImageAndNotifyObservers, 515 updateImage(nullptr, ImageResourceContent::ClearImageAndNotifyObservers,
(...skipping 86 matching lines...) Expand 10 before | Expand all | Expand 10 after
592 // reloading in Step 3 due to notifyObservers()'s 602 // reloading in Step 3 due to notifyObservers()'s
593 // schedulingReloadOrShouldReloadBrokenPlaceholder() check. 603 // schedulingReloadOrShouldReloadBrokenPlaceholder() check.
594 // 3. reloadIfLoFiOrPlaceholderImage() is called via ResourceFetcher 604 // 3. reloadIfLoFiOrPlaceholderImage() is called via ResourceFetcher
595 // (a) via didFinishLoading() called in decodeError(), or 605 // (a) via didFinishLoading() called in decodeError(), or
596 // (b) after returning ImageResource::updateImage(). 606 // (b) after returning ImageResource::updateImage().
597 decodeError(allDataReceived); 607 decodeError(allDataReceived);
598 } 608 }
599 } 609 }
600 610
601 } // namespace blink 611 } // namespace blink
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698