New issue
Advanced search Search tips

Issue 745732 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 713137



Sign in to add a comment

[jumbo] Conflicting names in Blink unit_tests

Project Member Reported by brat...@opera.com, Jul 18 2017

Issue description

Jumbo merges cpp files which means that if two cpp files use the same names for local functions or classes, those will collide. This happens in about 1-2% of browser files, but in 15-20% of unit_test files. Probably because tests are repetitive and things are copied between test files.

This bug tracks the effort to make names more unique.
 
Cc: mlamouri@chromium.org
What the goal of jumbo? Build time or binary size? What would be the benefit to have tests targets built that way? Or is it required for some reasons?
I guess this is answering my first question: https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/ThDAjO7fTro/discussion

(ie. it's for built time)
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 18 2017

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

commit 17f2e2b3a921e00006d23418e322c0c2a6bd284e
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Jul 18 15:15:22 2017

Renamed MockWebAudioDevice in webaudio unit tests

Both AudioContextTest and BaseAudioContextTest use a local test
class named MockWebAudioDevice. That is normally no big deal
but in jumbo builds they can be compiled in the same translation unit
and then the classes will collide. This patch gives the classes
unique prefixes so that no tests have to be excluded from jumbo.

Bug:  745732 

Change-Id: Ie736a0541082c56af745d51061897a081d2d1da5
Reviewed-on: https://chromium-review.googlesource.com/575142
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487475}
[modify] https://crrev.com/17f2e2b3a921e00006d23418e322c0c2a6bd284e/third_party/WebKit/Source/modules/webaudio/AudioContextTest.cpp
[modify] https://crrev.com/17f2e2b3a921e00006d23418e322c0c2a6bd284e/third_party/WebKit/Source/modules/webaudio/BaseAudioContextTest.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 18 2017

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

commit 99d414984026943ef8c5ff678033f77759d385c0
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Jul 18 15:27:01 2017

Renamed MockEventListener in presentation and remoteplayback unit tests

Both PresentationReceiverTest and RemotePlaybackTest use a local test
class named MockEventListener. That is normally no big deal
but in jumbo builds they can be compiled in the same translation unit
and then the classes will collide. This patch gives the classes
unique prefixes so that no tests have to be excluded from jumbo.

Bug:  745732 

Change-Id: Ic9a5e4e896b226d5ef7f99f2549cd3d4bb66fc42
Reviewed-on: https://chromium-review.googlesource.com/575141
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487479}
[modify] https://crrev.com/99d414984026943ef8c5ff678033f77759d385c0/third_party/WebKit/Source/modules/presentation/PresentationReceiverTest.cpp
[modify] https://crrev.com/99d414984026943ef8c5ff678033f77759d385c0/third_party/WebKit/Source/modules/remoteplayback/RemotePlaybackTest.cpp

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 18 2017

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

commit fcfcaa9965c21f1652f8616cc51ec1caa4603961
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Jul 18 15:44:21 2017

Made test class names unique in media_controls unit tests

MediaControlsImplTest and MediaControlsOrientationLockDelegate use the
same class names for Mock classes. Such as MockChromeClient and
StubLocalFrameClient. That is normally no big deal but in jumbo builds
they can be compiled in the same translation unit and then the classes
will collide. This patch gives the class names unique suffixes so that
media_controls tests don't have to be excluded from jumbo.

Bug:  745732 

Change-Id: I6c3a4d5d92ae690ece53aa093ffb4039eb79137e
Reviewed-on: https://chromium-review.googlesource.com/575140
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487485}
[modify] https://crrev.com/fcfcaa9965c21f1652f8616cc51ec1caa4603961/third_party/WebKit/Source/modules/media_controls/MediaControlsImplTest.cpp
[modify] https://crrev.com/fcfcaa9965c21f1652f8616cc51ec1caa4603961/third_party/WebKit/Source/modules/media_controls/MediaControlsOrientationLockDelegateTest.cpp

Comment 6 by brat...@opera.com, Jul 18 2017

The goal is 100% compile times. With non-accelerated compile times approaching 4 hours or more it is very hard to work on Chromium based code nowadays. 

There might be secondary positive side effects as in lower disk requirements, possibly better optimizations, maybe faster code, maybe smaller binaries, maybe more responsive and faster debuggers, maybe smaller debug data, but all those are secondary and apart from the disk usage might not happen at all.

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 18 2017

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

