Issue 338603003: Use abs() instead of std::abs() in Editor::firstRectForRange().

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 338603003: Use abs() instead of std::abs() in Editor::firstRectForRange(). (Closed)

Created:
6 years, 6 months ago by Lei Zhang

Modified:
6 years, 6 months ago

Reviewers:
ojan

CC:
blink-reviews, groby+blinkspell_chromium.org, tkent, Inactive, Sunil Ratnu

Base URL:
http://src.chromium.org/blink/trunk/

Project:
blink

Visibility:
Public.

More Reviews

Description

Use abs() instead of std::abs() in Editor::firstRectForRange(). The subtle switch to std::abs() in r176032 caused TSAN bots to fail. BUG=384541

Patch Set 1 #

Created: 6 years, 6 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+1 line, -1 line)			Patch
	M	Source/core/editing/Editor.cpp	View		1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Lei Zhang

The CL that caused the issue got reverted as r176125. Closing...

6 years, 6 months ago (2014-06-14 00:28:36 UTC) #2

haraken

On 2014/06/14 00:28:36, Lei Zhang wrote: > The CL that caused the issue got reverted ...

6 years, 6 months ago (2014-06-14 00:30:52 UTC) #3

Lei Zhang

6 years, 6 months ago (2014-06-14 00:36:03 UTC) #4

Message was sent while issue was closed.

On 2014/06/14 00:30:52, haraken wrote:
> On 2014/06/14 00:28:36, Lei Zhang wrote:
> > The CL that caused the issue got reverted as r176125. Closing...
> 
> It would be worth fixing this so that we can reland the CL :)
> 
> It looks like we have two other std::abs()s in the code base:
>
https://code.google.com/p/chromium/codesearch#search/&q=std::abs%20package:%5...
> 
> Do we need to update them to ::abs as well?

It depends on the type, but since they are not causing any build problems,
probably not. I updated the bug with more info.

Expand Messages | Collapse Messages