Commit 2627076a authored by tby's avatar tby Committed by Commit Bot

[Hashed logging] Make logging calls thread safe.

We want clients to be able to make logging calls from any sequence.
The data flow of the framework looks like this:

(client) -----> Recorder -----> Metrics Provider <----- UMA
         |               |                        |
         sends           sends                    grabs cached
         logging         log info                 log info
         calls                                    periodically

The UMA backend requests log info on the browser UI sequence, so we must
ensure all Recorder -> Metrics Provider calls also happen on the UI
sequence for thread safety.

This CL adds logic to the Recorder that checks which sequence a client
called on. If it is *not* the UI sequence, it re-calls itself on the UI
sequence. This changes the picture to:

                maybe reschedule
                on UI seq
                _______
                |     |
                |     V
(client) -----> Recorder -----> Metrics Provider <----- UMA
         |               |                        |
         any seq         UI seq                   UI seq

Bug: 1016655
Change-Id: I81d5168eef8b0ad138e09e89a28b6efca3c6c0d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1902179Reviewed-by: default avatarThanh Nguyen <thanhdng@chromium.org>
Commit-Queue: Tony Yeoman <tby@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713308}
parent fae4a99e
......@@ -10,12 +10,15 @@
#include <utility>
#include <vector>
#include "base/bind.h"
#include "base/callback_list.h"
#include "base/containers/flat_map.h"
#include "base/macros.h"
#include "base/no_destructor.h"
#include "base/observer_list.h"
#include "base/sequence_checker.h"
#include "base/task/post_task.h"
#include "content/public/browser/browser_thread.h"
#include "third_party/metrics_proto/chrome_os_app_list_launch_event.pb.h"
namespace app_list {
......@@ -70,6 +73,14 @@ class AppListLaunchRecorder {
static_assert(std::is_enum<T>::value,
"Non enum passed to AppListLaunchRecorder::Log");
if (!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) {
base::PostTask(
FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&AppListLaunchRecorder::Log<T>, base::Unretained(this),
client, hashed, unhashed));
return;
}
LaunchInfo event;
event.client = client;
for (const auto& pair : hashed)
......
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