New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 792730 link

Starred by 1 user
Status: Fixed
Owner:
s...@chromium.org
Closed: Dec 2017
Cc:
█ markusheintz@chromium.org
dullweber@chromium.org
mastiz@chromium.org
█ tschumann@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[Sync] sync-internals Test UserEvent no longer works.

Project Member Reported by s...@chromium.org, Dec 7 2017 
Since https://chromium-review.googlesource.com/c/chromium/src/+/764408 landed, it has not worked. This is because it never sets a event payload, which means its event_case() resolves to UserEventSpecifics::EVENT_NOT_SET and it gets a false from UserEventServiceImpl::ShouldRecordEvent().

While we're here, we should also allow to differentiate from the scenario of no navigation id, and a navigation id of 0, since UserEventServiceImpl::ShouldRecordEvent() also cares about the presence of navigation_id.
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4920476d9360e73d4b98d98c9fe29dbf48833dc8

commit 4920476d9360e73d4b98d98c9fe29dbf48833dc8
Author: Sky Malice <skym@chromium.org>
Date: Thu Dec 07 01:50:33 2017

[Sync] Fix test UserEvent creation.

Due to the recent rework of how to decide if events should be recorded
or not, the test events created through sync-internals was not updated.
This fixes them by making two changes.

1. event_case() is now correctly set so that the service can identify
the type of event.

2. navigation_id's presence is not set if the text field is not filled
in, allowing it to bypass the requirement for history sync to be
enabled.

Bug:  792730 
Change-Id: Icabf100da886cbd28efc8db2ce3e6e8cb4a50af1
Reviewed-on: https://chromium-review.googlesource.com/812975
Commit-Queue: Sky Malice <skym@chromium.org>
Reviewed-by: Patrick Noland <pnoland@google.com>
Cr-Commit-Position: refs/heads/master@{#522298}
[modify] https://crrev.com/4920476d9360e73d4b98d98c9fe29dbf48833dc8/chrome/browser/ui/webui/sync_internals_message_handler.cc
[modify] https://crrev.com/4920476d9360e73d4b98d98c9fe29dbf48833dc8/chrome/browser/ui/webui/sync_internals_message_handler_unittest.cc

Comment 2 by s...@chromium.org, Dec 7 2017

Status: Fixed (was: Started)
►

Sign in to add a comment