New issue
Advanced search Search tips

Issue 3806 link

Starred by 7 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
NextAction: ----
OS: ----
Pri: 2
Type: Enhancement

Blocked on:
issue 7480

Blocking:
issue 6829
issue 3380



Sign in to add a comment

Remove dependencies from rtc_base that are not needed for engine code

Project Member Reported by andrew@webrtc.org, Sep 11 2014

Issue description

We would like rtc_base to be used across webrtc (aka rtc/media or engine code) and libjingle. It currently drags in a bunch of dependencies unneeded by the engine code.

The root of the problem appears to be that MessageQueue depends on a socket server. (And since common.h -> logging.h -> thread.h -> messagequeue.h, this
dependency spreads quickly.) There may be other routes as well.
 
Project Member

Comment 1 by andrew@webrtc.org, Sep 11 2014

Cc: -henrike@webrtc.org andrew@webrtc.org
Owner: henrike@webrtc.org
Henrik, assigning to you for now, as I assume this will be part of uniting rtc_base and system_wrappers.

Comment 2 by hellner@google.com, Sep 11 2014

Blocking: webrtc:3380
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 16 2014

The following revision refers to this bug:
  http://code.google.com/p/webrtc/source/detail?r=7188

------------------------------------------------------------------
r7188 | andrew@webrtc.org | 2014-09-16T01:03:29.166569Z

Changed paths:
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/base/timeutils.cc&spec=svn7188&r_previous=7187&r=7188&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/modules/video_capture/video_capture.gypi&spec=svn7188&r_previous=7187&r=7188&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/base/base.gyp&spec=svn7188&r_previous=7187&r=7188&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/base/stringencode.cc&spec=svn7188&r_previous=7187&r=7188&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/talk/app/webrtc/statstypes.h&spec=svn7188&r_previous=7187&r=7188&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/base/stringencode.h&spec=svn7188&r_previous=7187&r=7188&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/base/stringutils.cc&spec=svn7188&r_previous=7187&r=7188&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/modules/desktop_capture/BUILD.gn&spec=svn7188&r_previous=7187&r=7188&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/base/window.h&spec=svn7188&r_previous=7187&r=7188&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/modules/desktop_capture/desktop_capture.gypi&spec=svn7188&r_previous=7187&r=7188&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/system_wrappers/BUILD.gn&spec=svn7188&r_previous=7187&r=7188&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/base/BUILD.gn&spec=svn7188&r_previous=7187&r=7188&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/system_wrappers/source/system_wrappers.gyp&spec=svn7188&r_previous=7187&r=7188&format=side

Add a target for the approved subset of rtc_base.

rtc_base drags in a bunch of unwieldly dependencies (e.g. nss and
json) not required for standalone webrtc (aka rtc/media). The root of
the problem appears to be that MessageQueue depends on a socket server.
(And since common.h -> logging.h -> thread.h -> messagequeue.h, this
dependency spreads quickly.)

This starts a new target for a "purified" subset of rtc_base. It adds
the files which are already being used, replacing the use of common.h
with checks.h. desktop_capture is a lost cause, and retains its
dependency on the full rtc_base.

The hope is that as additional components are desired they will be
cleaned and added to rtc_base_approved.

BUG=3806
R=andresp@webrtc.org, henrike@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/22649004
-----------------------------------------------------------------
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 16 2014

The following revision refers to this bug:
  http://code.google.com/p/webrtc/source/detail?r=7192

------------------------------------------------------------------
r7192 | kjellander@webrtc.org | 2014-09-16T11:16:12.084408Z

Changed paths:
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/base/BUILD.gn&spec=svn7192&r_previous=7191&r=7192&format=side

Fix GN for rtc_base_approved target.

In https://webrtc-codereview.appspot.com/22649004
a new target was introduced that duplicated some
source files, breaking the bots in
http://build.chromium.org/p/chromium.webrtc.fyi/waterfall

This updates the GN config to also remove them from
the target where they were moved from in base.gyp.

BUG=3806
TESTED=Trybots + Running GN in a Chromium checkout with
src/third_party/webrtc symlinked to the WebRTC checkout
with this CL applied + passing compile step.

R=perkj@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/23669004
-----------------------------------------------------------------
Project Member

Comment 5 by juberti@webrtc.org, Sep 17 2014

Cc: pthatcher@webrtc.org
Is there a plan for hacking back desktop_capture, as well as splitting off the minimal subset into its own dir?
Project Member

Comment 6 by juberti@webrtc.org, Sep 17 2014

I assume we will want Thread, CriticalSection, Event, and all the other synchronization-related stuff, as well as Logging in the minimal subset. Is anyone keeping track of what we need/want in the subset?
Project Member

Comment 7 by juberti@webrtc.org, Sep 17 2014

Here's the current contents of the approved target:
"checks.cc",
"checks.h",
"exp_filter.cc",
"exp_filter.h",
"md5.cc",
"md5.h",
"md5digest.h",
"stringencode.cc",
"stringencode.h",
"stringutils.cc",
"stringutils.h",
"timeutils.cc",
"timeutils.h",
Project Member

Comment 8 by andrew@webrtc.org, Sep 17 2014

#5: I figured desktop_capture would be invited back to the fold after rtc_base is cleaned. It only depends on macutils.cc, which was too wedded to logging.h to move to rtc_base_approved.

Adding another directory for the minimal subset makes sense.

#6: yes at least that much. I think anything that doesn't drag in the third_party dependencies is OK, but a more careful review is warranted. And no, I don't think anyone's keeping track of it.

Henrik do you want to put together a list of what should exist in the subset?  My short-term goal with rtc_base_approved was just to build a voice engine lib without unneeded dependencies. 

Comment 9 by vrk@webrtc.org, Sep 29 2014

Labels: Area-Internals
Owner: andrew@webrtc.org
Project Member

Comment 11 by andrew@webrtc.org, Feb 3 2015

Cc: kwiberg@webrtc.org
Owner: ----
Status: Available
I'm not actively working on this. Adding Karl who was doing a bit of consolidation between system_wrappers and base.
Project Member

Comment 12 by kjellander@webrtc.org, Mar 2 2016

Cc: tommi@webrtc.org kjellander@webrtc.org
We talked about this when discussing how to proceed with solving  bug 4243  yesterday.

One idea was to just move out everything from webrtc/base that's not in the rtc_base_approved to another location, to make webrtc/base smaller and more core.

The larger rtc_base target is depended upon in the following locations today:

webrtc/base/base_tests.gyp:rtc_base_tests_utils
webrtc/libjingle/xmllite/xmllite.gyp:rtc_xmllite
webrtc/libjingle/xmpp/xmpp.gyp:rtc_xmpp
webrtc/modules/desktop_capture/desktop_capture.gypi:desktop_capture
webrtc/p2p/p2p.gyp:webrtc/p2p/p2p.gyp:libstunprober
webrtc/p2p/p2p.gyp:libstunprober
webrtc/pc/pc.gyp:rtc_pc
webrtc/sound/sound.gyp:rtc_sound
webrtc/test/webrtc_test_common.gyp:webrtc_test_common
webrtc/webrtc_examples.gyp:relayserver
webrtc/webrtc_examples.gyp:stunserver
webrtc/webrtc_examples.gyp:turnserver
webrtc/webrtc_examples.gyp:peerconnection_server
webrtc/webrtc_tests.gypi:rtc_unittests
webrtc/webrtc_tests.gypi:webrtc_nonparallel_tests

So there's still a bit of cleanup to do.
Project Member

Comment 14 by henrika@webrtc.org, Sep 15 2016

Cc: henrika@webrtc.org
Project Member

Comment 15 by bugdroid1@chromium.org, Sep 15 2016

Project Member

Comment 16 by bugdroid1@chromium.org, Sep 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/89fb9201b70616a1c33e277f38bf9367112536e8

commit 89fb9201b70616a1c33e277f38bf9367112536e8
Author: maxmorin <maxmorin@webrtc.org>
Date: Thu Sep 15 08:45:25 2016

