New issue
Advanced search Search tips

Issue 718242 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

generated devtools code might force unnecessary copies

Project Member Reported by dcheng@chromium.org, May 4 2017

Issue description

Right now, it looks like string setters take the string by value:

https://cs.chromium.org/chromium/src/out/Debug/gen/headless/public/devtools/domains/types_storage.h?rcl=3ea47128369ba885a1c235866a35be7213c176e2&l=29

  void SetOrigin(std::string value) { origin_ = value; }

If the caller is passing an object that's not an rvalue, this results in one copy, and a further copy when assigning it to origin_.
 
Owner: skyos...@chromium.org
Status: Assigned (was: Untriaged)
I believe this is specific to headless generator. Content uses const-refs:
https://cs.chromium.org/chromium/src/out/Debug/gen/content/browser/devtools/protocol/target.h?rcl=3ea47128369ba885a1c235866a35be7213c176e2&l=53
Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, May 4 2017

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

commit 273f3371dea38a8348b2d854c560142cfd1bb05c
Author: skyostil <skyostil@chromium.org>
Date: Thu May 04 14:28:57 2017

headless: Remove references to nonexistent headers

BUG= 718242 

Review-Url: https://codereview.chromium.org/2858043005
Cr-Commit-Position: refs/heads/master@{#469328}

[modify] https://crrev.com/273f3371dea38a8348b2d854c560142cfd1bb05c/headless/BUILD.gn

Project Member

Comment 5 by bugdroid1@chromium.org, May 4 2017

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

commit 1d15faaea01846f698bbd4082c582eb180a2a49e
Author: skyostil <skyostil@chromium.org>
Date: Thu May 04 16:10:32 2017

headless: Avoid copies for string parameters in DevTools bindings

String parameters were being passed by value instead of by const
reference by mistake.

BUG= 718242 

Review-Url: https://codereview.chromium.org/2862843002
Cr-Commit-Position: refs/heads/master@{#469350}

[modify] https://crrev.com/1d15faaea01846f698bbd4082c582eb180a2a49e/headless/lib/browser/devtools_api/client_api_generator.py
[modify] https://crrev.com/1d15faaea01846f698bbd4082c582eb180a2a49e/headless/lib/browser/devtools_api/client_api_generator_unittest.py

Status: Fixed (was: Started)

Sign in to add a comment