chrome/browser/ui/webui/ntp/ntp_resource_cache.cc - Issue 1440113002: [MD] Change appearance of incognito NTP to MD spec.

Side by Side Diff: chrome/browser/ui/webui/ntp/ntp_resource_cache.cc

Issue 1440113002: [MD] Change appearance of incognito NTP to MD spec. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: guest Created 5 years, 1 month 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

OLD	NEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.	1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be	2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 #include "chrome/browser/ui/webui/ntp/ntp_resource_cache.h"	5 #include "chrome/browser/ui/webui/ntp/ntp_resource_cache.h"

6	6

7 #include <string>	7 #include <string>

8 #include <vector>	8 #include <vector>

9	9

10 #include "base/command_line.h"	10 #include "base/command_line.h"

(...skipping 243 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
254 // NOTE: NewTabCacheNeedsRefresh() must be called every time the new tab	254 // NOTE: NewTabCacheNeedsRefresh() must be called every time the new tab

255 // HTML is fetched, because it needs to initialize cached values.	255 // HTML is fetched, because it needs to initialize cached values.

256 if (NewTabCacheNeedsRefresh() \|\| !new_tab_html_.get())	256 if (NewTabCacheNeedsRefresh() \|\| !new_tab_html_.get())

257 CreateNewTabHTML();	257 CreateNewTabHTML();

258 return new_tab_html_.get();	258 return new_tab_html_.get();

259 }	259 }

260 }	260 }

261	261

262 base::RefCountedMemory* NTPResourceCache::GetNewTabCSS(WindowType win_type) {	262 base::RefCountedMemory* NTPResourceCache::GetNewTabCSS(WindowType win_type) {

263 DCHECK_CURRENTLY_ON(BrowserThread::UI);	263 DCHECK_CURRENTLY_ON(BrowserThread::UI);

264 if (win_type == GUEST) {	264 DCHECK_NE(win_type, GUEST);

265 if (!new_tab_guest_css_.get())	265

266 CreateNewTabGuestCSS();	266 if (win_type == INCOGNITO) {

267 return new_tab_guest_css_.get();

268 } else if (win_type == INCOGNITO) {

269 if (!new_tab_incognito_css_.get())	267 if (!new_tab_incognito_css_.get())

270 CreateNewTabIncognitoCSS();	268 CreateNewTabIncognitoCSS();

271 return new_tab_incognito_css_.get();	269 return new_tab_incognito_css_.get();

272 } else {

273 if (!new_tab_css_.get())

274 CreateNewTabCSS();

275 return new_tab_css_.get();

276 }	270 }

	271

	272 if (!new_tab_css_.get())
	Dan Beam 2015/11/13 01:32:22 nit: .get() isn't necessary here nit: .get() isn't necessary here Evan Stade 2015/11/13 01:39:00 Acknowledged (will fix). Show quoted text On 2015/11/13 01:32:22, Dan Beam wrote: > nit: .get() isn't necessary here Acknowledged (will fix). Evan Stade 2015/11/19 23:05:24 Done. Show quoted text On 2015/11/13 01:39:00, Evan Stade wrote: > On 2015/11/13 01:32:22, Dan Beam wrote: > > nit: .get() isn't necessary here > > Acknowledged (will fix). Done.
	273 CreateNewTabCSS();

	274 return new_tab_css_.get();

277 }	275 }

278	276

