Commit cfd29877 authored by Alexei Svitkine's avatar Alexei Svitkine Committed by Commit Bot

Reland "Cache brand code on Windows."

This reverts commit b93f6ee5.

Reason for revert: Reverting showed the relevant metrics regressed, so this change did have the expected improvement.

Original change's description:
> Revert "Cache brand code on Windows."
> 
> This reverts commit 9dd1ef42.
> 
> Reason for revert: Latest canary data did not have expected recovery on Omnibox.CharTypedToRepaintLatency and had a regression on Omnibox.CharTypedToRepaintLatency.ToPaint. Reverting to see if that movement is related to this CL or whether there were other changes in the meanwhile.
> 
> Original change's description:
> > Cache brand code on Windows.
> > 
> > Querying the registry is expensive and contributes to start up time,
> > omnibox query time and idle. Since the brand code is not expected to
> > change on Windows during the course of a session, this CL caches
> > the value, so that future queries are free.
> > 
> > This should restore the Omnibox perf that regressed by
> > https://chromium-review.googlesource.com/c/chromium/src/+/824363
> > and also improve perf elsewhere since the caching is now done at
> > a lower level than that change. Since this is expensive only on
> > Windows, the caching is done in the Windows codepath only.
> > 
> > Bug: 816698, 806130
> > Change-Id: I458fad29f18b1b1ff2d55b9334ae733fa448eec9
> > Reviewed-on: https://chromium-review.googlesource.com/947522
> > Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
> > Reviewed-by: Peter Kasting <pkasting@chromium.org>
> > Reviewed-by: Greg Thompson <grt@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#541009}
> 
> TBR=pkasting@chromium.org,asvitkine@chromium.org,grt@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug: 816698, 806130
> Change-Id: I09e12c17011f72f25ecb369ca3075e2067013f8a
> Reviewed-on: https://chromium-review.googlesource.com/958012
> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#542506}

TBR=pkasting@chromium.org,asvitkine@chromium.org,grt@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 816698, 806130
Change-Id: Id70bcaa6be1e91240010e74990dd2bd81d673258
Reviewed-on: https://chromium-review.googlesource.com/973581Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544723}
parent cb8d800b
...@@ -8,6 +8,8 @@ ...@@ -8,6 +8,8 @@
#include <string> #include <string>
#include "base/macros.h" #include "base/macros.h"
#include "base/no_destructor.h"
#include "base/optional.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "build/build_config.h" #include "build/build_config.h"
...@@ -41,11 +43,20 @@ bool GetBrand(std::string* brand) { ...@@ -41,11 +43,20 @@ bool GetBrand(std::string* brand) {
return true; return true;
} }
base::string16 brand16; // Cache brand code value, since it is queried a lot and registry queries are
bool ret = GoogleUpdateSettings::GetBrand(&brand16); // slow enough to actually affect top-level metrics like
if (ret) // Omnibox.CharTypedToRepaintLatency.
brand->assign(base::UTF16ToASCII(brand16)); static const base::NoDestructor<base::Optional<std::string>> brand_code(
return ret; []() -> base::Optional<std::string> {
base::string16 brand16;
if (!GoogleUpdateSettings::GetBrand(&brand16))
return base::nullopt;
return base::UTF16ToASCII(brand16);
}());
if (!brand_code->has_value())
return false;
brand->assign(**brand_code);
return true;
} }
bool GetReactivationBrand(std::string* brand) { bool GetReactivationBrand(std::string* brand) {
......
...@@ -18,6 +18,7 @@ namespace google_brand { ...@@ -18,6 +18,7 @@ namespace google_brand {
// Returns in |brand| the brand code or distribution tag that has been // Returns in |brand| the brand code or distribution tag that has been
// assigned to a partner. Returns false if the information is not available. // assigned to a partner. Returns false if the information is not available.
// TODO(asvitkine): These APIs should return base::Optional<std::string>.
bool GetBrand(std::string* brand); bool GetBrand(std::string* brand);
// Returns in |brand| the reactivation brand code or distribution tag // Returns in |brand| the reactivation brand code or distribution tag
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment