Bringing back enable_webrtc?

227 views
Skip to first unread message

Allan Sandfeld Jensen

unread,
Aug 24, 2018, 4:57:58 AM8/24/18
to chromi...@chromium.org
I noticed while updating our embedded project to Chromium 68 that you have
removed the enable_webrtc option because it didn't build for a couple of
releases. I can confirm it didn't build for a long time, we had patches
locally to fix it, but I admit I didn't really think of upstreaming them,
fixing combinations of options that we need, but doesn't build out of the box
is everyday life for an embedder of Chromium, we have fixes like that for many
more Chromium options that doesn't build out the box.

Since WebRTC is a very large feature with many third party dependencies and it
is not that commonly used or needed, it is one of the biggest things we can
disable for customers to bring the size of Chromium down. I would very much
like to keep the option to disable it. We will be reverting the patches in our
branch, but would like to bring it back upstream as well, and will promise to
upstream patches maintaining it in the future if necessary.

Would it be acceptable?

Best regards
'Allan Sandfeld Jensen
Principal Engineer
The Qt Company


Dirk Pranke

unread,
Aug 24, 2018, 1:28:34 PM8/24/18
to Allan Sandfeld Jensen, chromium-dev
I would be theoretically open to keeping it working -- i.e. to help embedders if this really is a common need -- but it would depend on the impact to the code base. Any sense of how big that'd be?

I would also probably defer to the media and other platform folks if they had compelling reasons for making it be mandatory.

-- Dirk



--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
    http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/23624630.1r3eYUQgxm%40twilight.


Brett Wilson

unread,
Aug 24, 2018, 4:29:10 PM8/24/18
to Dirk Pranke, k...@carewolf.com, Chromium-dev
You can see the patch that removed the defines:

There were a lot, and they were in some really hairy files where we'd want to reduce complexity.

According to the change that removed the flag (also relatively large):
it was "broken for months and no one has noticed".

Brett

Allan Sandfeld Jensen

unread,
Aug 24, 2018, 4:42:14 PM8/24/18
to Brett Wilson, Dirk Pranke, Chromium-dev
On Freitag, 24. August 2018 22:27:31 CEST Brett Wilson wrote:
> You can see the patch that removed the defines:
> https://chromium-review.googlesource.com/c/chromium/src/+/1059408
>
> There were a lot, and they were in some really hairy files where we'd want
> to reduce complexity.
>
> According to the change that removed the flag (also relatively large):
> https://chromium-review.googlesource.com/c/chromium/src/+/1044220
> it was "broken for months and no one has noticed".
>
Well, we noticed, but fixing it was very minor things like updating some
arguments to functions that now took one more argument. This is the kind of
stuff that usually just gets fixed in the next release of Chrome, but I can
see how that attitude can be a problem if all embedders think like that.

Br.
Allan


Matthew Menke

unread,
Aug 24, 2018, 10:35:31 PM8/24/18
to Chromium-dev, bre...@chromium.org, dpr...@chromium.org
473 lines of code across 84 files seems a bit much to disable a feature.  Maybe there are different places we could make the cut that aren't quite as intrusive?

Niklas Enbom

unread,
Aug 27, 2018, 11:41:10 AM8/27/18
to mme...@google.com, Chromium-dev, bre...@chromium.org, dpr...@chromium.org
On Fri, Aug 24, 2018 at 7:35 PM 'Matthew Menke' via Chromium-dev <chromi...@chromium.org> wrote:
473 lines of code across 84 files seems a bit much to disable a feature.  Maybe there are different places we could make the cut that aren't quite as intrusive?
 
That's because "WebRTC" isn't a single feature. And yes, if we reintroduce the flag it would make sense if was more clear what feature you disable, PeerConnection, Remoting etc.

 

On Friday, August 24, 2018 at 4:42:14 PM UTC-4, Allan Sandfeld Jensen wrote:
On Freitag, 24. August 2018 22:27:31 CEST Brett Wilson wrote:
> You can see the patch that removed the defines:
> https://chromium-review.googlesource.com/c/chromium/src/+/1059408
>
> There were a lot, and they were in some really hairy files where we'd want
> to reduce complexity.
>
> According to the change that removed the flag (also relatively large):
> https://chromium-review.googlesource.com/c/chromium/src/+/1044220
> it was "broken for months and no one has noticed".
>
Well, we noticed, but fixing it was very minor things like updating some
arguments to functions that now took one more argument. This is the kind of
stuff that usually just gets fixed in the next release of Chrome, but I can
see how that attitude can be a problem if all embedders think like that.

Br.
Allan


--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/ade95923-24e5-47e3-8049-33ea0006aeaf%40chromium.org.
Reply all
Reply to author
Forward
0 new messages