New issue
Advanced search Search tips

Issue 669776 link

Starred by 1 user
Status: Fixed
Owner:
toyoshim@chromium.org
Closed: Mar 2017
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocked on:
issue 668454


Sign in to add a comment

Faster Reload: check if existing URLDataSource inheritances work fine after the reload change

Project Member Reported by toyoshim@chromium.org, Nov 30 2016 
There is a regression on NTP page reload after the reload behavior change.

Backgrounds:
- Chrome uses URLDataSource to return response for internal resources, e.g. chrome-search://local-ntp/config.js, and data URLDataSource returns allows caching by default.
- Some resources are dynamically created and could be different on each load, or reload.
- On reload, only the main resource is forcibly revalidated after the reload behavior changes.

Results:
- Chrome load sub-resources from memory cache, and could not reflect configuration changes

See  crbug.com/668454  for actual regression issue.

Comment 1 by toyoshim@chromium.org, Nov 30 2016 

https://cs.chromium.org/chromium/src/content/public/browser/url_data_source.h?q=URLDataSource&dr=CSs&l=32

Subclasses (32 occurrences)
	src/chrome/browser/ui/webui/chromeos/certificate_manager_dialog_ui.cc	 (1 occurrence)
     36 class CertificateManagerDialogHTMLSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/devtools_ui.cc	 (1 occurrence)
    212 class DevToolsDataSource : public content::URLDataSource,
	src/chrome/browser/ui/webui/interstitials/interstitial_ui.cc	 (1 occurrence)
     66 class InterstitialHTMLSource : public content::URLDataSource {
	src/chrome/test/base/web_ui_browser_test.cc	 (1 occurrence)
    333 class MockWebUIDataSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/chromeos/proxy_settings_ui.cc	 (1 occurrence)
     34 class ProxySettingsHTMLSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/chromeos/image_source.h	 (1 occurrence)
     23 class ImageSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/chromeos/login/screenlock_icon_source.h	 (1 occurrence)
     17 class ScreenlockIconSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/chromeos/sim_unlock_ui.cc	 (1 occurrence)
     82 class SimUnlockUIHTMLSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/chromeos/slow_trace_ui.h	 (1 occurrence)
     26 class SlowTraceSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/options/chromeos/user_image_source.h	 (1 occurrence)
     28 class UserImageSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/about_ui.h	 (1 occurrence)
     19 class AboutUIHTMLSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/app_launcher_page_ui.h	 (1 occurrence)
     34 class HTMLSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/bookmarks_ui.h	 (1 occurrence)
     21 class BookmarksUIHTMLSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/fallback_icon_source.h	 (1 occurrence)
     54 class FallbackIconSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/favicon_source.h	 (1 occurrence)
     58 class FaviconSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/fileicon_source.h	 (1 occurrence)
     22 class FileIconSource : public content::URLDataSource {
	src/chrome/browser/search/iframe_source.h	 (1 occurrence)
     13 class IframeSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/large_icon_source.h	 (1 occurrence)
     38 class LargeIconSource : public content::URLDataSource {
	src/chrome/browser/search/local_ntp_source.h	 (1 occurrence)
     16 class LocalNtpSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc	 (1 occurrence)
    145 class MobileSetupUIHTMLSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/ntp/new_tab_ui.h	 (1 occurrence)
     57 class NewTabHTMLSource : public content::URLDataSource {
	src/chrome/browser/devtools/devtools_sanity_browsertest.cc	 (1 occurrence)
   1475 class StaticURLDataSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/system_info_ui.cc	 (1 occurrence)
     46 class SystemInfoUIHTMLSource : public content::URLDataSource{
	src/chrome/browser/ui/webui/theme_source.h	 (1 occurrence)
     17 class ThemeSource : public content::URLDataSource {
	src/chrome/browser/thumbnails/thumbnail_list_source.h	 (1 occurrence)
     30 class ThumbnailListSource : public content::URLDataSource {
	src/chrome/browser/search/thumbnail_source.h	 (1 occurrence)
     37 class ThumbnailSource : public content::URLDataSource {
	src/content/browser/webui/shared_resources_data_source.h	 (1 occurrence)
     15 class SharedResourcesDataSource : public URLDataSource {
	src/content/browser/webui/web_ui_data_source_impl.cc	 (1 occurrence)
     40 class WebUIDataSourceImpl::InternalDataSource : public URLDataSource {
	src/components/dom_distiller/content/browser/dom_distiller_viewer_source.h	 (1 occurrence)
     22 class DomDistillerViewerSource : public content::URLDataSource {
	src/chrome/browser/ui/webui/extensions/extension_icon_source.h	 (1 occurrence)
     52 class ExtensionIconSource : public content::URLDataSource,
	src/chrome/browser/ui/webui/options/options_ui.cc	 (1 occurrence)
    157 class OptionsUIHTMLSource : public content::URLDataSource {
	src/chrome/browser/search/suggestions/suggestions_ui.cc	 (1 occurrence)
    115 class SuggestionsSource : public content::URLDataSource {
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 25 2017 

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

commit c782ea5b9c5d4e85c75ab8a2d601a088376cc1af
Author: treib <treib@chromium.org>
Date: Wed Jan 25 13:24:40 2017

Faster Reload: Specify AllowCaching = false for some URLDataSources that need it

BUG= 669776 

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

[modify] https://crrev.com/c782ea5b9c5d4e85c75ab8a2d601a088376cc1af/chrome/browser/search/iframe_source.cc
[modify] https://crrev.com/c782ea5b9c5d4e85c75ab8a2d601a088376cc1af/chrome/browser/search/iframe_source.h
[modify] https://crrev.com/c782ea5b9c5d4e85c75ab8a2d601a088376cc1af/chrome/browser/search/thumbnail_source.cc
[modify] https://crrev.com/c782ea5b9c5d4e85c75ab8a2d601a088376cc1af/chrome/browser/search/thumbnail_source.h
[modify] https://crrev.com/c782ea5b9c5d4e85c75ab8a2d601a088376cc1af/chrome/browser/ui/webui/fallback_icon_source.cc
[modify] https://crrev.com/c782ea5b9c5d4e85c75ab8a2d601a088376cc1af/chrome/browser/ui/webui/fallback_icon_source.h
[modify] https://crrev.com/c782ea5b9c5d4e85c75ab8a2d601a088376cc1af/chrome/browser/ui/webui/favicon_source.cc
[modify] https://crrev.com/c782ea5b9c5d4e85c75ab8a2d601a088376cc1af/chrome/browser/ui/webui/favicon_source.h
[modify] https://crrev.com/c782ea5b9c5d4e85c75ab8a2d601a088376cc1af/chrome/browser/ui/webui/fileicon_source.cc
[modify] https://crrev.com/c782ea5b9c5d4e85c75ab8a2d601a088376cc1af/chrome/browser/ui/webui/fileicon_source.h
[modify] https://crrev.com/c782ea5b9c5d4e85c75ab8a2d601a088376cc1af/chrome/browser/ui/webui/large_icon_source.cc
[modify] https://crrev.com/c782ea5b9c5d4e85c75ab8a2d601a088376cc1af/chrome/browser/ui/webui/large_icon_source.h
[modify] https://crrev.com/c782ea5b9c5d4e85c75ab8a2d601a088376cc1af/chrome/browser/ui/webui/theme_source.cc
[modify] https://crrev.com/c782ea5b9c5d4e85c75ab8a2d601a088376cc1af/chrome/browser/ui/webui/theme_source.h
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 30 2017 

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

commit bb7ab1896220b565660dc68f6d6304c1bcea002e
Author: toyoshim <toyoshim@chromium.org>
Date: Thu Mar 30 05:28:08 2017

Specify AllowCaching = false for dynamically-generated contents

WebUI still have some resources that may be modified when user settings
are updated, or some other background tasks, e.g. updating Extensions,
run. Just in case, such URLDataSource inheritances should not allow
caching so to reflect modified contents immediately.

BUG= 669776 

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

[modify] https://crrev.com/bb7ab1896220b565660dc68f6d6304c1bcea002e/chrome/browser/ui/webui/app_launcher_page_ui.cc
[modify] https://crrev.com/bb7ab1896220b565660dc68f6d6304c1bcea002e/chrome/browser/ui/webui/app_launcher_page_ui.h
[modify] https://crrev.com/bb7ab1896220b565660dc68f6d6304c1bcea002e/chrome/browser/ui/webui/extensions/extension_icon_source.cc
[modify] https://crrev.com/bb7ab1896220b565660dc68f6d6304c1bcea002e/chrome/browser/ui/webui/extensions/extension_icon_source.h
[modify] https://crrev.com/bb7ab1896220b565660dc68f6d6304c1bcea002e/chrome/browser/ui/webui/options/options_ui.cc
[modify] https://crrev.com/bb7ab1896220b565660dc68f6d6304c1bcea002e/content/browser/webui/shared_resources_data_source.cc
[modify] https://crrev.com/bb7ab1896220b565660dc68f6d6304c1bcea002e/content/browser/webui/shared_resources_data_source.h

Comment 7 by toyoshim@chromium.org, Mar 30 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment