Issue 10407066: NTP: Prevent tab sync icons from being cached.

Issue 10407066: NTP: Prevent tab sync icons from being cached. (Closed)

Created:
8 years, 7 months ago by Patrick Dubroy

Modified:
8 years, 7 months ago

Reviewers:
James Hawkins

CC:
chromium-reviews

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

NTP: Prevent tab sync icons from being cached. BUG=127463 TEST=Manual. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=138112

Patch Set 2 : Address comments. #

Created: 8 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+24 lines, -6 lines)			Patch
M	chrome/browser/ui/webui/chrome_url_data_manager.h	View		1 chunk	+3 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/webui/chrome_url_data_manager_backend.cc	View	1	5 chunks	+13 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/webui/session_favicon_source.h	View	1	1 chunk	+2 lines, -4 lines	0 comments	Download
M	chrome/browser/ui/webui/session_favicon_source.cc	View		1 chunk	+6 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Patrick Dubroy

Hi James, Please take a look. BTW, we may consider merging this into M20, since ...

8 years, 7 months ago (2012-05-21 15:36:25 UTC) #1

James Hawkins

LGTM with nits. http://codereview.chromium.org/10407066/diff/1/chrome/browser/ui/webui/chrome_url_data_manager_backend.cc File chrome/browser/ui/webui/chrome_url_data_manager_backend.cc (right): http://codereview.chromium.org/10407066/diff/1/chrome/browser/ui/webui/chrome_url_data_manager_backend.cc#newcode180 chrome/browser/ui/webui/chrome_url_data_manager_backend.cc:180: void SetMimeType(const std::string& mime_type) { nit: ...

8 years, 7 months ago (2012-05-21 15:58:50 UTC) #2

Patrick Dubroy

8 years, 7 months ago (2012-05-21 16:26:28 UTC) #3

Thanks for the quick review!

http://codereview.chromium.org/10407066/diff/1/chrome/browser/ui/webui/chrome...
File chrome/browser/ui/webui/chrome_url_data_manager_backend.cc (right):

http://codereview.chromium.org/10407066/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chrome_url_data_manager_backend.cc:180: void
SetMimeType(const std::string& mime_type) {
On 2012/05/21 15:58:50, James Hawkins wrote:
> nit: Per style-guide, these should be set_mime_type and set_allow_caching.

Done.

http://codereview.chromium.org/10407066/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chrome_url_data_manager_backend.cc:210: bool
allow_caching_;
On 2012/05/21 15:58:50, James Hawkins wrote:
> nit: Document member variable.

Done.

http://codereview.chromium.org/10407066/diff/1/chrome/browser/ui/webui/sessio...
File chrome/browser/ui/webui/session_favicon_source.h (right):

http://codereview.chromium.org/10407066/diff/1/chrome/browser/ui/webui/sessio...
chrome/browser/ui/webui/session_favicon_source.h:24: virtual void
StartDataRequest(const std::string& path,
On 2012/05/21 15:58:50, James Hawkins wrote:
> nit:
> 
> // FaviconSource implementation.

Done.

http://codereview.chromium.org/10407066/diff/1/chrome/browser/ui/webui/sessio...
chrome/browser/ui/webui/session_favicon_source.h:27: 
On 2012/05/21 15:58:50, James Hawkins wrote:
> nit: No blank lines for Interface implementation blocks.

Done.

Expand Messages | Collapse Messages