Issue 612843 link

Starred by 1 user

Issue metadata

Status:	Fixed
Owner:	gab@chromium.org
Closed:	Jun 2016
Cc:	danakj@chromium.org etienneb@chromium.org thestig@chromium.org sky@chromium.org fdoray@chromium.org thakis@chromium.org mark@chromium.org
Components:	Internals
EstimatedDays:	----
NextAction:	----
OS:	All
Pri:	3
Type:	Bug

Change WaitableEvent's constructor to be enum-based.

Project Member Reported by gab@chromium.org, May 18 2016

Issue description

Everytime I see a call to WaitableEvent(bool, bool), I have to lookup waitable_event.h to remember the order of the bools...

How about we change it to:

class WaitableEvent { 

  enum class Type {
    MANUAL_INITIALLY_SIGNALED,
    MANUAL_NOT_SIGNALED,
    AUTOMATIC_INITIALLY_SIGNALED,
    AUTOMATIC_NOT_SIGNALED,
  }

  WaitableEvent(Type type);

};

I can see how to make this happen once we reach consensus on what we want (@ base owners to weigh in).

Comment 1 by thakis@chromium.org, May 18 2016

lgtm

Comment 2 by thakis@chromium.org, May 18 2016

But:
- why enum class, if it's nested in WaitableEvent already? How does this buy anything over a regular enum?
- if you don't make it nested, it could be forward-declared

(so some bikesheddng comments, but "use enum instead of bool" sounds great)

Comment 3 by gab@chromium.org, May 18 2016

1) I just tend to err towards enum class for everything that doesn't require the underlying numerical value these days.

2) True, but do we care? This should only be used by people calling WaitableEvent's constructor which should already be including waitable_event.h and thus have this definition (I'd be surprised to see a user of WaitableEvent take its Type as a parameter from another caller -- e.g., a fwd-decl would be useful in a call chain where callees merely pass it down but I don't see this as a pattern for WaitableEvent)

Comment 4 by thakis@chromium.org, May 18 2016

Ok, fair enough. I usually feel the additional typing most of the time isn't worth the can't-convert-to-int-ness, but I also try not to argue about nits like this with people actually doing the work :-)

Comment 5 by mark@chromium.org, May 18 2016

Might want to keep it as two arguments and have an enum class for each, but I don’t feel strongly.

Comment 6 by danakj@chromium.org, May 18 2016

2 enums sounds nice, but ya I hate those bools.

Comment 7 by gab@chromium.org, May 25 2016

One advantage of single-enum is not having to repeat the base::WaitableEvent::Type:: namespace twice I guess?

A typical call could be:

waitable_event_(base::WaitableEvent::Type::MANUAL_INITIALLY_SIGNALED)

instead of:

waitable_event_(base::WaitableEvent::WaitControl::MANUAL, base::WaitableEvent::InitiallySignaled::YES)

or something (also not sure how to name the enums if split in two which are essentially bools with names for true/false).

(Note: I'm open to using an "enum" instead of "enum class" to drop the extra namespace in either case)

WDYT?

Comment 8 by fdoray@chromium.org, May 25 2016

Possible names of we have 2 enums:
enum class InitialState { SIGNALED, NOT_SIGNALED }
enum class Reset { MANUAL, AUTOMATIC }

Comment 9 by gab@chromium.org, May 25 2016

That SGTM (if we go with 2 enums), so:

waitable_event_(base::WaitableEvent::Type::MANUAL_INITIALLY_SIGNALED)

or

waitable_event_(base::WaitableEvent::InitialState::SIGNALED,
                base::WaitableEvent::Reset::MANUAL)

?

Comment 10 by danakj@chromium.org, May 25 2016

Drop the enum class if it's tested in a class already IMO.

Comment 11 by danakj@chromium.org, May 25 2016

Well, I guess then you'd maybe want that context in the enum values. Your 2nd proposal in #9 sgtm.

Comment 12 by gab@chromium.org, May 30 2016

CL for proposed API up @ https://codereview.chromium.org/2020043002/

Comment 13 by gab@chromium.org, May 30 2016

Cc: etienneb@chromium.org

Follow-up migration CL is up at https://codereview.chromium.org/2024723002/ (it's too big and will be split in sub-components to land but is up for reference).

It was automated using clang-tidy on Linux with the help of etienneb@ and fdoray@ (ref. these notes: https://docs.google.com/document/d/1BOibvtTGZetCJUPqPNBzqLMlykM9EIpVZJPtkqmSi40/edit) followed by "git cl format".

matcher/checker code:

==========================================================================================
void WeventCheck::registerMatchers(MatchFinder *finder) {
  const auto waitable_event_expr =
      cxxConstructExpr(hasDeclaration(namedDecl(hasName("WaitableEvent"))),
          hasArgument(0, cxxBoolLiteral().bind("is_manual")),
          hasArgument(1, cxxBoolLiteral().bind("is_signaled"))).bind("call");

  finder->addMatcher(
      cxxConstructExpr(waitable_event_expr,
          hasAncestor(namespaceDecl(hasName("base")).bind("is_base"))),
      this);

  finder->addMatcher(
      cxxConstructExpr(waitable_event_expr,
          unless(hasAncestor(namespaceDecl(hasName("base"))))),
      this);
}

void WeventCheck::check(const MatchFinder::MatchResult &result) {
  const auto *is_manual_expr =
      result.Nodes.getNodeAs<CXXBoolLiteralExpr>("is_manual");
  const auto *is_signaled_expr =
      result.Nodes.getNodeAs<CXXBoolLiteralExpr>("is_signaled");

  const auto *is_base =
      result.Nodes.getNodeAs<NamedDecl>("is_base");

  const auto *call =
      result.Nodes.getNodeAs<CXXConstructExpr>("call");

  std::string is_manual_param = is_base ? "" : "base::";
  is_manual_param.append("WaitableEvent::ResetPolicy::");
  is_manual_param.append(is_manual_expr->getValue() ? "MANUAL" : "AUTOMATIC");

  std::string is_signaled_param = is_base ? "" : "base::";
  is_signaled_param.append("WaitableEvent::InitialState::");
  is_signaled_param.append(
      is_signaled_expr->getValue() ? "SIGNALED" : "NOT_SIGNALED");

  diag(call->getLocStart(), "Found boolean WaitableEvent usage")
      << FixItHint::CreateReplacement(
             call->getParenOrBraceRange(),
             "(" + is_manual_param + ", " + is_signaled_param + ")");
}
==========================================================================================


The generated CL will be split and uploaded in individual components with a technique similar to the one I described @  https://crbug.com/610438#c25 

(stragglers (e.g. not compiled on Linux or not fixable our simple clang-tidy script) will be migrated manually in a final pass)

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 1 2016

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

commit e5ff4d6d4f34c5a2c9b1ac5514f94d88912d4135
Author: gab <gab@chromium.org>
Date: Wed Jun 01 00:16:50 2016

Make WaitableEvent's constructor enum-based.

Cleanup CLs for existing code will follow and conclude
with removal of the boolean based constructor :-)

BUG= 612843 

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

[modify] https://crrev.com/e5ff4d6d4f34c5a2c9b1ac5514f94d88912d4135/base/synchronization/waitable_event.h
[modify] https://crrev.com/e5ff4d6d4f34c5a2c9b1ac5514f94d88912d4135/base/synchronization/waitable_event_posix.cc
[modify] https://crrev.com/e5ff4d6d4f34c5a2c9b1ac5514f94d88912d4135/base/synchronization/waitable_event_win.cc

Comment 15 by gab@chromium.org, Jun 1 2016

Component issues sent for review (tractable @ https://codereview.chromium.org/user/gab@chromium.org).

For reference, here was my methodology for mass splitting and uploading CLs:

1) Start with branch WEvent_enums with API update and WEvent_all with clang-tidy result (i.e. https://codereview.chromium.org/2024723002/).

2) git diff --stat WEvent_enums..WEvent_all > WEventfiles.txt

3) Open WEventfiles.txt in favorite editor and mass edit to only have one entry per top-level component (very easy with Sublime Text's multi-edit + "permute unique" feature).

Get:

base
cc
chrome
components
content
dbus
gpu
ipc
jingle
media
net
ppapi
remoting
services
storage
sync
tools
ui

4) Multi-edit this into a command like:
less base/OWNERS && less cc/OWNERS && less chrome/OWNERS && less components/OWNERS && less content/OWNERS && less dbus/OWNERS && less gpu/OWNERS && less ipc/OWNERS && less jingle/OWNERS && less media/OWNERS && less net/OWNERS && less ppapi/OWNERS && less remoting/OWNERS && less services/OWNERS && less storage/OWNERS && less sync/OWNERS && less tools/OWNERS && less ui/OWNERS