commit 352d0841e4c028c0b45f72501c9dc9a0a86bee52
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Jul 18 19:05:07 2017

Deduplicate CreateIDBValue() used in indexeddb unit_tests

IDBRequestTest and IDBTransactionTest both had an identical helper
function CreateIDBValue. In jumbo builds those identical functions
collided so this patch moves the code to a shared helper file.

Bug:  745732 

Change-Id: I36c658509c76deca4e03590858140e02a07f38d3
Reviewed-on: https://chromium-review.googlesource.com/575145
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487547}
[modify] https://crrev.com/352d0841e4c028c0b45f72501c9dc9a0a86bee52/third_party/WebKit/Source/modules/BUILD.gn
[modify] https://crrev.com/352d0841e4c028c0b45f72501c9dc9a0a86bee52/third_party/WebKit/Source/modules/indexeddb/IDBRequestTest.cpp
[add] https://crrev.com/352d0841e4c028c0b45f72501c9dc9a0a86bee52/third_party/WebKit/Source/modules/indexeddb/IDBTestHelper.cpp
[add] https://crrev.com/352d0841e4c028c0b45f72501c9dc9a0a86bee52/third_party/WebKit/Source/modules/indexeddb/IDBTestHelper.h
[modify] https://crrev.com/352d0841e4c028c0b45f72501c9dc9a0a86bee52/third_party/WebKit/Source/modules/indexeddb/IDBTransactionTest.cpp

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 20 2017

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

commit 9694c11943499756f6289c37324f5d2463b00e63
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Jul 20 21:14:13 2017

Renamed CreateDecoder helper functions in Image decoder tests

All image decoder test have a helper function "CreateDecoder" which
conflicts in jumbo builds where the tests are merged. This patch renames
them "CreateBMPDecoder", "CreateJPEGDecoder" and so on.

Bug:  745732 
Change-Id: I2cfd108c39ade76a09059d77778557ab44b82752
Reviewed-on: https://chromium-review.googlesource.com/576098
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#488407}
[modify] https://crrev.com/9694c11943499756f6289c37324f5d2463b00e63/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageDecoderTest.cpp
[modify] https://crrev.com/9694c11943499756f6289c37324f5d2463b00e63/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoderTest.cpp
[modify] https://crrev.com/9694c11943499756f6289c37324f5d2463b00e63/third_party/WebKit/Source/platform/image-decoders/jpeg/JPEGImageDecoderTest.cpp
[modify] https://crrev.com/9694c11943499756f6289c37324f5d2463b00e63/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoderTest.cpp
[modify] https://crrev.com/9694c11943499756f6289c37324f5d2463b00e63/third_party/WebKit/Source/platform/image-decoders/webp/WEBPImageDecoderTest.cpp

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 5 2017

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

commit 455bf38ce13b99fb232e4314f6d9fd414eb5f3e5
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Sep 05 09:54:31 2017

Put local types in the anonymous namespace to be consistent.

BytesConsumerTestUtil.cpp imports BytesConsumer::Result as blink::Result
and BlobBytesConsumerTest.cpp imports BytesConsumer::Result as
blink::{anonymous namespace}::Result and for the Windows compiler that
causes a naming conflict in jumbo builds.

This patch puts both BytesConsumer::Result in the anonymous namespace so
that the compiler won't get confused.

Bug:  745732 
Change-Id: I073a36fb71109be2e7fefff34268186d03a5547b
Reviewed-on: https://chromium-review.googlesource.com/649226
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#499595}
[modify] https://crrev.com/455bf38ce13b99fb232e4314f6d9fd414eb5f3e5/third_party/WebKit/Source/modules/fetch/BytesConsumerTestUtil.cpp

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 7 2017

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

commit e6dd853f39f974cf5f253e87a8a9470d25a43d2a
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Sep 07 16:25:24 2017

Avoid the name UnicodeRange in tests since it already exists

There is a platform/fonts type UnicodeRange so creating a function
with the same name causes confusion in jumbo builds where both
the type and the function will be known to the compiler at the same
time.

R=fs@opera.com

Bug:  745732 
Change-Id: Id8c9ebfb7289cd546979638814b3e67b169f6671
Reviewed-on: https://chromium-review.googlesource.com/655457
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#500320}
[modify] https://crrev.com/e6dd853f39f974cf5f253e87a8a9470d25a43d2a/third_party/WebKit/Source/core/css/parser/CSSTokenizerTest.cpp

