New issue
Advanced search Search tips

Issue 590749 link

Starred by 1 user
Status: Fixed
Owner:
jbroman@chromium.org
Closed: Mar 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 412064



Sign in to add a comment

Blink should not link unit tests into blink_web component in component build

Project Member Reported by jbroman@chromium.org, Feb 29 2016 
Now that core and modules are linked properly, it's possible to unwind this hack and link the unit tests into the webkit_unit_tests binary instead.

This means, among other things, that one can (in a component build) build targets that depend on Blink without also pulling in a heap of unit test code, gtest, etc.

Comment 1 by jbroman@chromium.org, Feb 29 2016

Components: -Blink>Architecture Blink>Tools>Test
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 2 2016 

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

commit 737c4d032a36133f1045a3e399c922a948b29f85
Author: jbroman <jbroman@chromium.org>
Date: Wed Mar 02 18:18:03 2016

Extract webkit_unit_tests from blink_web component.

- Make BLINK_WEB_IMPLEMENTATION a define exported by the build system, like most
  of the others, rather than defined in a header.
- Create a WebExport.h header (similar to CoreExport.h, etc.) which can be used
  to export symbols from blink_web to webkit_unit_tests.
- Export symbols used from unit tests that aren't yet exported.
- Remove build logic to compile unit tests into blink_web component in component
  (shared_library) builds.
- Handle MSVC C4275 issues: base classes are either exported or, if they are
  clearly pure-interface or pure-data, marked to suppress the warning.
- Adds an explicit web -> base dependency (due to platform using base, and gyp
  not propagating this). This is similar to the equivalent in core.
- Unexports WebRTCDataChannelHandlerClient. It's not exported from blink_web,
  and only happened to work before because one of the modules unit tests included
  the header, and that was previously linked into blink_web.

This makes the blink_web component substantially smaller, as it no
longer pulls in unit tests, gtest, and so on. It also unblocks
removal of several hacks that existed to accommodate the previous
setup.

BUG= 590749 

Review URL: https://codereview.chromium.org/1676083002