5) Run in terminal, pick an owner for each component and paste it in original list, e.g.:

base danakj@chromium.org
cc enne@chromium.org
chrome thakis@chromium.org
components caitkp@chromium.org
content avi@chromium.org
dbus hashimoto@chromium.org
gpu sievers@chromium.org
ipc jeremy@chromium.org
jingle zea@chromium.org
media jrummell@chromium.org
net pauljensen@chromium.org
ppapi bbudge@chromium.org
remoting joedow@chromium.org
services rockot@chromium.org
storage michaeln@chromium.org
sync stanisc@chromium.org
tools scottmg@chromium.org
ui sky@chromium.org

6) Multi-edit this into:

git co -b base WEvent_all && git reset --soft WEvent_enums && git reset && git add base/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in base/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=danakj@chromium.org --send-mail && git cl try &&
git co -b cc WEvent_all && git reset --soft WEvent_enums && git reset && git add cc/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in cc/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=enne@chromium.org --send-mail && git cl try &&
git co -b chrome WEvent_all && git reset --soft WEvent_enums && git reset && git add chrome/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in chrome/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=thakis@chromium.org --send-mail && git cl try &&
git co -b components WEvent_all && git reset --soft WEvent_enums && git reset && git add components/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in components/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=caitkp@chromium.org --send-mail && git cl try &&
git co -b content WEvent_all && git reset --soft WEvent_enums && git reset && git add content/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in content/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=avi@chromium.org --send-mail && git cl try &&
git co -b dbus WEvent_all && git reset --soft WEvent_enums && git reset && git add dbus/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in dbus/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=hashimoto@chromium.org --send-mail && git cl try &&
git co -b gpu WEvent_all && git reset --soft WEvent_enums && git reset && git add gpu/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in gpu/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=sievers@chromium.org --send-mail && git cl try &&
git co -b ipc WEvent_all && git reset --soft WEvent_enums && git reset && git add ipc/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in ipc/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=jeremy@chromium.org --send-mail && git cl try &&
git co -b jingle WEvent_all && git reset --soft WEvent_enums && git reset && git add jingle/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in jingle/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=zea@chromium.org --send-mail && git cl try &&
git co -b media WEvent_all && git reset --soft WEvent_enums && git reset && git add media/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in media/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=jrummell@chromium.org --send-mail && git cl try &&
git co -b net WEvent_all && git reset --soft WEvent_enums && git reset && git add net/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in net/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=pauljensen@chromium.org --send-mail && git cl try &&
git co -b ppapi WEvent_all && git reset --soft WEvent_enums && git reset && git add ppapi/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in ppapi/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=bbudge@chromium.org --send-mail && git cl try &&
git co -b remoting WEvent_all && git reset --soft WEvent_enums && git reset && git add remoting/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in remoting/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=joedow@chromium.org --send-mail && git cl try &&
git co -b services WEvent_all && git reset --soft WEvent_enums && git reset && git add services/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in services/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=rockot@chromium.org --send-mail && git cl try &&
git co -b storage WEvent_all && git reset --soft WEvent_enums && git reset && git add storage/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in storage/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=michaeln@chromium.org --send-mail && git cl try &&
git co -b sync WEvent_all && git reset --soft WEvent_enums && git reset && git add sync/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in sync/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=stanisc@chromium.org --send-mail && git cl try &&
git co -b tools WEvent_all && git reset --soft WEvent_enums && git reset && git add tools/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in tools/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=scottmg@chromium.org --send-mail && git cl try &&
git co -b ui WEvent_all && git reset --soft WEvent_enums && git reset && git add ui/ && git commit -m $'Migrate WaitableEvent to enum-based constructor in ui/\n\nBUG=612843' && git reset --hard HEAD && git br -u WEvent_enums && git clean -f && git cl upload -f --reviewers=sky@chromium.org --send-mail && git cl try &&
echo DONE

Explained:
 a) git co -b foo WEvent_all (create branch for component foo starting from full change)
 b) git reset --soft WEvent_enums (drop history of full change)
 c) git reset (unstage)
 d) git add foo/ (stage only changes for foo)
 e) git commit -m $'foo\n\nBUG=123456' (special trick for multi-line commit including BUG# from Bash shell)
 f) git reset --hard HEAD (get rid of everything else on that branch)
 g) git br -u WEvent_enums (set the API change as the upstream)
 h) git cl upload -f --reviewers=gab@chromium.org --send-mail (upload w/ reviewer (from multi-edit in (5) and send-mail without interactive prompt)
 i) git cl try (get a head start on try jobs :-))

7) Visit your https://codereview.chromium.org/ and make sure there are no assigned reviewers with an OOO message :-).

8) Wait for reviews, potentially edit local branches if reviewers spot something, and CQ.

Woot :-)!

Cheers,
Gab

Comment 16 by gab@chromium.org, Jun 1 2016

Oh and if you decide you want to mass-edit your CL descriptions like I did (wanted to mention this was automated by clang-tidy), you can do something like:

git co base && git cl description -n $'Migrate WaitableEvent to enum-based constructor in base/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co cc && git cl description -n $'Migrate WaitableEvent to enum-based constructor in cc/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co chrome && git cl description -n $'Migrate WaitableEvent to enum-based constructor in chrome/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co components && git cl description -n $'Migrate WaitableEvent to enum-based constructor in components/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co content && git cl description -n $'Migrate WaitableEvent to enum-based constructor in content/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co dbus && git cl description -n $'Migrate WaitableEvent to enum-based constructor in dbus/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co gpu && git cl description -n $'Migrate WaitableEvent to enum-based constructor in gpu/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co ipc && git cl description -n $'Migrate WaitableEvent to enum-based constructor in ipc/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co jingle && git cl description -n $'Migrate WaitableEvent to enum-based constructor in jingle/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co media && git cl description -n $'Migrate WaitableEvent to enum-based constructor in media/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co net && git cl description -n $'Migrate WaitableEvent to enum-based constructor in net/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co ppapi && git cl description -n $'Migrate WaitableEvent to enum-based constructor in ppapi/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co remoting && git cl description -n $'Migrate WaitableEvent to enum-based constructor in remoting/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co services && git cl description -n $'Migrate WaitableEvent to enum-based constructor in services/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co storage && git cl description -n $'Migrate WaitableEvent to enum-based constructor in storage/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co sync && git cl description -n $'Migrate WaitableEvent to enum-based constructor in sync/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co tools && git cl description -n $'Migrate WaitableEvent to enum-based constructor in tools/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
git co ui && git cl description -n $'Migrate WaitableEvent to enum-based constructor in ui/\n\nChange automated with clang-tidy (details @  https://crbug.com/612843#c13 )\n\nBUG=612843' &&
echo DONE

:-)

Comment 17 by gab@chromium.org, Jun 1 2016

Cc: sky@chromium.org

From sky @ 
https://codereview.chromium.org/2027323002/diff/1/ui/views/mus/views_mus_test_suite.cc#newcode70

"""
Did you consider making WaitableEvent take a bitmask of possible configurations
rather than two explicit policies? I suspect automatic and not-signaled are most
commonly used, so this code would become:

base::WaitableEvent event(base::WaitableEvent::DEFAULT);

Even better is to with a default value (which the style guide now allows) for
DEFAULT, so that this code becomes:

base::WaitableEvent event;

If you want to create manual, then it's:

event(base::WaitableEvent::MANUAL);

and if you want manual and signaled:

event(base::WaitableEvent::MANUAL | base::WaitableEvent::SIGNALED);

IMO this better matches how WaitableEvent is generally used and makes the api
nicer.
"""

@danakj/thakis: What do you guys think? The ball is already rolling on this change but we could consider this.

Comment 18 by danakj@chromium.org, Jun 1 2016