Project Member

Comment 11 by bugdroid1@chromium.org, Sep 11 2017

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

commit 810ca667e4510713cbeb98bb27e4bc41b85686e4
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Sep 11 08:35:10 2017

Renamed IsValid in blink/platform/network unit tests

Both ParsedContentDispositionTest and ParsedContentTypeTest used a local
helper function named IsValid. That is normally no big deal
but in jumbo builds they can be compiled in the same translation unit
and then the function will collide. This patch gives the functions
unique names so that no tests have to be excluded from jumbo.

Bug:  745732 
Change-Id: Icc591553e54062eb189d8c7df243191c3f75f42a
Reviewed-on: https://chromium-review.googlesource.com/657840
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#500860}
[modify] https://crrev.com/810ca667e4510713cbeb98bb27e4bc41b85686e4/third_party/WebKit/Source/platform/network/ParsedContentDispositionTest.cpp
[modify] https://crrev.com/810ca667e4510713cbeb98bb27e4bc41b85686e4/third_party/WebKit/Source/platform/network/ParsedContentTypeTest.cpp

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 18 2017

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

commit 3481dd49b33c060c8262a72b61a0a67b71090540
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Sep 18 17:08:02 2017

Reuse existing ValueToString<float> in test

There is a ValueToString<float> in FloatPolygon to be used by
POD tests so PODIntervalTreeTres doesn't need to define its own.

Noticed in a jumbo build where the two copies collided.

Bug:  745732 
Change-Id: I7dd3cd6429ac9c1357dad8c7e3cf98140a58f9d3
Reviewed-on: https://chromium-review.googlesource.com/671266
Commit-Queue: Daniel Bratell <bratell@opera.com>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502598}
[modify] https://crrev.com/3481dd49b33c060c8262a72b61a0a67b71090540/third_party/WebKit/Source/platform/PODIntervalTreeTest.cpp

Project Member

Comment 14 by bugdroid1@chromium.org, Sep 18 2017

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

commit 85338c7a821e5466ffb057e9386cac9339224bbe
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Sep 18 17:19:31 2017

Rename EXPECT_RECT_EQ to be more unique and accurate

There is an EXPECT_RECT_EQ in //cc/test/geometry_test_utils.h which is
used by other tests. Since that causes problems in jumbo builds and
since it is not an equality comparison, this renames the macro
EXPECT_FLOAT_RECT_NEAR (following a pattern used elsewhere).

Bug:  745732 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I2720f72dbf0b8dac603c19d312109fb83fdfa313
Reviewed-on: https://chromium-review.googlesource.com/671230
Commit-Queue: Daniel Bratell <bratell@opera.com>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502601}
[modify] https://crrev.com/85338c7a821e5466ffb057e9386cac9339224bbe/third_party/WebKit/Source/platform/graphics/paint/GeometryMapperTest.cpp

Project Member

Comment 15 by bugdroid1@chromium.org, Sep 18 2017

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

commit 5a2303efead1c274b78ce075d2e1cc3dc77c3245
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Sep 18 17:23:07 2017

Fix header include guard for MockImageDecoder.h

The include guard was missing the actual define.

Bug:  745732 
Change-Id: Id6435955c9011295b9583d29df6f45d008f0e192
Reviewed-on: https://chromium-review.googlesource.com/671233
Commit-Queue: Daniel Bratell <bratell@opera.com>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502602}
[modify] https://crrev.com/5a2303efead1c274b78ce075d2e1cc3dc77c3245/third_party/WebKit/Source/platform/graphics/test/MockImageDecoder.h

Project Member

Comment 16 by bugdroid1@chromium.org, Sep 18 2017

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

commit 51aa97bde5c6a02e0c30cdbf509465a336f069d1
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Sep 18 21:03:14 2017

Use differently named test classes in blink/platform tests

Both PaintChunkerTest and PaintControllerTest have test classes named
TestDisplayItem. These collide in jumbo builds so this patch renames
one of them TestDisplayItemGeneric to be unique inside blink/platform.

