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

Issue 834781 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 759794



Sign in to add a comment

Code Coverage report from Chrome browser binary

Project Member Reported by mmoroz@chromium.org, Apr 19 2018

Issue description

I guess users will be asking about that, so it's important to try that and out and document before announcing.
 

Comment 1 Deleted

Comment 2 by mmoroz@chromium.org, Apr 19 2018

(fixed some typos):

I've built the browser and used it for a little bit (opened a few news websites and YouTube), not am generating the reports.

Important information so far:
+ you can build it
+ linking takes a lot of time and RAM (1hr on my 56 cores with 128GB)
+ the binary is huge (5GB)
+ the browser functionality "works", but not really usable if you need to get some stuff done; most of things are slow / laggy
+ due to the point above, I wouldn't recommend building the whole browser as a component build. Likely that would be even slower, but actually worth to try at least once

Comment 3 by mmoroz@chromium.org, Apr 19 2018

One more important point:
+ you'll have to disable the sandbox, otherwise processes other than the browser will fail to write .profraw dumps


Comment 4 by mmoroz@chromium.org, Apr 19 2018

Here is the report: https://chrome-coverage.storage.googleapis.com/random_reports/chrome_549980/index.html

It doesn't look completely right to me, even though many parts of it look good.

My main concerns are v8 and media. I've been watching a video on youtube as well as had some JS executed. So, low numbers for those components seem to be wrong.

However, I haven't seen any failures during merge / generation process. Which means it's likely to be a build issue rather than a tooling problem.

---------

I'll create a FAQ section on the documentation page and cover everything I mentioned above.

Cc: sadrul@chromium.org
Owner: ----
Status: Available (was: Started)
Landing https://chromium-review.googlesource.com/c/chromium/src/+/1038149 and un-assigning myself to leave this issue available for further investigation.
Labels: -Coverage-v1-Blocker
Is not a blocker anymore as we at least have documented current behavior.
Project Member

Comment 7 by bugdroid1@chromium.org, May 2 2018

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

commit 63cd04daebb68a3d1e1c3d836c03eb11a73cb989
Author: Max Moroz <mmoroz@chromium.org>
Date: Wed May 02 16:40:23 2018

[Code Coverage] Add FAQ entry regarding use of the whole browser build.

Bug:  834781 
Change-Id: I6b21d7313c22e02e0454b36f2977ec11c1f8cfe0
Reviewed-on: https://chromium-review.googlesource.com/1038149
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Reviewed-by: Jonathan Metzman <metzman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555424}
[modify] https://crrev.com/63cd04daebb68a3d1e1c3d836c03eb11a73cb989/docs/code_coverage.md

Project Member

Comment 8 by bugdroid1@chromium.org, May 4 2018

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

commit 5aa6275b4f8a9668c1376e7ffc36bc496d41f6f5
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Fri May 04 23:07:49 2018

code coverage: Fix writing profiles from child processes.

Notable changes:
. Introduce CLANG_COVERAGE define if use_clang_coverage gn arg is set.
. For child processes to be able to correctly write the coverage profile
  during teardown, it is necessary to allow them to shut-down cleanly.
  So avoid terminating the processes from ~ChildProcessLauncher() when
  CLANG_COVERAGE is set.
. Child processes can do fast-shutdown by calling _exit(0). So force the
  profile file to be written from the child process before calling that.

With both changes, it is still necessary to run with --no-sandbox, since
otherwise the child processes are not able to write to the profile file.

BUG=838582,  834781 

Change-Id: I742521e756a7dead983f40462798f7c4dac2ac02
Reviewed-on: https://chromium-review.googlesource.com/1041271
Reviewed-by: Max Moroz <mmoroz@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556236}
[modify] https://crrev.com/5aa6275b4f8a9668c1376e7ffc36bc496d41f6f5/build/config/BUILD.gn
[modify] https://crrev.com/5aa6275b4f8a9668c1376e7ffc36bc496d41f6f5/content/browser/child_process_launcher.cc
[modify] https://crrev.com/5aa6275b4f8a9668c1376e7ffc36bc496d41f6f5/content/child/child_thread_impl.cc

Cc: -sadrul@chromium.org
Owner: sadrul@chromium.org
Status: Assigned (was: Available)
Sadrul@, can you please verify if things work fine, and then mark as Fixed.
Does the patch from c#8 also help to get coverage reported from V8?
Status: Fixed (was: Assigned)
It does seem to include coverage for V8 code too, yep.

Should I update the doc to remove the entry about incomplete coverage report for child-processes?
That would be perfect, Sadul! And thanks again for the patch!
I meant "Sadrul", sorry about the typo :)

Comment 14 by aarya@google.com, May 16 2018

Thanks a lot.

We have https://bugs.chromium.org/p/chromium/issues/detail?id=842424 on the need for --no-sandbox. Curious if you want to look at that one too :) ? Since many tests run with sandbox enabled, we lose coverage with disabling sandbox.
Sadrul, I was removing something else from FAQ and removed the browser-related note as well: https://chromium-review.googlesource.com/c/chromium/src/+/1063995

Project Member

Comment 16 by bugdroid1@chromium.org, May 17 2018

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

commit 6b33c6d64fbb9582f5f25086e3475c31ef30f484
Author: Max Moroz <mmoroz@chromium.org>
Date: Thu May 17 14:04:21 2018

[Code Coverage] Remove obsolete points from the FAQ list:

- Entry regarding coverage missing for some *_fuzzer.cc files.
- Note regarding coverage from child processes from the browser.

TBR=inferno@chromium.org

Bug:  821617 , 834781 
Change-Id: I69a645e3b311ac9c160d878afd518706b3a000fe
Reviewed-on: https://chromium-review.googlesource.com/1063995
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Reviewed-by: Max Moroz <mmoroz@chromium.org>
Reviewed-by: Abhishek Arya <inferno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559516}
[modify] https://crrev.com/6b33c6d64fbb9582f5f25086e3475c31ef30f484/docs/code_coverage.md

Thanks Max! :)

I have left a comment on https://bugs.chromium.org/p/chromium/issues/detail?id=842424#c12

Sign in to add a comment