I like bitflags here a bit less, because you can combine MANUAL | AUTOMATIC also, but these are not actually combinable. Bitflags make sense when everything in the set can be combined.

Project Member

Comment 19 by bugdroid1@chromium.org, Jun 1 2016

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

commit c96c5009a7e8712329d6407bf8d9022f88eaf186
Author: gab <gab@chromium.org>
Date: Wed Jun 01 19:56:52 2016

Migrate WaitableEvent to enum-based constructor in remoting/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/c96c5009a7e8712329d6407bf8d9022f88eaf186/remoting/base/auto_thread.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Jun 1 2016

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

commit f789bc6d984264de917571e7f03bbf7a6c472c30
Author: gab <gab@chromium.org>
Date: Wed Jun 01 20:01:11 2016

Migrate WaitableEvent to enum-based constructor in services/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/f789bc6d984264de917571e7f03bbf7a6c472c30/services/shell/background/background_shell.cc
[modify] https://crrev.com/f789bc6d984264de917571e7f03bbf7a6c472c30/services/shell/runner/host/child_process_base.cc
[modify] https://crrev.com/f789bc6d984264de917571e7f03bbf7a6c472c30/services/shell/runner/host/child_process_host.cc
[modify] https://crrev.com/f789bc6d984264de917571e7f03bbf7a6c472c30/services/shell/standalone/tracer.cc

Comment 21 by sky@chromium.org, Jun 1 2016

> I like bitflags here a bit less, because you can combine MANUAL | AUTOMATIC also

With my suggestion there would only be bitmask values for the non-default values. In other words:

enum class Configuration {
  DEFAULT = 0,
  MANUAL_RESET = 1 << 1,
  INITIALLY_SIGNALED = 1 << 2,
};

DEFAULT automatic reset and not initially signaled. The description would clarify this.

Project Member

Comment 22 by bugdroid1@chromium.org, Jun 1 2016

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

commit ece8fb59f3b2f52325a331323ffc869c81d99b0c
Author: gab <gab@chromium.org>
Date: Wed Jun 01 20:09:39 2016

Migrate WaitableEvent to enum-based constructor in cc/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/ece8fb59f3b2f52325a331323ffc869c81d99b0c/cc/trees/layer_tree_host_unittest.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Jun 1 2016

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

commit f40c0a5e0ccac827a449c314e920dcb9bfb7305c
Author: gab <gab@chromium.org>
Date: Wed Jun 01 20:10:46 2016

Migrate WaitableEvent to enum-based constructor in ppapi/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/f40c0a5e0ccac827a449c314e920dcb9bfb7305c/ppapi/host/resource_message_filter_unittest.cc
[modify] https://crrev.com/f40c0a5e0ccac827a449c314e920dcb9bfb7305c/ppapi/proxy/ppapi_proxy_test.cc
[modify] https://crrev.com/f40c0a5e0ccac827a449c314e920dcb9bfb7305c/ppapi/proxy/ppp_instance_proxy_unittest.cc
[modify] https://crrev.com/f40c0a5e0ccac827a449c314e920dcb9bfb7305c/ppapi/proxy/ppp_messaging_proxy_perftest.cc
[modify] https://crrev.com/f40c0a5e0ccac827a449c314e920dcb9bfb7305c/ppapi/proxy/ppp_messaging_proxy_unittest.cc
[modify] https://crrev.com/f40c0a5e0ccac827a449c314e920dcb9bfb7305c/ppapi/proxy/tracked_callback_unittest.cc
[modify] https://crrev.com/f40c0a5e0ccac827a449c314e920dcb9bfb7305c/ppapi/proxy/video_encoder_resource_unittest.cc

Comment 24 by danakj@chromium.org, Jun 1 2016

> DEFAULT automatic reset and not initially signaled. The description would clarify this.

Ah I see, that'd be okay then.

I don't have a strong preference either way. While the enums are verbose they also document if it's signalled or not initially at the callsite always. So, pluses and minuses. Anything is better than bools. :) So, up to you gab I think.

Project Member

Comment 25 by bugdroid1@chromium.org, Jun 1 2016

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

commit 3dbb1f62a6fbc115a027294b982db68d3d21012f
Author: gab <gab@chromium.org>
Date: Wed Jun 01 20:17:35 2016

Migrate WaitableEvent to enum-based constructor in tools/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/3dbb1f62a6fbc115a027294b982db68d3d21012f/tools/battor_agent/battor_agent_bin.cc
[modify] https://crrev.com/3dbb1f62a6fbc115a027294b982db68d3d21012f/tools/gn/input_file_manager.cc

Comment 26 by gab@chromium.org, Jun 1 2016

>> DEFAULT automatic reset and not initially signaled. The description would clarify this.

> Ah I see, that'd be okay then.

> I don't have a strong preference either way. While the enums are verbose they also document if it's signalled or not initially at the callsite always. So, pluses and minuses. Anything is better than bools. :) So, up to you gab I think.

The main goal of getting rid of the bools was to not have to check the header every time one reads one of those callsites. Is remembering the default just as hard? Verbosity has the advantage of being very clear (and making sure developers make a conscious explicit decision).

Project Member

Comment 27 by bugdroid1@chromium.org, Jun 1 2016

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

commit 90c2c5c5e952b2e67eb329bb849d4288cede4a42
Author: gab <gab@chromium.org>
Date: Wed Jun 01 20:34:06 2016

Migrate WaitableEvent to enum-based constructor in ipc/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/90c2c5c5e952b2e67eb329bb849d4288cede4a42/ipc/ipc_send_fds_test.cc
[modify] https://crrev.com/90c2c5c5e952b2e67eb329bb849d4288cede4a42/ipc/ipc_sync_channel.cc
[modify] https://crrev.com/90c2c5c5e952b2e67eb329bb849d4288cede4a42/ipc/ipc_sync_channel_unittest.cc
[modify] https://crrev.com/90c2c5c5e952b2e67eb329bb849d4288cede4a42/ipc/ipc_sync_message.cc
[modify] https://crrev.com/90c2c5c5e952b2e67eb329bb849d4288cede4a42/ipc/ipc_sync_message_filter.cc
[modify] https://crrev.com/90c2c5c5e952b2e67eb329bb849d4288cede4a42/ipc/unix_domain_socket_util_unittest.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Jun 1 2016

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

commit 9b4e2a0a5cf27c6c8097f704da34e4e6eb064dd6
Author: gab <gab@chromium.org>
Date: Wed Jun 01 20:37:08 2016

Migrate WaitableEvent to enum-based constructor in jingle/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/9b4e2a0a5cf27c6c8097f704da34e4e6eb064dd6/jingle/glue/thread_wrapper.cc
[modify] https://crrev.com/9b4e2a0a5cf27c6c8097f704da34e4e6eb064dd6/jingle/glue/thread_wrapper_unittest.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Jun 1 2016

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

commit 560392b6d278bf932ba30f8130902a4292d88e7b
Author: gab <gab@chromium.org>
Date: Wed Jun 01 20:48:42 2016

Migrate WaitableEvent to enum-based constructor in sync/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/560392b6d278bf932ba30f8130902a4292d88e7b/sync/internal_api/attachments/attachment_service_proxy_unittest.cc
[modify] https://crrev.com/560392b6d278bf932ba30f8130902a4292d88e7b/sync/internal_api/http_bridge.cc
[modify] https://crrev.com/560392b6d278bf932ba30f8130902a4292d88e7b/sync/internal_api/http_bridge_unittest.cc
[modify] https://crrev.com/560392b6d278bf932ba30f8130902a4292d88e7b/sync/internal_api/public/base/cancelation_signal_unittest.cc
[modify] https://crrev.com/560392b6d278bf932ba30f8130902a4292d88e7b/sync/internal_api/public/engine/model_safe_worker.cc
[modify] https://crrev.com/560392b6d278bf932ba30f8130902a4292d88e7b/sync/internal_api/syncapi_server_connection_manager_unittest.cc
[modify] https://crrev.com/560392b6d278bf932ba30f8130902a4292d88e7b/sync/test/fake_server/fake_server_http_post_provider.cc

Comment 30 by sky@chromium.org, Jun 1 2016