Bug:  745732 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I3cdd3b21f5523ef54fe3fbe29f4f7dc0607d7874
Reviewed-on: https://chromium-review.googlesource.com/671423
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#502675}
[modify] https://crrev.com/51aa97bde5c6a02e0c30cdbf509465a336f069d1/third_party/WebKit/Source/platform/graphics/paint/PaintChunkerTest.cpp
[modify] https://crrev.com/51aa97bde5c6a02e0c30cdbf509465a336f069d1/third_party/WebKit/Source/platform/graphics/paint/PaintControllerTest.cpp

Project Member

Comment 17 by bugdroid1@chromium.org, Sep 19 2017

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

commit 571a4e12b48a78b10ac6014ee084c972b43dd1db
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Sep 19 09:13:05 2017

Rename InsertionAndDeletionTest to avoid collisions with other tests

There are InsertionAndDeletionTest functions in both PODIntervalTreeTest
and PODRedBlackTreeTest and in jumbo builds they collide. This patch
renames one of them TreeInsertionAndDeletionTest.

Bug:  745732 
Change-Id: Ieba300f4532695060250cb70f88f53713158ccf1
Reviewed-on: https://chromium-review.googlesource.com/671228
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#502808}
[modify] https://crrev.com/571a4e12b48a78b10ac6014ee084c972b43dd1db/third_party/WebKit/Source/platform/PODIntervalTreeTest.cpp

Project Member

Comment 18 by bugdroid1@chromium.org, Sep 19 2017

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

commit b98f990c5330501faf14d9aa811a38a3b37af8de
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Sep 19 09:15:12 2017

Add prefix to test class names to make them more unique.

Jumbo didn't like that there were two TestExtraData classes in
platform/exported tests so this adds appropriate prefixes to them.

Bug:  745732 
Change-Id: I9286225bf37dd39342e28aec8ddcca3da6504063
Reviewed-on: https://chromium-review.googlesource.com/671270
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#502811}
[modify] https://crrev.com/b98f990c5330501faf14d9aa811a38a3b37af8de/third_party/WebKit/Source/platform/exported/WebURLRequestTest.cpp
[modify] https://crrev.com/b98f990c5330501faf14d9aa811a38a3b37af8de/third_party/WebKit/Source/platform/exported/WebURLResponseTest.cpp

Project Member

Comment 19 by bugdroid1@chromium.org, Sep 19 2017

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

commit f385f3ca2721baba103d62a07f9634d9f1a06a81
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Sep 19 09:15:42 2017

Use differently named test classes in blink/platform tests

Both PODFreeListArenaTest and PODArenaTest had local test classes
naemd TestClass and those collided in jumbo build. This patch
changes the colliding names to be more unique inside blink/platform.

Bug:  745732 
Change-Id: I8b31e62125c0e0eb84103d91f3e28dd9decd3f5f
Reviewed-on: https://chromium-review.googlesource.com/671265
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#502812}
[modify] https://crrev.com/f385f3ca2721baba103d62a07f9634d9f1a06a81/third_party/WebKit/Source/platform/PODArenaTest.cpp

Project Member

Comment 20 by bugdroid1@chromium.org, Sep 21 2017

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

commit 92a7d69784fc485d61c82af3d435ebf873084865
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Sep 21 08:33:08 2017

Rename MockScrollableArea to be more unique

There is a MockScrollableArea defined in ScrollbarTestSuite.h so if
the compiler sees that function then you get compilation errors. That
happens in jumbo builds so to avoid this problem this patch renames
the function MockScrollableAreaForAnimatorTest which follows a pattern
used elsewhere.

Bug:  745732 
Change-Id: Ia264efac21e4c358e50aa90ec666a95d32366642
Reviewed-on: https://chromium-review.googlesource.com/671424
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#503390}
[modify] https://crrev.com/92a7d69784fc485d61c82af3d435ebf873084865/third_party/WebKit/Source/platform/scroll/ScrollAnimatorTest.cpp

Project Member

Comment 21 by bugdroid1@chromium.org, Sep 22 2017

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

commit 1749928dadb25433cc65811e06e55929f3f2aa99
Author: Daniel Bratell <bratell@opera.com>
Date: Fri Sep 22 16:01:55 2017

Rename GarbageCollectedHolder to avoid clash

There is already a GarbageCollectedHolder class in webkit_unit_tests
so this renames the second one GarbageCollectedHolderForToV8Test 
to avoid collisions in jumbo builds.