279 void NTPResourceCache::Observe(int type,	277 void NTPResourceCache::Observe(int type,

280 const content::NotificationSource& source,	278 const content::NotificationSource& source,

281 const content::NotificationDetails& details) {	279 const content::NotificationDetails& details) {

282 // Invalidate the cache.	280 // Invalidate the cache.

283 if (chrome::NOTIFICATION_BROWSER_THEME_CHANGED == type \|\|	281 if (chrome::NOTIFICATION_BROWSER_THEME_CHANGED == type \|\|

284 chrome::NOTIFICATION_PROMO_RESOURCE_STATE_CHANGED == type) {	282 chrome::NOTIFICATION_PROMO_RESOURCE_STATE_CHANGED == type) {

285 Invalidate();	283 Invalidate();

286 } else {	284 } else {

(...skipping 245 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
532 webui::GetI18nTemplateHtml(new_tab_html, &load_time_data);	530 webui::GetI18nTemplateHtml(new_tab_html, &load_time_data);

533 new_tab_html_ = base::RefCountedString::TakeString(&full_html);	531 new_tab_html_ = base::RefCountedString::TakeString(&full_html);

534 }	532 }

535	533

536 void NTPResourceCache::CreateNewTabIncognitoCSS() {	534 void NTPResourceCache::CreateNewTabIncognitoCSS() {

537 ui::ThemeProvider* tp = ThemeServiceFactory::GetForProfile(profile_);	535 ui::ThemeProvider* tp = ThemeServiceFactory::GetForProfile(profile_);

538 DCHECK(tp);	536 DCHECK(tp);

539	537

540 // Get our theme colors	538 // Get our theme colors

541 SkColor color_background =	539 SkColor color_background =

542 GetThemeColor(tp, ThemeProperties::COLOR_NTP_BACKGROUND);	540 tp->HasCustomImage(IDR_THEME_NTP_BACKGROUND)

	541 ? GetThemeColor(tp, ThemeProperties::COLOR_NTP_BACKGROUND)

	542 : SkColorSetRGB(0x32, 0x32, 0x32);

543	543

544 // Generate the replacements.	544 // Generate the replacements.

545 std::map<base::StringPiece, std::string> substitutions;	545 std::map<base::StringPiece, std::string> substitutions;

546	546

547 // Cache-buster for background.	547 // Cache-buster for background.

548 substitutions["themeId"] =	548 substitutions["themeId"] =

549 profile_->GetPrefs()->GetString(prefs::kCurrentThemeID);	549 profile_->GetPrefs()->GetString(prefs::kCurrentThemeID);

550	550

551 // Colors.	551 // Colors.

552 substitutions["colorBackground"] = SkColorToRGBAString(color_background);	552 substitutions["colorBackground"] = SkColorToRGBAString(color_background);

553 substitutions["backgroundBarDetached"] = GetNewTabBackgroundCSS(tp, false);	553 substitutions["backgroundBarDetached"] = GetNewTabBackgroundCSS(tp, false);

554 substitutions["backgroundBarAttached"] = GetNewTabBackgroundCSS(tp, true);	554 substitutions["backgroundBarAttached"] = GetNewTabBackgroundCSS(tp, true);

555 substitutions["backgroundTiling"] = GetNewTabBackgroundTilingCSS(tp);	555 substitutions["backgroundTiling"] = GetNewTabBackgroundTilingCSS(tp);

	556 substitutions["hasCustomBackground"] =

	557 !tp->HasCustomImage(IDR_THEME_NTP_BACKGROUND)

	558 ? ".doesNotHaveCustomBackground"
	Dan Beam 2015/11/13 01:32:22 i get that this is simple, but I think it is very i get that this is simple, but I think it is very different to how you typically see classes applied in CSS could we invert the logic and add: <body class="${hasCustomBackground}"> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in the HTML and run ui::ReplaceTemplateExpressions() on it? fwiw: we'd like to eventually move all <div i18n-content="blah"></div> to <div>${blah}</div> to reduce flicker, remove the need to do i18nTemplate.process() when the page is starting, and in theory for better security (would remove some uses of innerHTML). this is a very simple page and a nice place to start. Evan Stade 2015/11/13 01:39:00 That sounds nightmarishly difficult because only t Show quoted text On 2015/11/13 01:32:22, Dan Beam wrote: > i get that this is simple, but I think it is very different to how you typically > see classes applied in CSS > > could we invert the logic and add: > > <body class="${hasCustomBackground}"> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > in the HTML and run ui::ReplaceTemplateExpressions() on it? fwiw: we'd like to > eventually move all > > <div i18n-content="blah"></div> > > to > > <div>${blah}</div> > > to reduce flicker, remove the need to do i18nTemplate.process() when the page is > starting, and in theory for better security (would remove some uses of > innerHTML). this is a very simple page and a nice place to start. That sounds nightmarishly difficult because only the theme css changes when the theme changes, and refreshing HTML too would be a huge headache, so I'd really like to do this in the css, in this function. I agree that it's oddball and backwards from what you'd expect. I think this whole file is a bit crooked so it's tricky to build something stable on top of it. Dan Beam 2015/11/16 19:57:25 can't you just add a hasCustomBackground boolean p Show quoted text On 2015/11/13 01:39:00, Evan Stade wrote: > On 2015/11/13 01:32:22, Dan Beam wrote: > > i get that this is simple, but I think it is very different to how you > typically > > see classes applied in CSS > > > > could we invert the logic and add: > > > > <body class="${hasCustomBackground}"> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > in the HTML and run ui::ReplaceTemplateExpressions() on it? fwiw: we'd like > to > > eventually move all > > > > <div i18n-content="blah"></div> > > > > to > > > > <div>${blah}</div> > > > > to reduce flicker, remove the need to do i18nTemplate.process() when the page > is > > starting, and in theory for better security (would remove some uses of > > innerHTML). this is a very simple page and a nice place to start. > > That sounds nightmarishly difficult because only the theme css changes when the > theme changes, and refreshing HTML too would be a huge headache, so I'd really > like to do this in the css, in this function. can't you just add a hasCustomBackground boolean parameter to this and also null out the cached HTML? https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/... Show quoted text > > I agree that it's oddball and backwards from what you'd expect. I think this > whole file is a bit crooked so it's tricky to build something stable on top of > it. i don't think something being oddly organized is a reason to make it more odd Evan Stade 2015/11/16 20:10:08 no. That would work for any NTPs you open after a Show quoted text On 2015/11/16 19:57:25, Dan Beam wrote: > On 2015/11/13 01:39:00, Evan Stade wrote: > > On 2015/11/13 01:32:22, Dan Beam wrote: > > > i get that this is simple, but I think it is very different to how you > > typically > > > see classes applied in CSS > > > > > > could we invert the logic and add: > > > > > > <body class="${hasCustomBackground}"> > > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > in the HTML and run ui::ReplaceTemplateExpressions() on it? fwiw: we'd like > > to > > > eventually move all > > > > > > <div i18n-content="blah"></div> > > > > > > to > > > > > > <div>${blah}</div> > > > > > > to reduce flicker, remove the need to do i18nTemplate.process() when the > page > > is > > > starting, and in theory for better security (would remove some uses of > > > innerHTML). this is a very simple page and a nice place to start. > > > > That sounds nightmarishly difficult because only the theme css changes when > the > > theme changes, and refreshing HTML too would be a huge headache, so I'd really > > like to do this in the css, in this function. > > can't you just add a hasCustomBackground boolean parameter to this and also null > out the cached HTML? no. That would work for any NTPs you open after a theme change, but it would not update the NTPs that are already open. Show quoted text > https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/... > > > > > I agree that it's oddball and backwards from what you'd expect. I think this > > whole file is a bit crooked so it's tricky to build something stable on top of > > it. > > i don't think something being oddly organized is a reason to make it more odd Dan Beam 2015/11/17 00:18:59 i'm saying pass a hasCustomBackground boolean to j Show quoted text On 2015/11/16 20:10:08, Evan Stade wrote: > On 2015/11/16 19:57:25, Dan Beam wrote: > > On 2015/11/13 01:39:00, Evan Stade wrote: > > > On 2015/11/13 01:32:22, Dan Beam wrote: > > > > i get that this is simple, but I think it is very different to how you > > > typically > > > > see classes applied in CSS > > > > > > > > could we invert the logic and add: > > > > > > > > <body class="${hasCustomBackground}"> > > > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > in the HTML and run ui::ReplaceTemplateExpressions() on it? fwiw: we'd > like > > > to > > > > eventually move all > > > > > > > > <div i18n-content="blah"></div> > > > > > > > > to > > > > > > > > <div>${blah}</div> > > > > > > > > to reduce flicker, remove the need to do i18nTemplate.process() when the > > page > > > is > > > > starting, and in theory for better security (would remove some uses of > > > > innerHTML). this is a very simple page and a nice place to start. > > > > > > That sounds nightmarishly difficult because only the theme css changes when > > the > > > theme changes, and refreshing HTML too would be a huge headache, so I'd > really > > > like to do this in the css, in this function. > > > > can't you just add a hasCustomBackground boolean parameter to this and also > null > > out the cached HTML? > > no. That would work for any NTPs you open after a theme change, but it would not > update the NTPs that are already open. i'm saying pass a hasCustomBackground boolean to js, in the js do: document.classList.toggle('has-custom-background', hasCustomBackground); then reload the theme stylesheet. that would update the running HTML page. Show quoted text > > > > https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/... > > > > > > > > I agree that it's oddball and backwards from what you'd expect. I think this > > > whole file is a bit crooked so it's tricky to build something stable on top > of > > > it. > > > > i don't think something being oddly organized is a reason to make it more odd > Dan Beam 2015/11/17 00:26:15 document.body. Show quoted text On 2015/11/17 00:18:59, Dan Beam wrote: > On 2015/11/16 20:10:08, Evan Stade wrote: > > On 2015/11/16 19:57:25, Dan Beam wrote: > > > On 2015/11/13 01:39:00, Evan Stade wrote: > > > > On 2015/11/13 01:32:22, Dan Beam wrote: > > > > > i get that this is simple, but I think it is very different to how you > > > > typically > > > > > see classes applied in CSS > > > > > > > > > > could we invert the logic and add: > > > > > > > > > > <body class="${hasCustomBackground}"> > > > > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > > in the HTML and run ui::ReplaceTemplateExpressions() on it? fwiw: we'd > > like > > > > to > > > > > eventually move all > > > > > > > > > > <div i18n-content="blah"></div> > > > > > > > > > > to > > > > > > > > > > <div>${blah}</div> > > > > > > > > > > to reduce flicker, remove the need to do i18nTemplate.process() when the > > > page > > > > is > > > > > starting, and in theory for better security (would remove some uses of > > > > > innerHTML). this is a very simple page and a nice place to start. > > > > > > > > That sounds nightmarishly difficult because only the theme css changes > when > > > the > > > > theme changes, and refreshing HTML too would be a huge headache, so I'd > > really > > > > like to do this in the css, in this function. > > > > > > can't you just add a hasCustomBackground boolean parameter to this and also > > null > > > out the cached HTML? > > > > no. That would work for any NTPs you open after a theme change, but it would > not > > update the NTPs that are already open. > > i'm saying pass a hasCustomBackground boolean to js, in the js do: > > document.classList.toggle('has-custom-background', hasCustomBackground); > > then reload the theme stylesheet. that would update the running HTML page. > > > > > > > > > https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/... > > > > > > > > > > > I agree that it's oddball and backwards from what you'd expect. I think > this > > > > whole file is a bit crooked so it's tricky to build something stable on > top > > of > > > > it. > > > > > > i don't think something being oddly organized is a reason to make it more > odd > > > document.body. Evan Stade 2015/11/19 23:05:24 Done. Show quoted text On 2015/11/17 00:26:15, Dan Beam wrote: > On 2015/11/17 00:18:59, Dan Beam wrote: > > On 2015/11/16 20:10:08, Evan Stade wrote: > > > On 2015/11/16 19:57:25, Dan Beam wrote: > > > > On 2015/11/13 01:39:00, Evan Stade wrote: > > > > > On 2015/11/13 01:32:22, Dan Beam wrote: > > > > > > i get that this is simple, but I think it is very different to how you > > > > > typically > > > > > > see classes applied in CSS > > > > > > > > > > > > could we invert the logic and add: > > > > > > > > > > > > <body class="${hasCustomBackground}"> > > > > > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > > > in the HTML and run ui::ReplaceTemplateExpressions() on it? fwiw: > we'd > > > like > > > > > to > > > > > > eventually move all > > > > > > > > > > > > <div i18n-content="blah"></div> > > > > > > > > > > > > to > > > > > > > > > > > > <div>${blah}</div> > > > > > > > > > > > > to reduce flicker, remove the need to do i18nTemplate.process() when > the > > > > page > > > > > is > > > > > > starting, and in theory for better security (would remove some uses of > > > > > > innerHTML). this is a very simple page and a nice place to start. > > > > > > > > > > That sounds nightmarishly difficult because only the theme css changes > > when > > > > the > > > > > theme changes, and refreshing HTML too would be a huge headache, so I'd > > > really > > > > > like to do this in the css, in this function. > > > > > > > > can't you just add a hasCustomBackground boolean parameter to this and > also > > > null > > > > out the cached HTML? > > > > > > no. That would work for any NTPs you open after a theme change, but it would > > not > > > update the NTPs that are already open. > > > > i'm saying pass a hasCustomBackground boolean to js, in the js do: > > > > document.classList.toggle('has-custom-background', hasCustomBackground); > > > > then reload the theme stylesheet. that would update the running HTML page. > > > > > > > > > > > > > > > https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/... > > > > > > > > > > > > > > I agree that it's oddball and backwards from what you'd expect. I think > > this > > > > > whole file is a bit crooked so it's tricky to build something stable on > > top > > > of > > > > > it. > > > > > > > > i don't think something being oddly organized is a reason to make it more > > odd > > > > > > > document.body.** Done.
	559 : std::string();

556	560

557 // Get our template.	561 // Get our template.

558 static const base::StringPiece new_tab_theme_css(	562 static const base::StringPiece new_tab_theme_css(

559 ResourceBundle::GetSharedInstance().GetRawDataResource(	563 ResourceBundle::GetSharedInstance().GetRawDataResource(

560 IDR_NEW_INCOGNITO_TAB_THEME_CSS));	564 IDR_NEW_INCOGNITO_TAB_THEME_CSS));

561	565

562 // Create the string from our template and the replacements.	566 // Create the string from our template and the replacements.

563 std::string full_css =	567 std::string full_css =

564 ui::ReplaceTemplateExpressions(new_tab_theme_css, substitutions);	568 ui::ReplaceTemplateExpressions(new_tab_theme_css, substitutions);

565	569

566 new_tab_incognito_css_ = base::RefCountedString::TakeString(&full_css);	570 new_tab_incognito_css_ = base::RefCountedString::TakeString(&full_css);

567 }	571 }

568	572

569 void NTPResourceCache::CreateNewTabGuestCSS() {

570 ui::ThemeProvider* tp = ThemeServiceFactory::GetForProfile(profile_);

571 DCHECK(tp);

572

573 // Get our theme colors

574 SkColor color_background =

575 GetThemeColor(tp, ThemeProperties::COLOR_NTP_BACKGROUND);

576

577 // Generate the replacements.

578 std::map<base::StringPiece, std::string> substitutions;

579

580 // Cache-buster for background.

581 substitutions["themeId"] =

582 profile_->GetPrefs()->GetString(prefs::kCurrentThemeID);

583

584 // Colors.

585 substitutions["colorBackground"] = SkColorToRGBAString(color_background);

586 substitutions["backgroundBarDetached"] = GetNewTabBackgroundCSS(tp, false);

587 substitutions["backgroundBarAttached"] = GetNewTabBackgroundCSS(tp, true);

588 substitutions["backgroundTiling"] = GetNewTabBackgroundTilingCSS(tp);

589

590 // Get our template.

591 static const base::StringPiece new_tab_theme_css(

592 ResourceBundle::GetSharedInstance().GetRawDataResource(

593 IDR_NEW_INCOGNITO_TAB_THEME_CSS));

594

595 // Create the string from our template and the replacements.

596 std::string full_css =

597 ui::ReplaceTemplateExpressions(new_tab_theme_css, substitutions);

598

599 new_tab_guest_css_ = base::RefCountedString::TakeString(&full_css);

600 }

601

602 void NTPResourceCache::CreateNewTabCSS() {	573 void NTPResourceCache::CreateNewTabCSS() {

603 ui::ThemeProvider* tp = ThemeServiceFactory::GetForProfile(profile_);	574 ui::ThemeProvider* tp = ThemeServiceFactory::GetForProfile(profile_);

604 DCHECK(tp);	575 DCHECK(tp);

605	576

606 // Get our theme colors	577 // Get our theme colors

607 SkColor color_background =	578 SkColor color_background =

608 GetThemeColor(tp, ThemeProperties::COLOR_NTP_BACKGROUND);	579 GetThemeColor(tp, ThemeProperties::COLOR_NTP_BACKGROUND);

609 SkColor color_text = GetThemeColor(tp, ThemeProperties::COLOR_NTP_TEXT);	580 SkColor color_text = GetThemeColor(tp, ThemeProperties::COLOR_NTP_TEXT);

610 SkColor color_text_light =	581 SkColor color_text_light =

611 GetThemeColor(tp, ThemeProperties::COLOR_NTP_TEXT_LIGHT);	582 GetThemeColor(tp, ThemeProperties::COLOR_NTP_TEXT_LIGHT);

(...skipping 54 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
666 // Get our template.	637 // Get our template.

667 static const base::StringPiece new_tab_theme_css(	638 static const base::StringPiece new_tab_theme_css(

668 ResourceBundle::GetSharedInstance().GetRawDataResource(	639 ResourceBundle::GetSharedInstance().GetRawDataResource(

669 IDR_NEW_TAB_4_THEME_CSS));	640 IDR_NEW_TAB_4_THEME_CSS));

670	641

671 // Create the string from our template and the replacements.	642 // Create the string from our template and the replacements.

672 std::string css_string =	643 std::string css_string =

673 ui::ReplaceTemplateExpressions(new_tab_theme_css, substitutions);	644 ui::ReplaceTemplateExpressions(new_tab_theme_css, substitutions);

674 new_tab_css_ = base::RefCountedString::TakeString(&css_string);	645 new_tab_css_ = base::RefCountedString::TakeString(&css_string);

675 }	646 }

OLD	NEW

« no previous file with comments | « chrome/browser/ui/webui/ntp/ntp_resource_cache.h ('k') | chrome/browser/ui/webui/theme_source.cc » ('j') | chrome/browser/ui/webui/theme_source.cc » ('J')