> The main goal of getting rid of the bools was to not have to check the header every time one reads one of those callsites. Is remembering the default just as hard? Verbosity has the advantage of being very clear (and making sure developers make a conscious explicit decision).

Ok, fair enough.

Project Member

Comment 31 by bugdroid1@chromium.org, Jun 1 2016

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

commit a0c6af14179a2ab33286a941997162b0d67f7e37
Author: gab <gab@chromium.org>
Date: Wed Jun 01 21:03:34 2016

Migrate WaitableEvent to enum-based constructor in ui/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/a0c6af14179a2ab33286a941997162b0d67f7e37/ui/views/mus/views_mus_test_suite.cc

Project Member

Comment 32 by bugdroid1@chromium.org, Jun 1 2016

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

commit 75d723398f481831c2b94325b2bea82c6e03bd56
Author: gab <gab@chromium.org>
Date: Wed Jun 01 21:15:33 2016

Migrate WaitableEvent to enum-based constructor in base/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/allocator/allocator_shim_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/files/file_path_watcher_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/memory/weak_ptr_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/message_loop/message_loop_task_runner_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/message_loop/message_pump_default.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/message_loop/message_pump_libevent_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/posix/unix_domain_socket_linux_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/profiler/stack_sampling_profiler.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/profiler/stack_sampling_profiler_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/synchronization/waitable_event_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/synchronization/waitable_event_watcher_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/task_scheduler/priority_queue_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/task_scheduler/scheduler_lock_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/task_scheduler/scheduler_service_thread_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/task_scheduler/scheduler_thread_pool_impl.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/task_scheduler/scheduler_thread_pool_impl_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/task_scheduler/scheduler_worker_thread.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/task_scheduler/scheduler_worker_thread_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/task_scheduler/task_scheduler_impl.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/task_scheduler/task_scheduler_impl_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/task_scheduler/task_tracker_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/test/test_io_thread.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/test/thread_test_helper.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/test/trace_event_analyzer_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/threading/platform_thread_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/threading/sequenced_task_runner_handle_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/threading/sequenced_worker_pool_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/threading/simple_thread.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/threading/simple_thread_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/threading/thread.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/threading/thread_local_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/threading/thread_perftest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/threading/thread_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/threading/worker_pool_posix_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/threading/worker_pool_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/trace_event/memory_dump_manager_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/trace_event/trace_event_unittest.cc
[modify] https://crrev.com/75d723398f481831c2b94325b2bea82c6e03bd56/base/trace_event/trace_sampling_thread.cc

Project Member

Comment 33 by bugdroid1@chromium.org, Jun 1 2016

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

commit 44ecd8f624c8e53411a28c5e6725402e40fc0faf
Author: gab <gab@chromium.org>
Date: Wed Jun 01 21:30:09 2016

Migrate WaitableEvent enum-based constructor in templated EventPerfTest

BUG= 612843 
R=danakj@chromium.org
NO_DEPENDENCY_CHECKS=true

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

[modify] https://crrev.com/44ecd8f624c8e53411a28c5e6725402e40fc0faf/base/threading/thread_perftest.cc

Project Member

Comment 34 by bugdroid1@chromium.org, Jun 2 2016

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

commit ca50ffb495174b15c60ecdd5a2403b98d0d43c64
Author: gab <gab@chromium.org>
Date: Wed Jun 01 23:59:04 2016

Migrate WaitableEvent to enum-based constructor in gpu/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/ca50ffb495174b15c60ecdd5a2403b98d0d43c64/gpu/command_buffer/common/gles2_cmd_format_test.cc
[modify] https://crrev.com/ca50ffb495174b15c60ecdd5a2403b98d0d43c64/gpu/command_buffer/service/in_process_command_buffer.cc
[modify] https://crrev.com/ca50ffb495174b15c60ecdd5a2403b98d0d43c64/gpu/command_buffer/tests/egl_test.cc

Project Member

Comment 35 by bugdroid1@chromium.org, Jun 2 2016

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

commit 0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38
Author: gab <gab@chromium.org>
Date: Thu Jun 02 00:00:23 2016

Migrate WaitableEvent to enum-based constructor in media/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/audio/audio_output_device.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/audio/clockless_audio_sink.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/audio/scoped_task_runner_observer.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/audio/virtual_audio_input_stream_unittest.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/audio/virtual_audio_output_stream_unittest.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/base/audio_renderer_mixer_unittest.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/base/bind_to_current_loop_unittest.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/base/pipeline_impl.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/cast/logging/log_event_dispatcher.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/cast/test/receiver.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/cast/test/utility/in_process_receiver.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/cast/test/utility/udp_proxy.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/filters/blocking_url_protocol.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/filters/vpx_video_decoder.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/gpu/ipc/client/gpu_jpeg_decode_accelerator_host.cc
[modify] https://crrev.com/0d77c7cb0a3c073cc8e368f4bcf0ce2374622a38/media/gpu/ipc/service/gpu_video_decode_accelerator.cc

Project Member

Comment 36 by bugdroid1@chromium.org, Jun 2 2016

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

commit 0faddf48a500f60b02b06b197678062ad3c8b856
Author: gab <gab@chromium.org>
Date: Thu Jun 02 12:26:55 2016

Migrate WaitableEvent to enum-based constructor in dbus/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/0faddf48a500f60b02b06b197678062ad3c8b856/dbus/bus.cc
[modify] https://crrev.com/0faddf48a500f60b02b06b197678062ad3c8b856/dbus/test_service.cc

Project Member

Comment 37 by bugdroid1@chromium.org, Jun 2 2016

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

commit 91637f2abc4cac94e0f123fd59ba918d85fb5fa4
Author: gab <gab@chromium.org>
Date: Thu Jun 02 13:21:24 2016

Migrate WaitableEvent to enum-based constructor in storage/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 
TBR=michaeln@chromium.org

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

[modify] https://crrev.com/91637f2abc4cac94e0f123fd59ba918d85fb5fa4/storage/common/database/database_connections.cc

Project Member

Comment 38 by bugdroid1@chromium.org, Jun 2 2016

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

commit 07efd1082dc0edfa53d8484a98433fc6e968c822
Author: gab <gab@chromium.org>
Date: Thu Jun 02 13:33:17 2016

Migrate WaitableEvent to enum-based constructor in net/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/07efd1082dc0edfa53d8484a98433fc6e968c822/net/base/address_tracker_linux_unittest.cc
[modify] https://crrev.com/07efd1082dc0edfa53d8484a98433fc6e968c822/net/base/keygen_handler_unittest.cc
[modify] https://crrev.com/07efd1082dc0edfa53d8484a98433fc6e968c822/net/disk_cache/blockfile/backend_impl.cc
[modify] https://crrev.com/07efd1082dc0edfa53d8484a98433fc6e968c822/net/disk_cache/blockfile/in_flight_io.cc
[modify] https://crrev.com/07efd1082dc0edfa53d8484a98433fc6e968c822/net/dns/serial_worker_unittest.cc
[modify] https://crrev.com/07efd1082dc0edfa53d8484a98433fc6e968c822/net/extras/sqlite/sqlite_persistent_cookie_store_perftest.cc
[modify] https://crrev.com/07efd1082dc0edfa53d8484a98433fc6e968c822/net/extras/sqlite/sqlite_persistent_cookie_store_unittest.cc
[modify] https://crrev.com/07efd1082dc0edfa53d8484a98433fc6e968c822/net/log/net_log_unittest.cc
[modify] https://crrev.com/07efd1082dc0edfa53d8484a98433fc6e968c822/net/proxy/multi_threaded_proxy_resolver_unittest.cc
[modify] https://crrev.com/07efd1082dc0edfa53d8484a98433fc6e968c822/net/proxy/proxy_config_service_linux_unittest.cc
[modify] https://crrev.com/07efd1082dc0edfa53d8484a98433fc6e968c822/net/proxy/proxy_resolver_v8_tracing.cc
[modify] https://crrev.com/07efd1082dc0edfa53d8484a98433fc6e968c822/net/tools/quic/test_tools/server_thread.cc

Project Member

Comment 39 by bugdroid1@chromium.org, Jun 2 2016

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

commit 77f88320bb9a119c44272ff1db588871653a33de
Author: gab <gab@chromium.org>
Date: Thu Jun 02 13:47:03 2016