Bug:  745732 
Change-Id: Idc625d35fd3590292d93e1a3a0b685db12bd7368
Reviewed-on: https://chromium-review.googlesource.com/677397
Reviewed-by: Jens Widell <jl@opera.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#503753}
[modify] https://crrev.com/1749928dadb25433cc65811e06e55929f3f2aa99/third_party/WebKit/Source/bindings/core/v8/ToV8Test.cpp

Project Member

Comment 22 by bugdroid1@chromium.org, Sep 26 2017

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

commit f3ea5c389b36f7e6b09f4ac8abf5e6bbba0c292a
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Sep 26 07:26:51 2017

Deduplicate a testing template in v8 bindings test.

The two implementations of ToV8 for testing collided in jumbo builds
so to avoid having to exclude some files in bindings tests from jumbo
this patch moves the template to a shared header.

Bug:  745732 
Change-Id: Iaa7cf7e19d987e7a048fc606eafaedb49606959a
Reviewed-on: https://chromium-review.googlesource.com/677394
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Jens Widell <jl@opera.com>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#504304}
[modify] https://crrev.com/f3ea5c389b36f7e6b09f4ac8abf5e6bbba0c292a/third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImplTest.cpp
[add] https://crrev.com/f3ea5c389b36f7e6b09f4ac8abf5e6bbba0c292a/third_party/WebKit/Source/bindings/core/v8/ToV8ForTesting.h
[modify] https://crrev.com/f3ea5c389b36f7e6b09f4ac8abf5e6bbba0c292a/third_party/WebKit/Source/bindings/core/v8/V8BindingTest.cpp

Project Member

Comment 23 by bugdroid1@chromium.org, Sep 26 2017

Project Member

Comment 24 by bugdroid1@chromium.org, Sep 26 2017

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

commit e303b7d801f7e81727da4c53f35a4147ad87b675
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Sep 26 15:55:23 2017

Deduplicate some testing helper functions in v8 bindings test.

V8ScriptValueSerializerTest and V8ScriptValueSerializerTestForModules
had a few methods in common. This patch makes those methods be
shared, or in the case or non-identical methods, renamed to not collide
in jumbo builds.

Bug:  745732 
Change-Id: Ia51d9f8dba1910902465dc4cffc193031b47ee3f
Reviewed-on: https://chromium-review.googlesource.com/677446
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#504379}
[modify] https://crrev.com/e303b7d801f7e81727da4c53f35a4147ad87b675/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializerTest.cpp
[add] https://crrev.com/e303b7d801f7e81727da4c53f35a4147ad87b675/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializerTest.h
[modify] https://crrev.com/e303b7d801f7e81727da4c53f35a4147ad87b675/third_party/WebKit/Source/bindings/modules/v8/serialization/V8ScriptValueSerializerForModulesTest.cpp

Project Member

Comment 25 by bugdroid1@chromium.org, Sep 27 2017

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

commit c7815a127f7842663ba65f6b0aa03b6c73f3f216
Author: Daniel Bratell <bratell@opera.com>
Date: Wed Sep 27 14:09:06 2017

Adding namespaces to prevent name collisions in jumbo builds

Several test classes in scheduler use the same name. As an alternative
to renaming them, or some of them, use unique namespaces.

Bug:  745732 
Change-Id: I00bbbc957c58c04013a77b89b2b0cdf7c0c6321d
Reviewed-on: https://chromium-review.googlesource.com/682055
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504655}
[modify] https://crrev.com/c7815a127f7842663ba65f6b0aa03b6c73f3f216/third_party/WebKit/Source/platform/scheduler/base/task_queue.h
[modify] https://crrev.com/c7815a127f7842663ba65f6b0aa03b6c73f3f216/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.h
[modify] https://crrev.com/c7815a127f7842663ba65f6b0aa03b6c73f3f216/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
[modify] https://crrev.com/c7815a127f7842663ba65f6b0aa03b6c73f3f216/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler_unittest.cc
[modify] https://crrev.com/c7815a127f7842663ba65f6b0aa03b6c73f3f216/third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl_unittest.cc
[modify] https://crrev.com/c7815a127f7842663ba65f6b0aa03b6c73f3f216/third_party/WebKit/Source/platform/scheduler/renderer/web_view_scheduler_impl_unittest.cc

Project Member

Comment 27 by bugdroid1@chromium.org, Sep 28 2017

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

commit 65fbf3e9b72ee2ba7fd0f01484a036dd6a3f0f93
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Sep 28 20:49:14 2017

