New issue
Advanced search Search tips

Issue 712308 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

SurfaceManager: consider a less verbose spew of LocalSurfaceId

Project Member Reported by fsam...@chromium.org, Apr 17 2017

Issue description

cc::LocalSurfaceIds hold a base::UnguessableToken for security purposes so that an untrusted client cannot embed the Chrome UI and intercept input by overlaying a transparent div for example. 

However, from a debugging perspective, the token is a meaningless set of bits. The LocalSurfaceId also has a local_id that is expected to be allocated by a LocalSurfaceIds store and be unique and monotonically increasing for a given fixed FrameSinkId. 

This, from a debugging perspective, we only care to see the local_id. We should only display that by default in LocalSurfaceId::ToString() unless we select more verbose spew. 

 

Comment 1 by danakj@chromium.org, Apr 18 2017

The bits are meaningless but not for comparison to other sets of bits. Could it be mapped into a number that is printed instead?
perhaps gratuitous: maybe each surface id "pretty-printed name" (like for tracing) could be the client name, size, sequence #. 
Labels: Type-Feature
Cc: weiliangc@chromium.org
Cc: varkha@chromium.org
Blocking: -601863
Components: -Internals>MUS

Comment 8 by fsamuel@google.com, Oct 24 2017

Owner: samans@chromium.org
Cc: yiyix@google.com
Cc: -yiyix@google.com yiyix@chromium.org
I think it's better to print the nonce like AB..CD instead of not printing at all.
Cc: -yiyix@chromium.org
Owner: yiyix@chromium.org
Enable the 2 different format: 
If enable_logging then shows the complete LocalSufaceId; otherwise shows the shorter version.

info on enable logging: https://www.chromium.org/for-testers/enable-logging

example of usage: https://cs.chromium.org/chromium/src/base/i18n/build_utf8_validator_tables.cc?type=cs&sq=package:chromium&l=455
Project Member

Comment 14 by bugdroid1@chromium.org, Feb 8 2018

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

commit 7a0d0edfa21d531698de1ba2af53ba456bb1f627
Author: Illia Martyniuk <illiam@google.com>
Date: Thu Feb 08 01:38:14 2018

Updating Local_Surface_Id::ToString()

If logging level is less that 1 (not verbose) we print LocalSurfaceId's corresponding
UnguessableToken as ABCD...; otherwise, we print full 16-character UnguessableToken.

Added TEST=LocalSurfaceIdTest.VerifyToString

Bug:  712308 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I577b15f23fc50643a8db4c555bdd95dcd0057755
Reviewed-on: https://chromium-review.googlesource.com/904855
Commit-Queue: Illia Martyniuk <illiam@google.com>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Yi Xu <yiyix@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535241}
[modify] https://crrev.com/7a0d0edfa21d531698de1ba2af53ba456bb1f627/components/viz/common/BUILD.gn
[modify] https://crrev.com/7a0d0edfa21d531698de1ba2af53ba456bb1f627/components/viz/common/surfaces/local_surface_id.cc
[add] https://crrev.com/7a0d0edfa21d531698de1ba2af53ba456bb1f627/components/viz/common/surfaces/local_surface_id_unittest.cc

Comment 15 by yiyix@chromium.org, Feb 21 2018

Owner: ----
Status: Fixed (was: Assigned)
@illiam has fixed it 

Sign in to add a comment