New issue
Advanced search Search tips

Issue 708949 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Avoid reused symbol names in third_party/WebKit/Source/editing

Project Member Reported by brat...@opera.com, Apr 6 2017

Issue description

I'm experimenting with unity builds (see various posts on blink-dev the last month) for much shorter compile times.

A unity build is a build where several/many compilation units are compiled together to avoid the overhead from shared headers (primarily) and with some other positive side effects. Measured speedup is around 8-15x in Blink.

When compiling many compilation units together you run into name conflicts when the same symbol name is used in global or file scope in several files.

It is not common because there are other good reasons to use unique names, but there are a couple of cases in Blink.

This task is tracking me renaming six symbols in core/editing (and possibly more if someone adds more of them).

 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 6 2017

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

commit b6ba28d755eb8cbcc5dba2f414d10cd2e434f6d8
Author: bratell <bratell@opera.com>
Date: Thu Apr 06 11:14:46 2017

Avoid duplicate functions/code in core/editing: computeDistance

While experimenting with unity builds I encountered a few duplicate
symbols and functions in core/editing. This patch renames, moves
and unifies them.

Several classes use computeDistanceToLeftGraphemeBoundary and instead
of copying the whole function, make a single copy in EditingUtilities.
Also move computeDistanceToRightGraphemeBoundary since those two functions
are too similar to split up.

BUG= 708949 
R=yosin@chromium.org

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

[modify] https://crrev.com/b6ba28d755eb8cbcc5dba2f414d10cd2e434f6d8/third_party/WebKit/Source/core/editing/EditingUtilities.cpp
[modify] https://crrev.com/b6ba28d755eb8cbcc5dba2f414d10cd2e434f6d8/third_party/WebKit/Source/core/editing/EditingUtilities.h
[modify] https://crrev.com/b6ba28d755eb8cbcc5dba2f414d10cd2e434f6d8/third_party/WebKit/Source/core/editing/InputMethodController.cpp
[modify] https://crrev.com/b6ba28d755eb8cbcc5dba2f414d10cd2e434f6d8/third_party/WebKit/Source/core/editing/commands/InsertIncrementalTextCommand.cpp

Project Member

Comment 2 by bugdroid1@chromium.org, Apr 6 2017

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

commit 96ea303ff0f2ed2bc7fa35387c0f1b85b1fc0e40
Author: bratell <bratell@opera.com>
Date: Thu Apr 06 11:25:54 2017

Avoid duplicate functions/code in core/editing: kUnsetCodePoint

While experimenting with unity builds I encountered a few duplicate
symbols and functions in core/editing. This patch renames, moves
and unifies them.

kInvalidCodePoint is used in both ForwardGraphemeBoundaryStateMachine and
BackwardGraphemeBoundaryStateMachine so to avoid clashes, use different
names for them.

BUG= 708949 
R=yosin@chromium.org

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

[modify] https://crrev.com/96ea303ff0f2ed2bc7fa35387c0f1b85b1fc0e40/third_party/WebKit/Source/core/editing/state_machines/ForwardGraphemeBoundaryStateMachine.cpp

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 6 2017

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

commit 0736e37b73d70fc56a93eeac32fff4a56c946c2d
Author: bratell <bratell@opera.com>
Date: Thu Apr 06 11:32:58 2017

Avoid duplicate functions/code in core/editing: DirectionalSelection

While experimenting with unity builds I encountered a few duplicate
symbols and functions in core/editing. This patch renames, moves
and unifies them.

shouldAlwaysUseDirectionalSelection is a common helper function
and since it's not identicallty implemented everywhere it cannot
be merged. Instead make the one in SelectionModifier a member function
to move it out of global scope.

BUG= 708949 
R=yosin@chromium.org

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

[modify] https://crrev.com/0736e37b73d70fc56a93eeac32fff4a56c946c2d/third_party/WebKit/Source/core/editing/SelectionModifier.cpp
[modify] https://crrev.com/0736e37b73d70fc56a93eeac32fff4a56c946c2d/third_party/WebKit/Source/core/editing/SelectionModifier.h

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 6 2017

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

commit 00a3908827f01ca5cd0bd911a7f12239975bee33
Author: bratell <bratell@opera.com>
Date: Thu Apr 06 11:34:45 2017

Avoid duplicate functions/code in core/editing: kInvalidOffset

While experimenting with unity builds I encountered a few duplicate
symbols and functions in core/editing. This patch renames, moves
and unifies them.

kInvalidOffset is a name used both by PositionIterator and TextIterator.
This renames on of them kInvalidTextOffset to better match its use and
to avoid the name collision.

BUG= 708949 
R=yosin@chromium.org

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

[modify] https://crrev.com/00a3908827f01ca5cd0bd911a7f12239975bee33/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 6 2017

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

commit 2bd991090f462a64efe0f6b26bf0ad35e19386b0
Author: bratell <bratell@opera.com>
Date: Thu Apr 06 11:46:41 2017

Avoid duplicate functions/code in core/editing: MatchResultICU

While experimenting with unity builds I encountered a few duplicate
symbols and functions in core/editing. This patch renames, moves
and unifies them.

There is a global MatchResult in css/SelectorChecker.h and if that one
is included anywhere in editing, then it will collide with a local
MatchResult used in iterators. Renaming the local one MatchResultICU
will both match the name of other symbols and avoid the collision.

BUG= 708949 
R=yosin@chromium.org

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

[modify] https://crrev.com/2bd991090f462a64efe0f6b26bf0ad35e19386b0/third_party/WebKit/Source/core/editing/iterators/SearchBuffer.cpp
[modify] https://crrev.com/2bd991090f462a64efe0f6b26bf0ad35e19386b0/third_party/WebKit/Source/core/editing/iterators/TextSearcherICU.cpp
[modify] https://crrev.com/2bd991090f462a64efe0f6b26bf0ad35e19386b0/third_party/WebKit/Source/core/editing/iterators/TextSearcherICU.h
[modify] https://crrev.com/2bd991090f462a64efe0f6b26bf0ad35e19386b0/third_party/WebKit/Source/core/editing/iterators/TextSearcherICUTest.cpp

Components: Blink>Editing
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 7 2017

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

commit a7159abf14936edb1b8782dc06253c1ff91b9000
Author: bratell <bratell@opera.com>
Date: Fri Apr 07 20:42:04 2017

Avoid duplicate functions/code in core/editing: endTag

While experimenting with unity builds I encountered a few duplicate
symbols and functions in core/editing. This patch renames, moves
and unifies them.

elementCannotHaveEndTag is a utility function used in serializers
and since it is used in multiple places, and MarkupFormatter is
not a good place for it, let us put it in EditingUtilities.

BUG= 708949 
R=yosin@chromium.org

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

[modify] https://crrev.com/a7159abf14936edb1b8782dc06253c1ff91b9000/third_party/WebKit/Source/core/editing/EditingUtilities.cpp
[modify] https://crrev.com/a7159abf14936edb1b8782dc06253c1ff91b9000/third_party/WebKit/Source/core/editing/EditingUtilities.h
[modify] https://crrev.com/a7159abf14936edb1b8782dc06253c1ff91b9000/third_party/WebKit/Source/core/editing/serializers/MarkupAccumulator.cpp
[modify] https://crrev.com/a7159abf14936edb1b8782dc06253c1ff91b9000/third_party/WebKit/Source/core/editing/serializers/MarkupFormatter.cpp

Comment 8 by brat...@opera.com, Jul 14 2017

Status: Fixed (was: Started)

Sign in to add a comment