Adding namespaces to prevent name collisions in jumbo builds

Several test classes in scheduler use the same name. As an alternative
to renaming them, or some of them, use unique namespaces. This
solves problems with MockTask and MockObserver.

Bug:  745732 
Change-Id: I56da313dbbfb116d28d45ccd5f261cdfbf48d188
Reviewed-on: https://chromium-review.googlesource.com/687674
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505147}
[modify] https://crrev.com/65fbf3e9b72ee2ba7fd0f01484a036dd6a3f0f93/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
[modify] https://crrev.com/65fbf3e9b72ee2ba7fd0f01484a036dd6a3f0f93/third_party/WebKit/Source/platform/scheduler/base/task_queue_selector_unittest.cc
[modify] https://crrev.com/65fbf3e9b72ee2ba7fd0f01484a036dd6a3f0f93/third_party/WebKit/Source/platform/scheduler/child/scheduler_helper_unittest.cc
[modify] https://crrev.com/65fbf3e9b72ee2ba7fd0f01484a036dd6a3f0f93/third_party/WebKit/Source/platform/scheduler/child/webthread_impl_for_worker_scheduler_unittest.cc
[modify] https://crrev.com/65fbf3e9b72ee2ba7fd0f01484a036dd6a3f0f93/third_party/WebKit/Source/platform/scheduler/renderer/render_widget_signals_unittest.cc
[modify] https://crrev.com/65fbf3e9b72ee2ba7fd0f01484a036dd6a3f0f93/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler_unittest.cc
[modify] https://crrev.com/65fbf3e9b72ee2ba7fd0f01484a036dd6a3f0f93/third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl_unittest.cc
[modify] https://crrev.com/65fbf3e9b72ee2ba7fd0f01484a036dd6a3f0f93/third_party/WebKit/Source/platform/scheduler/renderer/web_view_scheduler_impl_unittest.cc
[modify] https://crrev.com/65fbf3e9b72ee2ba7fd0f01484a036dd6a3f0f93/third_party/WebKit/Source/platform/scheduler/renderer/webthread_impl_for_renderer_scheduler_unittest.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Sep 29 2017

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

commit 251084a843fcc2184854c4f2905e3b1b26600868
Author: Daniel Bratell <bratell@opera.com>
Date: Fri Sep 29 09:44:54 2017

Adding namespaces to prevent NopTask collisions in jumbo builds

Several test classes in scheduler have implemented NopTask and
this patch uses unique namespaces to prevent them from clashing
in jumbo builds.

Bug:  745732 
Change-Id: I8c3ecc80ab0696fbe4634d330dfe1e942dbcfa8b
Reviewed-on: https://chromium-review.googlesource.com/690345
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#505338}
[modify] https://crrev.com/251084a843fcc2184854c4f2905e3b1b26600868/third_party/WebKit/Source/platform/scheduler/child/idle_helper.h
[modify] https://crrev.com/251084a843fcc2184854c4f2905e3b1b26600868/third_party/WebKit/Source/platform/scheduler/child/idle_helper_unittest.cc
[modify] https://crrev.com/251084a843fcc2184854c4f2905e3b1b26600868/third_party/WebKit/Source/platform/scheduler/renderer/auto_advancing_virtual_time_domain_unittest.cc
[modify] https://crrev.com/251084a843fcc2184854c4f2905e3b1b26600868/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.h
[modify] https://crrev.com/251084a843fcc2184854c4f2905e3b1b26600868/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl_unittest.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Sep 29 2017

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

commit a4cf9021d9217b688e27589f4586a8c38b93baa2
Author: Daniel Bratell <bratell@opera.com>
Date: Fri Sep 29 14:13:41 2017

Adding namespaces to prevent AppendToVectorTestTask clashes

Several test classes in scheduler have implemented
AppendToVectorTestTask and this patch gives those unique namespaces
to prevent them from clashing in jumbo builds.

Bug:  745732 
Change-Id: I265c5507cbcc0f934f5904cc62033443442163f9
Reviewed-on: https://chromium-review.googlesource.com/692018
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#505364}
[modify] https://crrev.com/a4cf9021d9217b688e27589f4586a8c38b93baa2/third_party/WebKit/Source/platform/scheduler/child/worker_global_scope_scheduler_unittest.cc
[modify] https://crrev.com/a4cf9021d9217b688e27589f4586a8c38b93baa2/third_party/WebKit/Source/platform/scheduler/child/worker_scheduler_impl_unittest.cc

