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

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/958012Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542506}
parent 300288db
...@@ -8,8 +8,6 @@ ...@@ -8,8 +8,6 @@
#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"
...@@ -43,20 +41,11 @@ bool GetBrand(std::string* brand) { ...@@ -43,20 +41,11 @@ bool GetBrand(std::string* brand) {
return true; return true;
} }
// Cache brand code value, since it is queried a lot and registry queries are base::string16 brand16;
// slow enough to actually affect top-level metrics like bool ret = GoogleUpdateSettings::GetBrand(&brand16);
// Omnibox.CharTypedToRepaintLatency. if (ret)
static const base::NoDestructor<base::Optional<std::string>> brand_code( brand->assign(base::UTF16ToASCII(brand16));
[]() -> base::Optional<std::string> { return ret;
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,7 +18,6 @@ namespace google_brand { ...@@ -18,7 +18,6 @@ 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