Cr-Commit-Position: refs/heads/master@{#378773}

[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/core/plugins/PluginView.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/modules/BUILD.gn
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/modules/mediastream/RTCDataChannel.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/modules/modules.gyp
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/BUILD.gn
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/ChromeClientImpl.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/ExternalPopupMenu.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/FindInPageCoordinates.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/LinkHighlightImpl.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/PageOverlay.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/PageWidgetDelegate.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/TextFinder.h
[add] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/WebExport.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/WebFrameImplBase.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/WebInputEventConversion.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/WebPluginContainerImpl.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/WebRemoteFrameImpl.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/WebSettingsImpl.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/WebViewImpl.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/web.gyp
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/web.gypi
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/web/web_tests.gyp
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/Source/wtf/Compiler.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/public/platform/WebCommon.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/public/platform/WebRTCDataChannelHandlerClient.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/public/web/WebDocument.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/public/web/WebElement.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/public/web/WebHistoryItem.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/public/web/WebNode.h
[modify] https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85/third_party/WebKit/public/web/WebTestingSupport.h
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 2 2016 

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

commit 4a75239f5b05e0b031dcb94eac91cc58a6c9e584
Author: dimich <dimich@chromium.org>
Date: Wed Mar 02 22:06:15 2016

Revert of Extract webkit_unit_tests from blink_web component. (patchset #16 id:300001 of https://codereview.chromium.org/1676083002/ )

Reason for revert:
Broke compile: https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win%20non-Oilpan%20%28dbg%29

Original issue's description:
> Extract webkit_unit_tests from blink_web component.
>
> - Make BLINK_WEB_IMPLEMENTATION a define exported by the build system, like most
>   of the others, rather than defined in a header.
> - Create a WebExport.h header (similar to CoreExport.h, etc.) which can be used
>   to export symbols from blink_web to webkit_unit_tests.
> - Export symbols used from unit tests that aren't yet exported.
> - Remove build logic to compile unit tests into blink_web component in component
>   (shared_library) builds.
> - Handle MSVC C4275 issues: base classes are either exported or, if they are
>   clearly pure-interface or pure-data, marked to suppress the warning.
> - Adds an explicit web -> base dependency (due to platform using base, and gyp
>   not propagating this). This is similar to the equivalent in core.
> - Unexports WebRTCDataChannelHandlerClient. It's not exported from blink_web,
>   and only happened to work before because one of the modules unit tests included
>   the header, and that was previously linked into blink_web.
>
> This makes the blink_web component substantially smaller, as it no
> longer pulls in unit tests, gtest, and so on. It also unblocks
> removal of several hacks that existed to accommodate the previous
> setup.
>
> BUG= 590749 
>
> Committed: https://crrev.com/737c4d032a36133f1045a3e399c922a948b29f85
> Cr-Commit-Position: refs/heads/master@{#378773}

TBR=haraken@chromium.org,pdr@chromium.org,jbroman@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 590749 

Review URL: https://codereview.chromium.org/1756343002

Cr-Commit-Position: refs/heads/master@{#378853}

[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/core/plugins/PluginView.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/modules/BUILD.gn
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/modules/mediastream/RTCDataChannel.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/modules/modules.gyp
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/BUILD.gn
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/ChromeClientImpl.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/ExternalPopupMenu.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/FindInPageCoordinates.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/LinkHighlightImpl.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/PageOverlay.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/PageWidgetDelegate.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/TextFinder.h
[delete] https://crrev.com/f3516738fbd701a28abd8995ad17d3d143b6034f/third_party/WebKit/Source/web/WebExport.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/WebFrameImplBase.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/WebInputEventConversion.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/WebPluginContainerImpl.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/WebRemoteFrameImpl.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/WebSettingsImpl.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/WebViewImpl.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/web.gyp
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/web.gypi
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/web/web_tests.gyp
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/Source/wtf/Compiler.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/public/platform/WebCommon.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/public/platform/WebRTCDataChannelHandlerClient.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/public/web/WebDocument.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/public/web/WebElement.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/public/web/WebHistoryItem.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/public/web/WebNode.h
[modify] https://crrev.com/4a75239f5b05e0b031dcb94eac91cc58a6c9e584/third_party/WebKit/public/web/WebTestingSupport.h
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 3 2016 

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

commit 401833786b54e6b043f2c9b72f97b8fa377156c3
Author: jbroman <jbroman@chromium.org>
Date: Thu Mar 03 04:19:45 2016

Extract webkit_unit_tests from blink_web component.

- Make BLINK_WEB_IMPLEMENTATION a define exported by the build system, like most
  of the others, rather than defined in a header.
- Create a WebExport.h header (similar to CoreExport.h, etc.) which can be used
  to export symbols from blink_web to webkit_unit_tests.
- Export symbols used from unit tests that aren't yet exported.
- Remove build logic to compile unit tests into blink_web component in component
  (shared_library) builds.
- Handle MSVC C4275 issues: base classes are either exported or, if they are
  clearly pure-interface or pure-data, marked to suppress the warning.
- Adds an explicit web -> base dependency (due to platform using base, and gyp
  not propagating this). This is similar to the equivalent in core.
- Unexports WebRTCDataChannelHandlerClient. It's not exported from blink_web,
  and only happened to work before because one of the modules unit tests included
  the header, and that was previously linked into blink_web.

This makes the blink_web component substantially smaller, as it no
longer pulls in unit tests, gtest, and so on. It also unblocks
removal of several hacks that existed to accommodate the previous
setup.

BUG= 590749 

Review URL: https://codereview.chromium.org/1676083002

Cr-Commit-Position: refs/heads/master@{#378954}

[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/core/plugins/PluginView.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/modules/BUILD.gn
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/modules/mediastream/RTCDataChannel.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/modules/modules.gyp
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/BUILD.gn
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/ChromeClientImpl.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/ExternalPopupMenu.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/FindInPageCoordinates.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/LinkHighlightImpl.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/PageOverlay.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/PageWidgetDelegate.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/TextFinder.h
[add] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/WebExport.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/WebFrameImplBase.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/WebInputEventConversion.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/WebPluginContainerImpl.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/WebRemoteFrameImpl.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/WebSettingsImpl.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/WebViewImpl.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/web.gyp
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/web.gypi
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/web/web_tests.gyp
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/Source/wtf/Compiler.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/public/platform/WebCommon.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/public/platform/WebRTCDataChannelHandlerClient.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/public/web/WebDocument.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/public/web/WebElement.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/public/web/WebHistoryItem.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/public/web/WebNode.h
[modify] https://crrev.com/401833786b54e6b043f2c9b72f97b8fa377156c3/third_party/WebKit/public/web/WebTestingSupport.h
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 3 2016 

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

commit 7080b7b60fbde15a4b1cb8e61fbce5476c079b82
Author: jbroman <jbroman@chromium.org>
Date: Thu Mar 03 14:44:29 2016

Merge WebUnitTests.cpp into RunAllTests.cpp.

Blink unit test code is no longer linked into blink_web in component builds:
  https://chromium.googlesource.com/chromium/src.git/+/737c4d03

Now that these are always both in the webkit_unit_tests binary, there is no
need to separate them. Merge them together into a simpler, cohesive
RunAllTests.cpp with the same behavior.

BUG= 590749 

Review URL: https://codereview.chromium.org/1758063002

Cr-Commit-Position: refs/heads/master@{#379006}

[modify] https://crrev.com/7080b7b60fbde15a4b1cb8e61fbce5476c079b82/third_party/WebKit/Source/web/tests/DEPS
[modify] https://crrev.com/7080b7b60fbde15a4b1cb8e61fbce5476c079b82/third_party/WebKit/Source/web/tests/RunAllTests.cpp
[delete] https://crrev.com/444b1055212b09ec5fbd6126e2c3610b475655a6/third_party/WebKit/Source/web/tests/WebUnitTests.cpp
[delete] https://crrev.com/444b1055212b09ec5fbd6126e2c3610b475655a6/third_party/WebKit/Source/web/tests/WebUnitTests.h
[modify] https://crrev.com/7080b7b60fbde15a4b1cb8e61fbce5476c079b82/third_party/WebKit/Source/web/web.gypi
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 3 2016 

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

commit d2f87840e21d49e5580d58dc0ecbf26bf0802a47
Author: jbroman <jbroman@chromium.org>
Date: Thu Mar 03 16:41:47 2016

[GN] Set testonly unconditionally.

Blink unit test code is no longer linked into blink_web in component builds:
  https://chromium.googlesource.com/chromium/src.git/+/737c4d03

Thus the hack to only mark targets as testonly in non-component builds is no
longer required, and `gn check` continues to pass in component builds.

BUG= 412064 , 590749 

Review URL: https://codereview.chromium.org/1758653004

Cr-Commit-Position: refs/heads/master@{#379020}

[modify] https://crrev.com/d2f87840e21d49e5580d58dc0ecbf26bf0802a47/base/test/BUILD.gn
[modify] https://crrev.com/d2f87840e21d49e5580d58dc0ecbf26bf0802a47/build/secondary/testing/gmock/BUILD.gn
[modify] https://crrev.com/d2f87840e21d49e5580d58dc0ecbf26bf0802a47/build/secondary/testing/gtest/BUILD.gn
[modify] https://crrev.com/d2f87840e21d49e5580d58dc0ecbf26bf0802a47/third_party/WebKit/Source/platform/BUILD.gn

Comment 7 by jbroman@chromium.org, Mar 3 2016

Status: Fixed (was: Started)

Comment 8 Deleted

Comment 9 Deleted

Comment 10 Deleted

Comment 11 Deleted

Comment 12 Deleted

Comment 13 by sshru...@google.com, Mar 18 2016

Components: -Blink>Tools>Test Blink>ToolsTest
Fixing component name after Monorail migration

Comment 14 by sshru...@google.com, Apr 18 2016

Components: -Blink>ToolsTest Blink>Infra
Blink>ToolsTest renamed to Blink>Infra
►

Sign in to add a comment