Project Member

Comment 30 by bugdroid1@chromium.org, Oct 2 2017

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

commit 894d8f6db291cf3579de30bc192d5e504bacb67b
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Oct 02 15:03:48 2017

Move shared test classes in wtf tests to a shared file

HashSet, HashMap and ListHashSet, Vector and Deque tests have the
same test classes: MoveOnly, CountCopy, DestructCounter, WrappedInt,
LivenessCounter, ...

This both deduplicates the code and prevents symbol collisions in
jumbo builds.

Bug:  745732 
Change-Id: I19874a89ba4933260f3c80727ac5ba1160831ad1
Reviewed-on: https://chromium-review.googlesource.com/678934
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505614}
[modify] https://crrev.com/894d8f6db291cf3579de30bc192d5e504bacb67b/third_party/WebKit/Source/platform/wtf/DequeTest.cpp
[modify] https://crrev.com/894d8f6db291cf3579de30bc192d5e504bacb67b/third_party/WebKit/Source/platform/wtf/FunctionalTest.cpp
[modify] https://crrev.com/894d8f6db291cf3579de30bc192d5e504bacb67b/third_party/WebKit/Source/platform/wtf/HashMapTest.cpp
[modify] https://crrev.com/894d8f6db291cf3579de30bc192d5e504bacb67b/third_party/WebKit/Source/platform/wtf/HashSetTest.cpp
[modify] https://crrev.com/894d8f6db291cf3579de30bc192d5e504bacb67b/third_party/WebKit/Source/platform/wtf/ListHashSetTest.cpp
[modify] https://crrev.com/894d8f6db291cf3579de30bc192d5e504bacb67b/third_party/WebKit/Source/platform/wtf/VectorTest.cpp
[add] https://crrev.com/894d8f6db291cf3579de30bc192d5e504bacb67b/third_party/WebKit/Source/platform/wtf/WTFTestHelper.h

Project Member

Comment 31 by bugdroid1@chromium.org, Oct 5 2017

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

commit 5cedd9c40b563e30ce7c82e855fe30295bf8dbf4
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Oct 05 09:06:24 2017

Avoid using classes named Function in blink

There is a Function template in blink's wtf library and it is easy
to get mixed up in it if you try to use the symbol "Function"
elsewhere in blink.

In particular you can easily get an include of
platform/wtf/Functional.h (which exports WTF::Function to the global
scope) in jumbo builds:

Bug:  745732 
Change-Id: Ic4f4a02e1aacc5819ce824b94e284bfc6caa964d
Reviewed-on: https://chromium-review.googlesource.com/702236
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#506686}
[modify] https://crrev.com/5cedd9c40b563e30ce7c82e855fe30295bf8dbf4/third_party/WebKit/Source/bindings/core/v8/ScriptPromiseTest.cpp

Project Member

Comment 32 by bugdroid1@chromium.org, Oct 10 2017

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

commit a726bfa56c224c00f7e31ab5a61c849f99f92b02
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Oct 10 15:33:02 2017

Remove duplicate helper class DestructCounter

There is already a DestructCounter in WTFTestHelpers.h. Having two
complicates jumbo builds and since they are identical, this one
can just be removed.

Bug:  745732 
Change-Id: Ieca99932ff66f90d06b3fa8e21fa5b51da0d2f5c
Reviewed-on: https://chromium-review.googlesource.com/708743
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#507675}
[modify] https://crrev.com/a726bfa56c224c00f7e31ab5a61c849f99f92b02/third_party/WebKit/Source/platform/wtf/VectorTest.cpp

Project Member

Comment 33 by bugdroid1@chromium.org, Oct 13 2017

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

commit 1e7338af076315721c9d4852e8ecc5e9d28652b5
Author: Daniel Bratell <bratell@opera.com>
Date: Fri Oct 13 19:56:42 2017

Jumbo for Blink core/editing unit_tests (-8.5 CPU minutes)

Final cleanup of parts of Blink core that had not been jumbified.
Some of the tests in core/editing are very similar so they have
been given custom namespaces.