Migrate WaitableEvent to enum-based constructor in components/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/autofill/core/browser/webdata/web_data_service_unittest.cc
[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/bookmarks/browser/bookmark_model.cc
[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/browser_sync/browser/profile_sync_service_autofill_unittest.cc
[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/font_service/public/cpp/font_service_thread.cc
[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/leveldb/leveldb_mojo_proxy.cc
[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/mus/gles2/command_buffer_local.cc
[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/mus/gles2/gpu_state.cc
[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/prefs/pref_member_unittest.cc
[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/scheduler/child/webthread_impl_for_worker_scheduler.cc
[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/scheduler/child/webthread_impl_for_worker_scheduler_unittest.cc
[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/storage_monitor/media_storage_util_unittest.cc
[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/storage_monitor/test_storage_monitor.cc
[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/sync_driver/glue/sync_backend_registrar_unittest.cc
[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/sync_driver/glue/ui_model_worker_unittest.cc
[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/sync_driver/non_ui_data_type_controller_unittest.cc
[modify] https://crrev.com/77f88320bb9a119c44272ff1db588871653a33de/components/sync_driver/shared_change_processor_unittest.cc

Project Member

Comment 40 by bugdroid1@chromium.org, Jun 2 2016

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

commit d6f9bff227238cd52644eac6bb49e6502bbb8c37
Author: gab <gab@chromium.org>
Date: Thu Jun 02 13:48:20 2016

Migrate WaitableEvent to enum-based constructor in content/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/appcache/appcache_request_handler_unittest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/appcache/appcache_response_unittest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/appcache/appcache_storage_impl_unittest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/appcache/appcache_update_job_unittest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/appcache/appcache_url_request_job_unittest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/browser_shutdown_profile_dumper.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/device_sensors/data_fetcher_shared_memory_base_unittest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/device_sensors/device_inertial_sensor_browsertest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/frame_host/debug_urls.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/frame_host/interstitial_page_impl_browsertest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/gamepad/gamepad_test_helpers.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/gpu/browser_gpu_channel_host_factory.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/gpu/browser_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/indexed_db/indexed_db_browsertest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/media/capture/desktop_capture_device_unittest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/media/capture/web_contents_audio_input_stream_unittest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/message_port_provider_browsertest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/net/quota_policy_cookie_store_unittest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/browser/plugin_data_remover_impl.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/child/blink_platform_impl.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/child/child_process.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/child/fileapi/webfilesystem_impl.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/child/fileapi/webfilewriter_impl.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/renderer/devtools/v8_sampling_profiler.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/renderer/devtools/v8_sampling_profiler_browsertest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/renderer/media/media_stream_audio_unittest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/renderer/media/rtc_peer_connection_handler.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/renderer/media/rtc_video_decoder.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/renderer/media/rtc_video_decoder_unittest.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/renderer/media/rtc_video_encoder.cc
[modify] https://crrev.com/d6f9bff227238cd52644eac6bb49e6502bbb8c37/content/renderer/media/webrtc/peer_connection_dependency_factory.cc

Project Member

Comment 41 by bugdroid1@chromium.org, Jun 2 2016

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

commit 46cab0b13bf5f4009dcfe0cb7fa270d8b0f99f1a
Author: gab <gab@chromium.org>
Date: Thu Jun 02 15:34:43 2016

Migrate WaitableEvent to enum-based constructor in thread_watcher_unittest.cc

And fix presubmit checks about usage of static strings.

(and also get rid of constant for thread ID constants..)

BUG= 612843 
NO_DEPENDENCY_CHECKS=true
TBR=rkaplow@chromium.org (to bypass incorrect owner presubmit inherited from dependency)

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

[modify] https://crrev.com/46cab0b13bf5f4009dcfe0cb7fa270d8b0f99f1a/chrome/browser/metrics/thread_watcher_unittest.cc

Project Member

Comment 42 by bugdroid1@chromium.org, Jun 2 2016

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

commit eb8ecd35077892610e593a31c8fd6dc9117520a8
Author: gab <gab@chromium.org>
Date: Thu Jun 02 15:54:44 2016

Migrate WaitableEvent to enum-based constructor in completion_event.h

Not sure why that one was missed by clang-tidy...

BUG= 612843 
TBR=enne@chromium.org
NO_DEPENDENCY_CHECKS=true
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[modify] https://crrev.com/eb8ecd35077892610e593a31c8fd6dc9117520a8/cc/base/completion_event.h

Project Member

Comment 43 by bugdroid1@chromium.org, Jun 2 2016

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

commit 56afd66cf306f84b4d7e2b6b57c59cef789b5851
Author: gab <gab@chromium.org>
Date: Thu Jun 02 16:01:00 2016

Migrate WaitableEvent to enum-based constructor in read_write_lock_unittest.cc

Just added in https://codereview.chromium.org/1988563002

BUG= 612843 
TBR=danakj@chromium.org
NO_DEPENDENCY_CHECKS=true

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

[modify] https://crrev.com/56afd66cf306f84b4d7e2b6b57c59cef789b5851/base/synchronization/read_write_lock_unittest.cc

Project Member

Comment 44 by bugdroid1@chromium.org, Jun 4 2016

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

commit d955d78dcaa5815b958f3baf078c7da16006d896
Author: gab <gab@chromium.org>
Date: Sat Jun 04 13:15:38 2016

Migrate WaitableEvent to enum-based constructor in chrome/

Change automated with clang-tidy (details @  https://crbug.com/612843#c13 )

BUG= 612843 

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

[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/extensions/api/socket/socket_apitest.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_browsertest.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/extensions/extension_messages_apitest.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/net/predictor.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/password_manager/native_backend_gnome_x.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/password_manager/native_backend_kwallet_x.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/password_manager/native_backend_kwallet_x_unittest.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/process_singleton_browsertest.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/process_singleton_posix_unittest.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/profiles/profile_browsertest.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/sessions/session_service_unittest.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/spellchecker/spellcheck_service_browsertest.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/ssl/ssl_client_certificate_selector_test.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/sync/test/integration/autofill_helper.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/sync/test/integration/bookmarks_helper.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/sync/test/integration/extension_settings_helper.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/sync/test/integration/passwords_helper.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/sync/test/integration/single_client_directory_sync_test.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/sync/test/integration/typed_urls_helper.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/ui/crypto_module_delegate_nss.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/browser/ui/views/ssl_client_certificate_selector_browsertest.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/common/service_process_util_posix.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/service/service_ipc_server_unittest.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/service/service_process.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/test/base/mojo_test_connector.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/test/base/ui_test_utils.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/test/chromedriver/chrome/adb_impl.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/test/chromedriver/net/net_util.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/test/chromedriver/net/net_util_unittest.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/test/chromedriver/net/port_server_unittest.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/test/chromedriver/net/sync_websocket_impl.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/test/chromedriver/net/sync_websocket_impl_unittest.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/test/chromedriver/net/test_http_server.cc
[modify] https://crrev.com/d955d78dcaa5815b958f3baf078c7da16006d896/chrome/test/media_router/media_router_base_browsertest.cc

Comment 45 by gab@chromium.org, Jun 6 2016

Status: Started (was: Assigned)

Project Member

Comment 46 by bugdroid1@chromium.org, Jun 7 2016

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

commit 2f2f2948b161a068fa80d01de88ec463b8d294ec
Author: gab <gab@chromium.org>
Date: Tue Jun 07 04:14:23 2016

Migrate WaitableEvent to enum-based constructor in ppapi/nacl_irt

Changes made manually because clang-tidy doesn't interact well with cross-compile.

BUG= 612843 

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

[modify] https://crrev.com/2f2f2948b161a068fa80d01de88ec463b8d294ec/ppapi/nacl_irt/manifest_service.cc
[modify] https://crrev.com/2f2f2948b161a068fa80d01de88ec463b8d294ec/ppapi/nacl_irt/plugin_startup.cc

Project Member

Comment 47 by bugdroid1@chromium.org, Jun 7 2016

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

commit cc1d091abd177e9b57101db16503418cfca07e20
Author: gab <gab@chromium.org>
Date: Tue Jun 07 17:56:04 2016

clang-tidy WaitableEvent refactor (Windows side)

Many PCH errors while running clang-tidy on Windows but still generated
a couple fixes.

Also some NACL changes per this being the first build with enabled_nacl
(had forgotten in Linux change @ https://codereview.chromium.org/2024723002/)

BUG= 612843 
TBR=danakj@chromium.org (purely mechanical and automated C++ change doesn't warrant another round of components OWNER IMO)

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

[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/chrome/app/chrome_watcher_client_unittest_win.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/chrome/chrome_watcher/chrome_watcher_main.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/components/browser_watcher/window_hang_monitor_win_unittest.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/components/mus/common/gpu_service.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/components/mus/gpu/gpu_service_mus.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/components/nacl/loader/nacl_listener.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/components/storage_monitor/test_volume_mount_watcher_win.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/extensions/browser/api/dns/mock_host_resolver_creator.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/extensions/browser/api/sockets_tcp/sockets_tcp_apitest.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/extensions/browser/api/sockets_tcp_server/sockets_tcp_server_apitest.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/extensions/browser/api/sockets_udp/sockets_udp_apitest.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/media/audio/win/audio_low_latency_input_win_unittest.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/media/capture/video/win/video_capture_device_mf_win.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/media/gpu/rendering_helper.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/media/gpu/video_decode_accelerator_unittest.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/mojo/public/cpp/bindings/tests/bind_task_runner_unittest.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/remoting/host/win/host_service.cc
[modify] https://crrev.com/cc1d091abd177e9b57101db16503418cfca07e20/ui/views/mus/views_mus_test_suite.cc

Project Member

Comment 48 by bugdroid1@chromium.org, Jun 7 2016

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

commit 77b6ad828b2e428ee448c28f695846b7afa2f43c
Author: gab <gab@chromium.org>
Date: Tue Jun 07 21:42:32 2016

clang-tidy WaitableEvent refactor (Android side)

BUG= 612843 
TBR=danakj@chromium.org
(purely mechanical and automated C++ change doesn't warrant another round of components OWNER IMO)

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

[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/android_webview/browser/aw_form_database_service.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/android_webview/browser/test/fake_window.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/android_webview/native/cookie_manager.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/base/android/application_status_listener_unittest.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/base/android/java_handler_thread.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/base/test/test_support_android.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/base/trace_event/trace_event_android.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/blimp/client/feature/compositor/blimp_input_manager.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/chrome/browser/android/bookmarks/partner_bookmarks_reader.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/chrome/browser/android/history_report/data_provider.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/chrome/browser/android/history_report/delta_file_service.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/chrome/browser/android/history_report/usage_reports_buffer_service.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/chrome/browser/android/provider/blocking_ui_thread_async_request.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/chrome/browser/history/android/sqlite_cursor.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/chrome/browser/metrics/thread_watcher_android_unittest.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/chrome/chrome_watcher/chrome_watcher_main.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/content/browser/media/android/browser_surface_view_manager.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/content/public/test/nested_message_pump_android.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/content/renderer/media/android/webmediaplayer_android.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/media/audio/android/audio_android_unittest.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/media/gpu/avda_shared_state.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/mojo/message_pump/message_pump_mojo.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/net/test/spawned_test_server/spawner_communicator.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/remoting/client/jni/chromoting_jni_runtime.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions_unittests.cc
[modify] https://crrev.com/77b6ad828b2e428ee448c28f695846b7afa2f43c/tools/cygprofile/cygprofile_unittest.cc

Comment 49 by gab@chromium.org, Jun 8 2016

Interesting :-)! Just stumbled on Blink's implementation of WaitableEvent and they made the same choice as we just did (nested enum-classes -- even named the same way!), but they allow parameters to have defaults (ResetPolicy::AUTOMATIC, InitialState::NON_SIGNALED).

That could be worthwhile since the vast majority of callers use InitialState::NON_SIGNALED (i.e. very rarely need two params) and more than half of those callers use ResetPolicy::AUTOMATIC.

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/WaitableEvent.h?rcl=0&l=52

WDYT?

Project Member

Comment 50 by bugdroid1@chromium.org, Jun 8 2016

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

commit cca5311b816c24b07adcdfda0b03297118feee2c
Author: gab <gab@chromium.org>
Date: Wed Jun 08 20:13:28 2016

Migrate WaitableEvent to enum-based constructor for callsites using non-inlined bools.

As expected: clang-tidy couldn't auto-refactor those non-inlined bools

BUG= 612843 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
NO_DEPENDENCY_CHECKS=true
TBR=Have owners approval, skipping NO_DEPENDENCY_CHECKS bug for presubmit ( http://crbug.com/616912 )

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

[modify] https://crrev.com/cca5311b816c24b07adcdfda0b03297118feee2c/cc/layers/texture_layer_unittest.cc
[modify] https://crrev.com/cca5311b816c24b07adcdfda0b03297118feee2c/components/mus/gpu/gpu_service_mus.cc

Project Member

Comment 51 by bugdroid1@chromium.org, Jun 8 2016

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

commit 7c19b196f653ec0e2b3aab703611231efc04cb2a
Author: gab <gab@chromium.org>
Date: Wed Jun 08 20:24:08 2016

Manual refactor to enum-based WaitableEvent for remaining _win files

BUG= 612843 
R=fdoray@chromium.org
TBR=danakj@chromium.org
(Francois please fact check me, I'm TBRing dana for otherwise mechanical change)

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

[modify] https://crrev.com/7c19b196f653ec0e2b3aab703611231efc04cb2a/base/sync_socket_win.cc
[modify] https://crrev.com/7c19b196f653ec0e2b3aab703611231efc04cb2a/chrome/browser/chrome_select_file_dialog_factory_win.cc
[modify] https://crrev.com/7c19b196f653ec0e2b3aab703611231efc04cb2a/chrome/browser/password_manager/password_store_win_unittest.cc
[modify] https://crrev.com/7c19b196f653ec0e2b3aab703611231efc04cb2a/components/policy/core/common/policy_loader_win.cc
[modify] https://crrev.com/7c19b196f653ec0e2b3aab703611231efc04cb2a/net/proxy/dhcp_proxy_script_adapter_fetcher_win_unittest.cc
[modify] https://crrev.com/7c19b196f653ec0e2b3aab703611231efc04cb2a/net/proxy/dhcp_proxy_script_fetcher_win_unittest.cc

Project Member

Comment 52 by bugdroid1@chromium.org, Jun 8 2016

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

commit 3a2088cc4d83578a6465125207d505728f4c3899
Author: gab <gab@chromium.org>
Date: Wed Jun 08 20:42:55 2016

clang-tidy WaitableEvent refactor (ChromeOS side)

BUG= 612843 
TBR=danakj@chromium.org
(purely mechanical and automated C++ change doesn't warrant another round of components OWNER IMO)

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

[modify] https://crrev.com/3a2088cc4d83578a6465125207d505728f4c3899/chrome/browser/chromeos/extensions/default_app_order.cc
[modify] https://crrev.com/3a2088cc4d83578a6465125207d505728f4c3899/chrome/browser/chromeos/login/signin/oauth2_browsertest.cc
[modify] https://crrev.com/3a2088cc4d83578a6465125207d505728f4c3899/chromeos/dbus/blocking_method_caller.cc
[modify] https://crrev.com/3a2088cc4d83578a6465125207d505728f4c3899/chromeos/system/statistics_provider.cc
[modify] https://crrev.com/3a2088cc4d83578a6465125207d505728f4c3899/components/exo/wayland/server_unittest.cc

Project Member

Comment 53 by bugdroid1@chromium.org, Jun 8 2016

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

commit 5600d5b816bb658eeeae87a53158a500d985f1da
Author: gab <gab@chromium.org>
Date: Wed Jun 08 21:44:41 2016

Manual refactor to enum-based WaitableEvent for remaining mac/ios files

BUG= 612843 
NO_DEPENDENCY_CHECKS=true
R=fdoray@chromium.org
TBR=danakj@chromium.org
(Francois please fact check me, I'm TBRing dana for otherwise mechanical change)

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

[modify] https://crrev.com/5600d5b816bb658eeeae87a53158a500d985f1da/chrome/browser/media_galleries/mac/mtp_device_delegate_impl_mac_unittest.mm
[modify] https://crrev.com/5600d5b816bb658eeeae87a53158a500d985f1da/components/cronet/ios/cronet_environment.cc
[modify] https://crrev.com/5600d5b816bb658eeeae87a53158a500d985f1da/components/cronet/ios/test/cronet_bidirectional_stream_test.mm
[modify] https://crrev.com/5600d5b816bb658eeeae87a53158a500d985f1da/components/cronet/ios/test/quic_test_server.cc
[modify] https://crrev.com/5600d5b816bb658eeeae87a53158a500d985f1da/content/browser/mach_broker_mac_unittest.cc
[modify] https://crrev.com/5600d5b816bb658eeeae87a53158a500d985f1da/ios/chrome/browser/browser_state/chrome_browser_state_impl.cc
[modify] https://crrev.com/5600d5b816bb658eeeae87a53158a500d985f1da/media/audio/mac/audio_auhal_mac_unittest.cc
[modify] https://crrev.com/5600d5b816bb658eeeae87a53158a500d985f1da/ui/accelerated_widget_mac/window_resize_helper_mac.cc

Project Member

Comment 54 by bugdroid1@chromium.org, Jun 8 2016

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

commit 7a5d04b5299133bf31df423e0eb125fbb2998d65
Author: gab <gab@chromium.org>
Date: Wed Jun 08 21:54:14 2016

Adapt WebKit's WaitableEvent to base's new enum-based WaitableEvent

BUG= 612843 
NO_DEPENDENCY_CHECKS=true

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

[modify] https://crrev.com/7a5d04b5299133bf31df423e0eb125fbb2998d65/third_party/WebKit/Source/platform/WaitableEvent.cpp

Project Member

Comment 55 by bugdroid1@chromium.org, Jun 9 2016

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

commit 4d5284c4618a9f95cbe7cb60709432db05b38e68
Author: gab <gab@chromium.org>
Date: Thu Jun 09 02:52:35 2016

Manual refactor of few more stragglers to enum-based WaitableEvent constructor.

Found a couple more sites with regex "(WaitableEvent|event_)\((true|false),", hoping those are the last ones..!

BUG= 612843 
R=fdoray@chromium.org
TBR=danakj@chromium.org
(Francois please fact check me, I'm TBRing dana for otherwise mechanical change)

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

[modify] https://crrev.com/4d5284c4618a9f95cbe7cb60709432db05b38e68/base/mac/mach_port_broker_unittest.cc
[modify] https://crrev.com/4d5284c4618a9f95cbe7cb60709432db05b38e68/chrome/browser/media_galleries/fileapi/itunes_file_util_unittest.cc
[modify] https://crrev.com/4d5284c4618a9f95cbe7cb60709432db05b38e68/components/nacl/loader/nonsfi/nonsfi_listener.cc
[modify] https://crrev.com/4d5284c4618a9f95cbe7cb60709432db05b38e68/tools/ipc_fuzzer/message_replay/replay_process.cc
[modify] https://crrev.com/4d5284c4618a9f95cbe7cb60709432db05b38e68/ui/ozone/platform/wayland/fake_server.cc

Project Member

Comment 56 by bugdroid1@chromium.org, Jun 9 2016

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

commit fd84d02f5300d4659644d19912bbb964cd6d5a95
Author: gab <gab@chromium.org>
Date: Thu Jun 09 14:18:14 2016

More WaitableEvent stragglers migrated to enum-based constructor

Previous manual regex was finding "new WaitableEvent(...)" and
"...event_(...)", i.e. heap allocs and initializer lists but
was missing stack variables.

This change uses the following regex to catch those and manually fix them:
"WaitableEvent\ [\w]+\((true|false),"

Also fixed missing space presubmit error in media_message_fifo_unittest.cc

BUG= 612843 
R=fdoray@chromium.org
TBR=danakj@chromium.org

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

[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/chrome/browser/android/provider/run_on_ui_thread_blocking.h
[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/chromecast/base/bind_to_task_runner_unittest.cc
[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/chromecast/media/base/media_resource_tracker_unittest.cc
[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/chromecast/media/cma/ipc/media_message_fifo_unittest.cc
[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/media/audio/cras/cras_input_unittest.cc
[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/media/audio/cras/cras_unified_unittest.cc
[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/media/gpu/v4l2_slice_video_decode_accelerator.cc
[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/ui/ozone/platform/drm/gpu/proxy_helpers.cc

Project Member

Comment 57 by bugdroid1@chromium.org, Jun 9 2016

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

commit 6e92a38b62da839d5dbf084b8496e67c07fae63a
Author: gab <gab@chromium.org>
Date: Thu Jun 09 17:22:14 2016

Fix two more compile errors for WaitableEvent boolean-based constructor removal.

Caught by dry-run @ https://codereview.chromium.org/2030683005/

BUG= 612843 
R=fdoray@chromium.org
TBR=danakj@chromium.org

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

[modify] https://crrev.com/6e92a38b62da839d5dbf084b8496e67c07fae63a/base/mac/libdispatch_task_runner.cc
[modify] https://crrev.com/6e92a38b62da839d5dbf084b8496e67c07fae63a/media/gpu/v4l2_video_decode_accelerator.cc

Comment 58 by danakj@chromium.org, Jun 9 2016

> Interesting :-)! Just stumbled on Blink's implementation of WaitableEvent and they made the same choice as we just did (nested enum-classes -- even named the same way!), but they allow parameters to have defaults (ResetPolicy::AUTOMATIC, InitialState::NON_SIGNALED).

Not really a fan of mimicing that. Then you have to look up the constructor when you read the callsite, which was the point of removing the bools again.

Project Member

Comment 59 by bugdroid1@chromium.org, Jun 9 2016

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

commit d9aa1d1595be22ea06a83eb6216414cebae6b2e8
Author: gab <gab@chromium.org>
Date: Thu Jun 09 18:41:22 2016

Remove boolean-based WaitableEvent constructor.

BUG= 612843 
TBR=danakj@chromium.org

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

[modify] https://crrev.com/d9aa1d1595be22ea06a83eb6216414cebae6b2e8/base/synchronization/waitable_event.h
[modify] https://crrev.com/d9aa1d1595be22ea06a83eb6216414cebae6b2e8/base/synchronization/waitable_event_posix.cc
[modify] https://crrev.com/d9aa1d1595be22ea06a83eb6216414cebae6b2e8/base/synchronization/waitable_event_win.cc

Comment 60 by gab@chromium.org, Jun 9 2016

Status: Fixed (was: Started)

All done, woohoo :-)!

Project Member

Comment 61 by bugdroid1@chromium.org, Jun 15 2016

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

commit fd84d02f5300d4659644d19912bbb964cd6d5a95
Author: gab <gab@chromium.org>
Date: Thu Jun 09 14:18:14 2016

More WaitableEvent stragglers migrated to enum-based constructor

Previous manual regex was finding "new WaitableEvent(...)" and
"...event_(...)", i.e. heap allocs and initializer lists but
was missing stack variables.

This change uses the following regex to catch those and manually fix them:
"WaitableEvent\ [\w]+\((true|false),"

Also fixed missing space presubmit error in media_message_fifo_unittest.cc

BUG= 612843 
R=fdoray@chromium.org
TBR=danakj@chromium.org

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

[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/chrome/browser/android/provider/run_on_ui_thread_blocking.h
[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/chromecast/base/bind_to_task_runner_unittest.cc
[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/chromecast/media/base/media_resource_tracker_unittest.cc
[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/chromecast/media/cma/ipc/media_message_fifo_unittest.cc
[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/media/audio/cras/cras_input_unittest.cc
[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/media/audio/cras/cras_unified_unittest.cc
[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/media/gpu/v4l2_slice_video_decode_accelerator.cc
[modify] https://crrev.com/fd84d02f5300d4659644d19912bbb964cd6d5a95/ui/ozone/platform/drm/gpu/proxy_helpers.cc

Project Member

Comment 62 by bugdroid1@chromium.org, Jun 15 2016

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

commit 6e92a38b62da839d5dbf084b8496e67c07fae63a
Author: gab <gab@chromium.org>
Date: Thu Jun 09 17:22:14 2016

Fix two more compile errors for WaitableEvent boolean-based constructor removal.

Caught by dry-run @ https://codereview.chromium.org/2030683005/

BUG= 612843 
R=fdoray@chromium.org
TBR=danakj@chromium.org

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

[modify] https://crrev.com/6e92a38b62da839d5dbf084b8496e67c07fae63a/base/mac/libdispatch_task_runner.cc
[modify] https://crrev.com/6e92a38b62da839d5dbf084b8496e67c07fae63a/media/gpu/v4l2_video_decode_accelerator.cc

Project Member

Comment 63 by bugdroid1@chromium.org, Jun 15 2016

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

commit d9aa1d1595be22ea06a83eb6216414cebae6b2e8
Author: gab <gab@chromium.org>
Date: Thu Jun 09 18:41:22 2016

Remove boolean-based WaitableEvent constructor.

BUG= 612843 
TBR=danakj@chromium.org

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

[modify] https://crrev.com/d9aa1d1595be22ea06a83eb6216414cebae6b2e8/base/synchronization/waitable_event.h
[modify] https://crrev.com/d9aa1d1595be22ea06a83eb6216414cebae6b2e8/base/synchronization/waitable_event_posix.cc
[modify] https://crrev.com/d9aa1d1595be22ea06a83eb6216414cebae6b2e8/base/synchronization/waitable_event_win.cc

Project Member

Comment 64 by bugdroid1@chromium.org, May 10 2018

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

commit c9ba7a990ab0b915bf85cc17f5a9a5474cb92da7
Author: Gabriel Charette <gab@chromium.org>
Date: Thu May 10 21:10:59 2018

[base] Add default arguments for WaitableEvent construction.

While having these enums since r396989 makes callsites more readable,
I find that most of the time for simple use cases in tests I don't care
much about the ResetPolicy (one time usage) and always want it to be
NOT_SIGNALED initially.

Surverying the codebase yields a slight majority of MANUAL over
AUTOMATIC (from experience I've written AUTOMATIC in some places where
it didn't matter either way as well). And NOT_SIGNALED is an obvious
winner for InitialState.

Also, IMO, MANUAL is the less-surprising default (i.e. can be used as a
flag).

As such I think having default arguments will vastly simplify
writeability for code using WaitableEvents (tests in particular). Few
sites will need to care and most that do will only need to specify the
ResetPolicy.

R=danakj@chromium.org

Bug:  612843 
Change-Id: Ib0ef193b041e64ea9456748633bc49c64af60f19
Reviewed-on: https://chromium-review.googlesource.com/1053843
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557672}
[modify] https://crrev.com/c9ba7a990ab0b915bf85cc17f5a9a5474cb92da7/base/synchronization/waitable_event.h

Project Member

Comment 65 by bugdroid1@chromium.org, May 31 2018

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

commit 93d1c1d411c8dd18091d890c4f4721e30b4a4591
Author: Francois Doray <fdoray@chromium.org>
Date: Thu May 31 22:11:28 2018

TaskScheduler: Use WaitableEvent's default constructor when possible.

It is not necessary to provide explict arguments to construct
a MANUAL/NOT_SIGNALED WaitableEvent.

Bug:  612843 
Change-Id: I4880d37443742c44f129ef149d16d94136155dc0
Reviewed-on: https://chromium-review.googlesource.com/1080259
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563395}
[modify] https://crrev.com/93d1c1d411c8dd18091d890c4f4721e30b4a4591/base/task_scheduler/priority_queue_unittest.cc
[modify] https://crrev.com/93d1c1d411c8dd18091d890c4f4721e30b4a4591/base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc
[modify] https://crrev.com/93d1c1d411c8dd18091d890c4f4721e30b4a4591/base/task_scheduler/scheduler_worker_pool_impl_unittest.cc
[modify] https://crrev.com/93d1c1d411c8dd18091d890c4f4721e30b4a4591/base/task_scheduler/scheduler_worker_pool_unittest.cc
[modify] https://crrev.com/93d1c1d411c8dd18091d890c4f4721e30b4a4591/base/task_scheduler/scheduler_worker_unittest.cc
[modify] https://crrev.com/93d1c1d411c8dd18091d890c4f4721e30b4a4591/base/task_scheduler/task_scheduler_impl_unittest.cc
[modify] https://crrev.com/93d1c1d411c8dd18091d890c4f4721e30b4a4591/base/task_scheduler/task_tracker.cc
[modify] https://crrev.com/93d1c1d411c8dd18091d890c4f4721e30b4a4591/base/task_scheduler/task_tracker_unittest.cc
[modify] https://crrev.com/93d1c1d411c8dd18091d890c4f4721e30b4a4591/base/task_scheduler/tracked_ref.h

►

Issue 612843 link

Issue metadata

Change WaitableEvent's constructor to be enum-based.

Issue description

Comment 1 by thakis@chromium.org, May 18 2016

Comment 1 Deleted

Comment 2 by thakis@chromium.org, May 18 2016

Comment 2 Deleted

Comment 3 by gab@chromium.org, May 18 2016

Comment 3 Deleted

Comment 4 by thakis@chromium.org, May 18 2016

Comment 4 Deleted

Comment 5 by mark@chromium.org, May 18 2016

Comment 5 Deleted

Comment 6 by danakj@chromium.org, May 18 2016

Comment 6 Deleted

Comment 7 by gab@chromium.org, May 25 2016

Comment 7 Deleted

Comment 8 by fdoray@chromium.org, May 25 2016

Comment 8 Deleted

Comment 9 by gab@chromium.org, May 25 2016

Comment 9 Deleted

Comment 10 by danakj@chromium.org, May 25 2016

Comment 10 Deleted

Comment 11 by danakj@chromium.org, May 25 2016

Comment 11 Deleted

Comment 12 by gab@chromium.org, May 30 2016

Comment 12 Deleted

Comment 13 by gab@chromium.org, May 30 2016

Comment 13 Deleted

Comment 14 by bugdroid1@chromium.org, Jun 1 2016

Comment 14 Deleted

Comment 15 by gab@chromium.org, Jun 1 2016

Comment 15 Deleted

Comment 16 by gab@chromium.org, Jun 1 2016

Comment 16 Deleted

Comment 17 by gab@chromium.org, Jun 1 2016

Comment 17 Deleted

Comment 18 by danakj@chromium.org, Jun 1 2016

Comment 18 Deleted

Comment 19 by bugdroid1@chromium.org, Jun 1 2016

Comment 19 Deleted

Comment 20 by bugdroid1@chromium.org, Jun 1 2016

Comment 20 Deleted

Comment 21 by sky@chromium.org, Jun 1 2016

Comment 21 Deleted

Comment 22 by bugdroid1@chromium.org, Jun 1 2016

Comment 22 Deleted

Comment 23 by bugdroid1@chromium.org, Jun 1 2016

Comment 23 Deleted

Comment 24 by danakj@chromium.org, Jun 1 2016

Comment 24 Deleted

Comment 25 by bugdroid1@chromium.org, Jun 1 2016

Comment 25 Deleted

Comment 26 by gab@chromium.org, Jun 1 2016

Comment 26 Deleted

Comment 27 by bugdroid1@chromium.org, Jun 1 2016

Comment 27 Deleted

Comment 28 by bugdroid1@chromium.org, Jun 1 2016

Comment 28 Deleted

Comment 29 by bugdroid1@chromium.org, Jun 1 2016

Comment 29 Deleted

Comment 30 by sky@chromium.org, Jun 1 2016

Comment 30 Deleted

Comment 31 by bugdroid1@chromium.org, Jun 1 2016

Comment 31 Deleted

Comment 32 by bugdroid1@chromium.org, Jun 1 2016

Comment 32 Deleted

Comment 33 by bugdroid1@chromium.org, Jun 1 2016

Comment 33 Deleted

Comment 34 by bugdroid1@chromium.org, Jun 2 2016

Comment 34 Deleted

Comment 35 by bugdroid1@chromium.org, Jun 2 2016

Comment 35 Deleted

Comment 36 by bugdroid1@chromium.org, Jun 2 2016

Comment 36 Deleted

Comment 37 by bugdroid1@chromium.org, Jun 2 2016

Comment 37 Deleted

Comment 38 by bugdroid1@chromium.org, Jun 2 2016

Comment 38 Deleted