Revert of Add arraysize to rtc_base_approved. Remove dependency of audio_device on rtc_base. (patchset #1 id:1 of https://codereview.webrtc.org/2346763002/ )

Reason for revert:
Breaks iOS

Original issue's description:
> Add arraysize to rtc_base_approved. Remove dependency of audio_device on rtc_base.
>
> BUG=webrtc:3806
> NOTRY=True
>
> Committed: https://crrev.com/100c9d02669910bce06099b3cc1eaad60fd661dd
> Cr-Commit-Position: refs/heads/master@{#14223}

TBR=kjellander@webrtc.org,henrika@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:3806

Review-Url: https://codereview.webrtc.org/2340253003
Cr-Commit-Position: refs/heads/master@{#14224}

[modify] https://crrev.com/89fb9201b70616a1c33e277f38bf9367112536e8/webrtc/base/BUILD.gn
[modify] https://crrev.com/89fb9201b70616a1c33e277f38bf9367112536e8/webrtc/base/base.gyp
[modify] https://crrev.com/89fb9201b70616a1c33e277f38bf9367112536e8/webrtc/modules/audio_device/BUILD.gn

Project Member

Comment 17 by bugdroid1@chromium.org, Sep 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/ec62374ccd4aaf1a32bcf3cedb9bfbab6d189c65

commit ec62374ccd4aaf1a32bcf3cedb9bfbab6d189c65
Author: maxmorin <maxmorin@webrtc.org>
Date: Thu Sep 15 12:11:55 2016

Reland of Add arraysize to rtc_base_approved. Remove dependency of audio_device on rtc_base. (patchset #1 id:1 of https://codereview.webrtc.org/2340253003/ )

Reason for revert:
Fix: let audio_device depend on rtc_base on IOS.

Original issue's description:
> Revert of Add arraysize to rtc_base_approved. Remove dependency of audio_device on rtc_base. (patchset #1 id:1 of https://codereview.webrtc.org/2346763002/ )
>
> Reason for revert:
> Breaks iOS
>
> Original issue's description:
> > Add arraysize to rtc_base_approved. Remove dependency of audio_device on rtc_base.
> >
> > BUG=webrtc:3806
> > NOTRY=True
> >
> > Committed: https://crrev.com/100c9d02669910bce06099b3cc1eaad60fd661dd
> > Cr-Commit-Position: refs/heads/master@{#14223}
>
> TBR=kjellander@webrtc.org,henrika@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:3806
>
> Committed: https://crrev.com/89fb9201b70616a1c33e277f38bf9367112536e8
> Cr-Commit-Position: refs/heads/master@{#14224}

TBR=kjellander@webrtc.org,henrika@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOTRY=true
BUG=webrtc:3806

Review-Url: https://codereview.webrtc.org/2340233003
Cr-Commit-Position: refs/heads/master@{#14233}

[modify] https://crrev.com/ec62374ccd4aaf1a32bcf3cedb9bfbab6d189c65/PRESUBMIT.py
[modify] https://crrev.com/ec62374ccd4aaf1a32bcf3cedb9bfbab6d189c65/webrtc/base/BUILD.gn
[modify] https://crrev.com/ec62374ccd4aaf1a32bcf3cedb9bfbab6d189c65/webrtc/base/base.gyp
[modify] https://crrev.com/ec62374ccd4aaf1a32bcf3cedb9bfbab6d189c65/webrtc/modules/audio_device/BUILD.gn
[modify] https://crrev.com/ec62374ccd4aaf1a32bcf3cedb9bfbab6d189c65/webrtc/modules/audio_device/audio_device.gypi

Project Member

Comment 18 by bugdroid1@chromium.org, Dec 6 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/dd87d580e8ffe546bd6e22cbd1522ffe76d675a7

commit dd87d580e8ffe546bd6e22cbd1522ffe76d675a7
Author: zijiehe <zijiehe@chromium.org>
Date: Tue Dec 06 23:04:02 2016

Add File::Open / Create functions to take an rtc::Pathname

When implementing ISOLATED_OUTDIR feature in WebRTC, I found two issues,
1. pathutils and flags are not accessible in testsupport. But both of them are
useful for the feature. Pathname can help to combine path with filename, while
a flag is needed to handle command line parameter.
2. rtc::File cannot accept an rtc::Pathname, which is a little bit inconvenient.

After investigating bug webrtc:3806, flags, pathutils and urlencode are
removed from rtc_base_approved because of the including of common.h. So I
replaced common.h with checks.h, and ASSERT with RTC_DCHECK. flags,
pathutils and urlencode pairs now can be placed into rtc_base_approved to
unblock file.h to include pathutils.h.

Please kindly let me know if you have other concerns about this change.

BUG=webrtc:3806,  webrtc:6732 

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng;master.tryserver.chromium.mac:mac_chromium_rel_ng;master.tryserver.chromium.win:win_chromium_rel_ng;master.tryserver.chromium.android:linux_android_rel_ng

Review-Url: https://codereview.webrtc.org/2533213005
Cr-Commit-Position: refs/heads/master@{#15451}

[modify] https://crrev.com/dd87d580e8ffe546bd6e22cbd1522ffe76d675a7/webrtc/base/BUILD.gn
[modify] https://crrev.com/dd87d580e8ffe546bd6e22cbd1522ffe76d675a7/webrtc/base/file.cc
[modify] https://crrev.com/dd87d580e8ffe546bd6e22cbd1522ffe76d675a7/webrtc/base/file.h
[modify] https://crrev.com/dd87d580e8ffe546bd6e22cbd1522ffe76d675a7/webrtc/base/file_unittest.cc
[modify] https://crrev.com/dd87d580e8ffe546bd6e22cbd1522ffe76d675a7/webrtc/base/fileutils.h
[modify] https://crrev.com/dd87d580e8ffe546bd6e22cbd1522ffe76d675a7/webrtc/base/flags.h
[modify] https://crrev.com/dd87d580e8ffe546bd6e22cbd1522ffe76d675a7/webrtc/base/pathutils.cc
[modify] https://crrev.com/dd87d580e8ffe546bd6e22cbd1522ffe76d675a7/webrtc/base/pathutils.h
[modify] https://crrev.com/dd87d580e8ffe546bd6e22cbd1522ffe76d675a7/webrtc/base/urlencode.cc

Project Member

Comment 19 by kjellander@webrtc.org, Dec 8 2016

Cc: terelius@webrtc.org solenberg@webrtc.org
Edward and I decided to remove the PRESUBMIT rule that disallows depending on rtc_base since it's blocking enabling GN check across our repo ( bug 6806 ).
We're doing that in https://codereview.webrtc.org/2558813003

Having the (previously hidden) dependencies to rtc_base being more visible will make it easier to clean up the dependencies. We thought this is the right call to do now since there's also the work on splitting up webrtc/base driven by solenberg@ and terelius@ taking place.

Project Member

Comment 20 by solenberg@webrtc.org, Dec 13 2016

Blocking: 6829
Project Member

Comment 21 by kjellander@webrtc.org, Apr 12 2017

Blockedon: 7480
Project Member

Comment 22 by bugdroid1@chromium.org, Apr 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/ed754e71ae8866db641677073274e86fe704eeac

commit ed754e71ae8866db641677073274e86fe704eeac
Author: kjellander <kjellander@webrtc.org>
Date: Wed Apr 19 15:37:36 2017

Enable GN check for webrtc/base

It's not possible to enable it for the rtc_base_approved
target but since a larger refactoring is ongoing for webrtc/base
this CL doesn't attempt to fix that.

Changes made:
* Move webrtc/system_wrappers/include/stringize_macros.h into
  webrtc/base:rtc_base_approved_unittests (and corresponding
  unit test to rtc_base_approved_unittests).
* Move md5digest.* from rtc_base_approved to rtc_base_test_utils target.
* Move webrtc/system_wrappers/include/stringize_macros.h (+test) into
  webrtc/base.
* Remove unused use include of webrtc/base/fileutils.h in
  webrtc/base/pathutils.cc

BUG= webrtc:6828 , webrtc:3806,  webrtc:7480 
NOTRY=True

Review-Url: https://codereview.webrtc.org/2717083002
Cr-Commit-Position: refs/heads/master@{#17766}

[modify] https://crrev.com/ed754e71ae8866db641677073274e86fe704eeac/.gn
[modify] https://crrev.com/ed754e71ae8866db641677073274e86fe704eeac/webrtc/base/BUILD.gn
[modify] https://crrev.com/ed754e71ae8866db641677073274e86fe704eeac/webrtc/base/location.h
[modify] https://crrev.com/ed754e71ae8866db641677073274e86fe704eeac/webrtc/base/pathutils.cc
[rename] https://crrev.com/ed754e71ae8866db641677073274e86fe704eeac/webrtc/base/stringize_macros.h
[rename] https://crrev.com/ed754e71ae8866db641677073274e86fe704eeac/webrtc/base/stringize_macros_unittest.cc
[modify] https://crrev.com/ed754e71ae8866db641677073274e86fe704eeac/webrtc/common_audio/resampler/sinc_resampler_unittest.cc
[modify] https://crrev.com/ed754e71ae8866db641677073274e86fe704eeac/webrtc/modules/audio_coding/BUILD.gn
[modify] https://crrev.com/ed754e71ae8866db641677073274e86fe704eeac/webrtc/system_wrappers/BUILD.gn
[modify] https://crrev.com/ed754e71ae8866db641677073274e86fe704eeac/webrtc/test/fuzzers/BUILD.gn
[modify] https://crrev.com/ed754e71ae8866db641677073274e86fe704eeac/webrtc/voice_engine/BUILD.gn

Project Member

Comment 23 by bugdroid1@chromium.org, Apr 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/11ed366c487a938815cd52ad2ab5467b0f90e1ae

commit 11ed366c487a938815cd52ad2ab5467b0f90e1ae
Author: mbonadei <mbonadei@webrtc.org>
Date: Mon Apr 24 19:26:27 2017

Revert of Enable GN check for webrtc/base (patchset #13 id:240001 of https://codereview.webrtc.org/2717083002/ )

Reason for revert:
Breaks Chromium because in Chromium we import WebRTC with rtc_include_tests=false (https://bugs.chromium.org/p/chromium/issues/detail?id=713179#c6).

Chromium uses webrtc/test/fuzzers and this CL adds test dependencies to neteq_rtc_fuzzer.

Original issue's description:
> Enable GN check for webrtc/base
>
> It's not possible to enable it for the rtc_base_approved
> target but since a larger refactoring is ongoing for webrtc/base
> this CL doesn't attempt to fix that.
>
> Changes made:
> * Move webrtc/system_wrappers/include/stringize_macros.h into
>   webrtc/base:rtc_base_approved_unittests (and corresponding
>   unit test to rtc_base_approved_unittests).
> * Move md5digest.* from rtc_base_approved to rtc_base_test_utils target.
> * Move webrtc/system_wrappers/include/stringize_macros.h (+test) into
>   webrtc/base.
> * Remove unused use include of webrtc/base/fileutils.h in
>   webrtc/base/pathutils.cc
>
> BUG= webrtc:6828 , webrtc:3806,  webrtc:7480 
> NOTRY=True
>
> Review-Url: https://codereview.webrtc.org/2717083002
> Cr-Commit-Position: refs/heads/master@{#17766}
> Committed: https://chromium.googlesource.com/external/webrtc/+/ed754e71ae8866db641677073274e86fe704eeac

TBR=perkj@webrtc.org,tommi@webrtc.org,nisse@webrtc.org,kjellander@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= webrtc:6828 , webrtc:3806,  webrtc:7480 
NOTRY=True

Review-Url: https://codereview.webrtc.org/2838683002
Cr-Commit-Position: refs/heads/master@{#17849}

[modify] https://crrev.com/11ed366c487a938815cd52ad2ab5467b0f90e1ae/.gn
[modify] https://crrev.com/11ed366c487a938815cd52ad2ab5467b0f90e1ae/webrtc/base/BUILD.gn
[modify] https://crrev.com/11ed366c487a938815cd52ad2ab5467b0f90e1ae/webrtc/base/location.h
[modify] https://crrev.com/11ed366c487a938815cd52ad2ab5467b0f90e1ae/webrtc/base/pathutils.cc
[modify] https://crrev.com/11ed366c487a938815cd52ad2ab5467b0f90e1ae/webrtc/common_audio/resampler/sinc_resampler_unittest.cc
[modify] https://crrev.com/11ed366c487a938815cd52ad2ab5467b0f90e1ae/webrtc/modules/audio_coding/BUILD.gn
[modify] https://crrev.com/11ed366c487a938815cd52ad2ab5467b0f90e1ae/webrtc/system_wrappers/BUILD.gn
[rename] https://crrev.com/11ed366c487a938815cd52ad2ab5467b0f90e1ae/webrtc/system_wrappers/include/stringize_macros.h
[rename] https://crrev.com/11ed366c487a938815cd52ad2ab5467b0f90e1ae/webrtc/system_wrappers/source/stringize_macros_unittest.cc
[modify] https://crrev.com/11ed366c487a938815cd52ad2ab5467b0f90e1ae/webrtc/test/fuzzers/BUILD.gn
[modify] https://crrev.com/11ed366c487a938815cd52ad2ab5467b0f90e1ae/webrtc/voice_engine/BUILD.gn

Project Member

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

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/7054085e59c3123613cd0014bedb0fe91a56e26f

commit 7054085e59c3123613cd0014bedb0fe91a56e26f
Author: mbonadei <mbonadei@webrtc.org>
Date: Wed Apr 26 07:28:08 2017

Reland of Enable GN check for webrtc/base (patchset #3 id:230001 of https://codereview.webrtc.org/2838683002/ )

Reason for revert:
Try to fix the webrtc/test/fuzzers issue and reland this CL because it
contains lots of fixes for our BUILD.gn files.

Original issue's description:
> Revert of Enable GN check for webrtc/base (patchset #13 id:240001 of https://codereview.webrtc.org/2717083002/ )
>
> Reason for revert:
> Breaks Chromium because in Chromium we import WebRTC with rtc_include_tests=false (https://bugs.chromium.org/p/chromium/issues/detail?id=713179#c6).
>
> Chromium uses webrtc/test/fuzzers and this CL adds test dependencies to neteq_rtc_fuzzer.
>
> Original issue's description:
> > Enable GN check for webrtc/base
> >
> > It's not possible to enable it for the rtc_base_approved
> > target but since a larger refactoring is ongoing for webrtc/base
> > this CL doesn't attempt to fix that.
> >
> > Changes made:
> > * Move webrtc/system_wrappers/include/stringize_macros.h into
> >   webrtc/base:rtc_base_approved_unittests (and corresponding
> >   unit test to rtc_base_approved_unittests).
> > * Move md5digest.* from rtc_base_approved to rtc_base_test_utils target.
> > * Move webrtc/system_wrappers/include/stringize_macros.h (+test) into
> >   webrtc/base.
> > * Remove unused use include of webrtc/base/fileutils.h in
> >   webrtc/base/pathutils.cc
> >
> > BUG= webrtc:6828 , webrtc:3806,  webrtc:7480 
> > NOTRY=True
> >
> > Review-Url: https://codereview.webrtc.org/2717083002
> > Cr-Commit-Position: refs/heads/master@{#17766}
> > Committed: https://chromium.googlesource.com/external/webrtc/+/ed754e71ae8866db641677073274e86fe704eeac
>
> TBR=perkj@webrtc.org,tommi@webrtc.org,nisse@webrtc.org,kjellander@webrtc.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG= webrtc:6828 , webrtc:3806,  webrtc:7480 
> NOTRY=True
>
> Review-Url: https://codereview.webrtc.org/2838683002
> Cr-Commit-Position: refs/heads/master@{#17849}
> Committed: https://chromium.googlesource.com/external/webrtc/+/11ed366c487a938815cd52ad2ab5467b0f90e1ae

TBR=perkj@webrtc.org,tommi@webrtc.org,nisse@webrtc.org,kjellander@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= webrtc:6828 , webrtc:3806,  webrtc:7480 

Review-Url: https://codereview.webrtc.org/2840453004
Cr-Commit-Position: refs/heads/master@{#17876}

[modify] https://crrev.com/7054085e59c3123613cd0014bedb0fe91a56e26f/.gn
[modify] https://crrev.com/7054085e59c3123613cd0014bedb0fe91a56e26f/webrtc/base/BUILD.gn
[modify] https://crrev.com/7054085e59c3123613cd0014bedb0fe91a56e26f/webrtc/base/location.h
[modify] https://crrev.com/7054085e59c3123613cd0014bedb0fe91a56e26f/webrtc/base/pathutils.cc
[rename] https://crrev.com/7054085e59c3123613cd0014bedb0fe91a56e26f/webrtc/base/stringize_macros.h
[rename] https://crrev.com/7054085e59c3123613cd0014bedb0fe91a56e26f/webrtc/base/stringize_macros_unittest.cc
[modify] https://crrev.com/7054085e59c3123613cd0014bedb0fe91a56e26f/webrtc/common_audio/resampler/sinc_resampler_unittest.cc
[modify] https://crrev.com/7054085e59c3123613cd0014bedb0fe91a56e26f/webrtc/modules/audio_coding/BUILD.gn
[modify] https://crrev.com/7054085e59c3123613cd0014bedb0fe91a56e26f/webrtc/system_wrappers/BUILD.gn
[modify] https://crrev.com/7054085e59c3123613cd0014bedb0fe91a56e26f/webrtc/test/fuzzers/BUILD.gn
[modify] https://crrev.com/7054085e59c3123613cd0014bedb0fe91a56e26f/webrtc/voice_engine/BUILD.gn

Project Member

Comment 25 by bugdroid1@chromium.org, Apr 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/3d7b0e2fda64327a0182099160305e7098c1a277

commit 3d7b0e2fda64327a0182099160305e7098c1a277
Author: mbonadei <mbonadei@webrtc.org>
Date: Wed Apr 26 07:38:48 2017

Revert of Enable GN check for webrtc/base (patchset #9 id:350001 of https://codereview.webrtc.org/2840453004/ )

Reason for revert:
It causes a Chromium build error:

ERROR at //third_party/webrtc/test/BUILD.gn:113:5: Can't load input file.
    "//third_party/gflags",

Original issue's description:
> Reland of Enable GN check for webrtc/base (patchset #3 id:230001 of https://codereview.webrtc.org/2838683002/ )
>
> Reason for revert:
> Try to fix the webrtc/test/fuzzers issue and reland this CL because it
> contains lots of fixes for our BUILD.gn files.
>
> Original issue's description:
> > Revert of Enable GN check for webrtc/base (patchset #13 id:240001 of https://codereview.webrtc.org/2717083002/ )
> >
> > Reason for revert:
> > Breaks Chromium because in Chromium we import WebRTC with rtc_include_tests=false (https://bugs.chromium.org/p/chromium/issues/detail?id=713179#c6).
> >
> > Chromium uses webrtc/test/fuzzers and this CL adds test dependencies to neteq_rtc_fuzzer.
> >
> > Original issue's description:
> > > Enable GN check for webrtc/base
> > >
> > > It's not possible to enable it for the rtc_base_approved
> > > target but since a larger refactoring is ongoing for webrtc/base
> > > this CL doesn't attempt to fix that.
> > >
> > > Changes made:
> > > * Move webrtc/system_wrappers/include/stringize_macros.h into
> > >   webrtc/base:rtc_base_approved_unittests (and corresponding
> > >   unit test to rtc_base_approved_unittests).
> > > * Move md5digest.* from rtc_base_approved to rtc_base_test_utils target.
> > > * Move webrtc/system_wrappers/include/stringize_macros.h (+test) into
> > >   webrtc/base.
> > > * Remove unused use include of webrtc/base/fileutils.h in
> > >   webrtc/base/pathutils.cc
> > >
> > > BUG= webrtc:6828 , webrtc:3806,  webrtc:7480 
> > > NOTRY=True
> > >
> > > Review-Url: https://codereview.webrtc.org/2717083002
> > > Cr-Commit-Position: refs/heads/master@{#17766}
> > > Committed: https://chromium.googlesource.com/external/webrtc/+/ed754e71ae8866db641677073274e86fe704eeac
> >
> > TBR=perkj@webrtc.org,tommi@webrtc.org,nisse@webrtc.org,kjellander@webrtc.org
> > # Not skipping CQ checks because original CL landed more than 1 days ago.
> > BUG= webrtc:6828 , webrtc:3806,  webrtc:7480 
> > NOTRY=True
> >
> > Review-Url: https://codereview.webrtc.org/2838683002
> > Cr-Commit-Position: refs/heads/master@{#17849}
> > Committed: https://chromium.googlesource.com/external/webrtc/+/11ed366c487a938815cd52ad2ab5467b0f90e1ae
>
> TBR=perkj@webrtc.org,tommi@webrtc.org,nisse@webrtc.org,kjellander@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= webrtc:6828 , webrtc:3806,  webrtc:7480 
>
> Review-Url: https://codereview.webrtc.org/2840453004
> Cr-Commit-Position: refs/heads/master@{#17876}
> Committed: https://chromium.googlesource.com/external/webrtc/+/7054085e59c3123613cd0014bedb0fe91a56e26f

TBR=perkj@webrtc.org,tommi@webrtc.org,nisse@webrtc.org,kjellander@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= webrtc:6828 , webrtc:3806,  webrtc:7480 

Review-Url: https://codereview.webrtc.org/2846483002
Cr-Commit-Position: refs/heads/master@{#17877}

[modify] https://crrev.com/3d7b0e2fda64327a0182099160305e7098c1a277/.gn
[modify] https://crrev.com/3d7b0e2fda64327a0182099160305e7098c1a277/webrtc/base/BUILD.gn
[modify] https://crrev.com/3d7b0e2fda64327a0182099160305e7098c1a277/webrtc/base/location.h
[modify] https://crrev.com/3d7b0e2fda64327a0182099160305e7098c1a277/webrtc/base/pathutils.cc
[modify] https://crrev.com/3d7b0e2fda64327a0182099160305e7098c1a277/webrtc/common_audio/resampler/sinc_resampler_unittest.cc
[modify] https://crrev.com/3d7b0e2fda64327a0182099160305e7098c1a277/webrtc/modules/audio_coding/BUILD.gn
[modify] https://crrev.com/3d7b0e2fda64327a0182099160305e7098c1a277/webrtc/system_wrappers/BUILD.gn
[rename] https://crrev.com/3d7b0e2fda64327a0182099160305e7098c1a277/webrtc/system_wrappers/include/stringize_macros.h
[rename] https://crrev.com/3d7b0e2fda64327a0182099160305e7098c1a277/webrtc/system_wrappers/source/stringize_macros_unittest.cc
[modify] https://crrev.com/3d7b0e2fda64327a0182099160305e7098c1a277/webrtc/test/fuzzers/BUILD.gn
[modify] https://crrev.com/3d7b0e2fda64327a0182099160305e7098c1a277/webrtc/voice_engine/BUILD.gn

Project Member

Comment 26 by bugdroid1@chromium.org, Apr 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/148d5a2dcaa54bfaa1fd1ce77eb57a399bf04557

commit 148d5a2dcaa54bfaa1fd1ce77eb57a399bf04557
Author: mbonadei <mbonadei@webrtc.org>
Date: Fri Apr 28 12:24:50 2017

Reland of Enable GN check for webrtc/base (patchset #3 id:230001 of https://codereview.webrtc.org/2838683002/ )

Reason for revert:
Fourth attempt to land.

Waiting for https://codereview.webrtc.org/2845013003 to
avoid conflicts on webrtc/modules/audio_coding:neteq_unittest_tools.

Original issue's description:
> Revert of Enable GN check for webrtc/base (patchset #13 id:240001 of https://codereview.webrtc.org/2717083002/ )
>
> Reason for revert:
> Breaks Chromium because in Chromium we import WebRTC with rtc_include_tests=false (https://bugs.chromium.org/p/chromium/issues/detail?id=713179#c6).
>
> Chromium uses webrtc/test/fuzzers and this CL adds test dependencies to neteq_rtc_fuzzer.
>
> Original issue's description:
> > Enable GN check for webrtc/base
> >
> > It's not possible to enable it for the rtc_base_approved
> > target but since a larger refactoring is ongoing for webrtc/base
> > this CL doesn't attempt to fix that.
> >
> > Changes made:
> > * Move webrtc/system_wrappers/include/stringize_macros.h into
> >   webrtc/base:rtc_base_approved_unittests (and corresponding
> >   unit test to rtc_base_approved_unittests).
> > * Move md5digest.* from rtc_base_approved to rtc_base_test_utils target.
> > * Move webrtc/system_wrappers/include/stringize_macros.h (+test) into
> >   webrtc/base.
> > * Remove unused use include of webrtc/base/fileutils.h in
> >   webrtc/base/pathutils.cc
> >
> > BUG= webrtc:6828 , webrtc:3806,  webrtc:7480 
> > NOTRY=True
> >
> > Review-Url: https://codereview.webrtc.org/2717083002
> > Cr-Commit-Position: refs/heads/master@{#17766}
> > Committed: https://chromium.googlesource.com/external/webrtc/+/ed754e71ae8866db641677073274e86fe704eeac
>
> TBR=perkj@webrtc.org,tommi@webrtc.org,nisse@webrtc.org,kjellander@webrtc.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG= webrtc:6828 , webrtc:3806,  webrtc:7480 
> NOTRY=True
>
> Review-Url: https://codereview.webrtc.org/2838683002
> Cr-Commit-Position: refs/heads/master@{#17849}
> Committed: https://chromium.googlesource.com/external/webrtc/+/11ed366c487a938815cd52ad2ab5467b0f90e1ae

TBR=perkj@webrtc.org,tommi@webrtc.org,nisse@webrtc.org,kjellander@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= webrtc:6828 , webrtc:3806,  webrtc:7480 

Review-Url: https://codereview.webrtc.org/2852663002
Cr-Commit-Position: refs/heads/master@{#17927}

[modify] https://crrev.com/148d5a2dcaa54bfaa1fd1ce77eb57a399bf04557/.gn
[modify] https://crrev.com/148d5a2dcaa54bfaa1fd1ce77eb57a399bf04557/webrtc/base/BUILD.gn
[modify] https://crrev.com/148d5a2dcaa54bfaa1fd1ce77eb57a399bf04557/webrtc/base/location.h
[modify] https://crrev.com/148d5a2dcaa54bfaa1fd1ce77eb57a399bf04557/webrtc/base/pathutils.cc
[rename] https://crrev.com/148d5a2dcaa54bfaa1fd1ce77eb57a399bf04557/webrtc/base/stringize_macros.h
[rename] https://crrev.com/148d5a2dcaa54bfaa1fd1ce77eb57a399bf04557/webrtc/base/stringize_macros_unittest.cc
[modify] https://crrev.com/148d5a2dcaa54bfaa1fd1ce77eb57a399bf04557/webrtc/common_audio/resampler/sinc_resampler_unittest.cc
[modify] https://crrev.com/148d5a2dcaa54bfaa1fd1ce77eb57a399bf04557/webrtc/modules/audio_coding/BUILD.gn
[modify] https://crrev.com/148d5a2dcaa54bfaa1fd1ce77eb57a399bf04557/webrtc/system_wrappers/BUILD.gn
[modify] https://crrev.com/148d5a2dcaa54bfaa1fd1ce77eb57a399bf04557/webrtc/test/BUILD.gn
[modify] https://crrev.com/148d5a2dcaa54bfaa1fd1ce77eb57a399bf04557/webrtc/test/fuzzers/BUILD.gn
[modify] https://crrev.com/148d5a2dcaa54bfaa1fd1ce77eb57a399bf04557/webrtc/voice_engine/BUILD.gn

Project Member

Comment 27 by sheriffbot@chromium.org, Sep 6

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.


Project Member

Comment 28 by henrika@webrtc.org, Sep 7

Cc: -pthatcher@webrtc.org -andresp@webrtc.org -tommi@webrtc.org -kjellander@webrtc.org -andrew@webrtc.org -juberti@webrtc.org
Owner: nisse@webrtc.org
Status: Assigned (was: Untriaged)
Most likely no longer valid. Reassigning just in case to ensure that it can be closed.
Project Member

Comment 29 by nisse@webrtc.org, Sep 7

It seems we still have the messagequeue --> socketserver dependency.

Maybe we can cut that replacing the socket sever pointer (member MessageQueue::ss_) by a simpler wakeup callback?

Also, the MessageQueue may or may not own the socket server. Doing it one way or the other would make things a bit simpler.

I stumbled on rtc::Thread not belonging in rtc_base_approved just the other day, in code that needed to use the system_wrapper's SleepMs rather than rtc::Thread::SleepMs because it shouldn't depend on rtc_base.

Sign in to add a comment