Bug:  745732 
Change-Id: I4db280458053cb31461247345cc2613b38eae884
Reviewed-on: https://chromium-review.googlesource.com/718207
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#508791}
[modify] https://crrev.com/1e7338af076315721c9d4852e8ecc5e9d28652b5/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/1e7338af076315721c9d4852e8ecc5e9d28652b5/third_party/WebKit/Source/core/editing/LayoutSelectionTest.cpp
[modify] https://crrev.com/1e7338af076315721c9d4852e8ecc5e9d28652b5/third_party/WebKit/Source/core/editing/state_machines/BackspaceStateMachineTest.cpp
[modify] https://crrev.com/1e7338af076315721c9d4852e8ecc5e9d28652b5/third_party/WebKit/Source/core/editing/state_machines/BackwardCodePointStateMachineTest.cpp
[modify] https://crrev.com/1e7338af076315721c9d4852e8ecc5e9d28652b5/third_party/WebKit/Source/core/editing/state_machines/BackwardGraphemeBoundaryStateMachineTest.cpp
[modify] https://crrev.com/1e7338af076315721c9d4852e8ecc5e9d28652b5/third_party/WebKit/Source/core/editing/state_machines/ForwardGraphemeBoundaryStateMachineTest.cpp

Comment 35 by brat...@opera.com, Nov 20 2017

Status: Fixed (was: Assigned)
Even if something else might turn up in the future, most of the code is fixed now.

For the future, wrapping the code in testcase files in a namespace named for the file seems to be a smoother and more reliable way to avoid conflicts between testcases than to rename things.
Project Member

Comment 36 by bugdroid1@chromium.org, Nov 21 2017

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

commit 1112a3a1a8ba9eedecf51ffeefb2990caad27070
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Nov 21 13:48:01 2017

Wrap test in namespace to avoid jumbo collisions

Tests often use the same symbols because of copy/paste or
the repetitive nature of testing and

flat_tree_traversal_ng_test.cc has a TestCommonAncestor, same as
FlatTreeTraversalTest.cpp. Since tests don't export any symbols
wrapping them in a namespace is a simple way to avoid jumbo
clashes.

Bug:  745732 
Change-Id: I91f7e732a28d67ebe5495791cb632a06c87534cc
Reviewed-on: https://chromium-review.googlesource.com/781779
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#518236}
[modify] https://crrev.com/1112a3a1a8ba9eedecf51ffeefb2990caad27070/third_party/WebKit/Source/core/dom/ng/flat_tree_traversal_ng_test.cc

Project Member

Comment 37 by bugdroid1@chromium.org, Jan 2 2018

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

commit e9889d5c571cbc055685c9fff115c088a564ee95
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Jan 02 22:26:53 2018

Changed some test code to avoid jumbo problems

In certain non-standard jumbo configurations (very large
jumbo units) there were some symbol clashes that do not
appear in normal jumbo builds. This resolves them by
renaming one test class to better match the test, and by
wrapping the other test in a custom namespace as done in other
places in blink tests.

Bug:  745732 
Change-Id: Ic0388e9abd19b24366133c577059f59ef63a6b6b
Reviewed-on: https://chromium-review.googlesource.com/847009
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#526543}
[modify] https://crrev.com/e9889d5c571cbc055685c9fff115c088a564ee95/third_party/WebKit/Source/core/exported/PrerenderingTest.cpp
[modify] https://crrev.com/e9889d5c571cbc055685c9fff115c088a564ee95/third_party/WebKit/Source/core/loader/resource/MultipartImageResourceParserTest.cpp

Project Member

Comment 38 by bugdroid1@chromium.org, Jan 3 2018

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

commit 8eec813f79843ed6dae826c592ed24fe9431e9a6
Author: Daniel Bratell <bratell@opera.com>
Date: Wed Jan 03 14:49:47 2018

Avoid collision between two ScriptExecutionCallbackHelper

ScriptExecutionCallbackHelper is the name of two different
helper functions which in some non-standard jumbo configurations
(huuuge jumbo chunks) end up in the same translation unit. This
patch changes the scope of one of the classes to not clash.

Bug:  745732 
Change-Id: Ifed9bcd247c43afb110c9b008062383a2a7d74df
Reviewed-on: https://chromium-review.googlesource.com/848835
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#526690}
[modify] https://crrev.com/8eec813f79843ed6dae826c592ed24fe9431e9a6/third_party/WebKit/Source/core/scheduler/VirtualTimeTest.cpp

Sign in to add a comment