Link Blink into the browser process less.

Link Blink into the browser process less.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be enabled when the bug is fixed.

Fix the set of fixable dependencies.

One big one is that content/browser depends on libjingle_webrtc.

This also links libpeerconnection into content/browser because (at least on Windows) libpeerconnection and libjingle_webrtc are both required to link at the same time. I don't fully understand the webrtc build here (I tried and failed to fix the internal deps) but I can tell that third_party/libjingle/source/talk/app/webrtc/peerconnectionfactory.cc in the libjingle_webrtc target uses MediaEngineInterface from third_party/libjingle/source/talk/media/webrtc/webrtcmediaengine.cc in the libpeerconnection target.

BUG=582206, 581766
Committed: https://crrev.com/e9ce0c5eedfc73b15404b5c0a5e737cce94670bf
Cr-Commit-Position: refs/heads/master@{#373666}

[brettw, 2016-01-27 18:55:34]

brettw@chromium.org changed reviewers:
+ dpranke@chromium.org

[brettw, 2016-01-27 18:55:34]

I was quite surprised how wrong our dependencies were!

[brettw, 2016-01-27 18:55:38]

The CQ bit was checked by brettw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-27 18:56:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1643633003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1643633003/1

[commit-bot: I haz the power, 2016-01-27 19:14:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-27 19:14:23]

Dry run: Try jobs failed on following builders:
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[brettw, 2016-01-27 21:21:22]

The CQ bit was checked by brettw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-27 21:23:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1643633003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1643633003/20001

[commit-bot: I haz the power, 2016-01-27 22:17:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-27 22:17:24]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[brettw, 2016-01-27 23:16:27]

The CQ bit was checked by brettw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-27 23:17:41]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1643633003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1643633003/40001

[commit-bot: I haz the power, 2016-01-28 01:18:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-28 01:18:08]

Dry run: Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[Dirk Pranke, 2016-01-28 01:54:30]

lgtm w/ minor notes ...

https://codereview.chromium.org/1643633003/diff/40001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/1643633003/diff/40001/chrome/BUILD.gn#newcode466
chrome/BUILD.gn:466: # Note that the blink_headers target is OK, so we can't doa
wildcard for
s/doa/do a/

https://codereview.chromium.org/1643633003/diff/40001/components/guest_view/b...
File components/guest_view/browser/BUILD.gn (right):

https://codereview.chromium.org/1643633003/diff/40001/components/guest_view/b...
components/guest_view/browser/BUILD.gn:29: "//url",
hmm ... if this is the actual dependency, I wonder why `gn check` wasn't
complaining about this?

https://codereview.chromium.org/1643633003/diff/40001/content/common/BUILD.gn
File content/common/BUILD.gn (left):

https://codereview.chromium.org/1643633003/diff/40001/content/common/BUILD.gn...
content/common/BUILD.gn:207: "//third_party/WebKit/public:blink",
//content/common doesn't require blink_headers ? that's actually a bit
surprising to me ...

[brettw, 2016-01-28 21:30:56]

https://codereview.chromium.org/1643633003/diff/40001/components/guest_view/b...
File components/guest_view/browser/BUILD.gn (right):

https://codereview.chromium.org/1643633003/diff/40001/components/guest_view/b...
components/guest_view/browser/BUILD.gn:29: "//url",
On 2016/01/28 01:54:30, Dirk Pranke wrote:
> hmm ... if this is the actual dependency, I wonder why `gn check` wasn't
> complaining about this?

This was a public dep of public:blink before so when I removed blink, I needed
to add url here for check to be happy.

https://codereview.chromium.org/1643633003/diff/40001/content/common/BUILD.gn
File content/common/BUILD.gn (left):

https://codereview.chromium.org/1643633003/diff/40001/content/common/BUILD.gn...
content/common/BUILD.gn:207: "//third_party/WebKit/public:blink",
On 2016/01/28 01:54:30, Dirk Pranke wrote:
> //content/common doesn't require blink_headers ? that's actually a bit
> surprising to me ...

blink_headers is already in the public_deps section above.

[scottmg, 2016-01-28 21:37:58]

scottmg@chromium.org changed reviewers:
+ scottmg@chromium.org

[scottmg, 2016-01-28 21:37:59]

"Fortunately, no code actually used Blink and these were all incorrectly written
dependencies in the BUILD files." in the description was apparently a bit
optimistic.

[brettw, 2016-01-28 22:19:46]

Description was changed from

==========
Don't link Blink into the browser process.

Adds an assert_no_deps from chrome/browser to Blink so GN can check that there
are no such dependencies.

Fix the existing dependencies. Fortunately, no code actually used Blink and
these were all incorrectly written dependencies in the BUILD files.
==========

to

==========
Don't link Blink into the browser process.

Adds an assert_no_deps from chrome/browser to Blink so GN can check that there
are no such dependencies.

Fix the existing dependencies.
==========

[brettw, 2016-01-28 23:34:55]

Comment fix

[brettw, 2016-02-04 17:43:41]

Description was changed from

==========
Don't link Blink into the browser process.

Adds an assert_no_deps from chrome/browser to Blink so GN can check that there
are no such dependencies.

Fix the existing dependencies.
==========

to

==========
Don't link Blink into the browser process.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be
enabled when the bug is fixed.

Fix the set of fixable dependencies.
==========

[brettw, 2016-02-04 17:43:55]

Description was changed from

==========
Don't link Blink into the browser process.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be
enabled when the bug is fixed.

Fix the set of fixable dependencies.
==========

to

==========
Link Blink into the browser process less.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be
enabled when the bug is fixed.

Fix the set of fixable dependencies.
==========

[brettw, 2016-02-04 21:10:13]

Description was changed from

==========
Link Blink into the browser process less.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be
enabled when the bug is fixed.

Fix the set of fixable dependencies.
==========

to

==========
Link Blink into the browser process less.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be
enabled when the bug is fixed.

Fix the set of fixable dependencies.

This adds a dependency on webrtc from libjingle_webrtc. Previously this linked
to some isolated portions of webrtc. But more than this is used and it only
linked in the end because the incorrect deps removed in this patch brought all
of webrtc into the browser process. e.g.
third_party/libjingle/source/talk/app/webrtc/mediacontroller.cc depends on
webrtc::Call::Create).

Just adding //third_party/webrtc/call to the dependency list there gives lots
more errors. The "webrtc/call" target depends on lots of other parts of WebRtc
that it lacks proper dependencies for (e.g. Call::CreateVideoReceiveStream calls
webrtc::CallStats::CallStats in webrtc/video).
==========

[brettw, 2016-02-04 21:37:18]

Description was changed from

==========
Link Blink into the browser process less.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be
enabled when the bug is fixed.

Fix the set of fixable dependencies.

This adds a dependency on webrtc from libjingle_webrtc. Previously this linked
to some isolated portions of webrtc. But more than this is used and it only
linked in the end because the incorrect deps removed in this patch brought all
of webrtc into the browser process. e.g.
third_party/libjingle/source/talk/app/webrtc/mediacontroller.cc depends on
webrtc::Call::Create).

Just adding //third_party/webrtc/call to the dependency list there gives lots
more errors. The "webrtc/call" target depends on lots of other parts of WebRtc
that it lacks proper dependencies for (e.g. Call::CreateVideoReceiveStream calls
webrtc::CallStats::CallStats in webrtc/video).
==========

to

==========
Link Blink into the browser process less.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be
enabled when the bug is fixed.

Fix the set of fixable dependencies.

This adds a dependency on webrtc from libjingle_webrtc. Previously this linked
to some isolated portions of webrtc. But more than this is used and it only
linked in the end because the incorrect deps removed in this patch brought all
of webrtc into the browser process. e.g.
third_party/libjingle/source/talk/app/webrtc/mediacontroller.cc depends on
webrtc::Call::Create).

Just adding //third_party/webrtc/call to the dependency list there gives lots
more errors. The "webrtc/call" target depends on lots of other parts of WebRtc
that it lacks proper dependencies for (e.g. Call::CreateVideoReceiveStream calls
webrtc::CallStats::CallStats in webrtc/video).

This also links libpeerconnection into all targets that link libjingle_webrtc
because (at least on Windows) libpeerconnection and libjingle_webrtc are both
required to link at the same time. I don't fully understand the webrtc build
here (I tried and failed to fix the internal deps) but I can tell that
third_party/libjingle/source/talk/app/webrtc/peerconnectionfactory.cc in the
libjingle_webrtc target uses MediaEngineInterface from
third_party/libjingle/source/talk/media/webrtc/webrtcmediaengine.cc in the
libpeerconnection target.

libjingle_webrtc_common because
third_party/libjingle/source/talk/app/webrtc/peerconnectionfactory.cc uses
==========

[brettw, 2016-02-04 21:41:14]

Description was changed from

==========
Link Blink into the browser process less.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be
enabled when the bug is fixed.

Fix the set of fixable dependencies.

This adds a dependency on webrtc from libjingle_webrtc. Previously this linked
to some isolated portions of webrtc. But more than this is used and it only
linked in the end because the incorrect deps removed in this patch brought all
of webrtc into the browser process. e.g.
third_party/libjingle/source/talk/app/webrtc/mediacontroller.cc depends on
webrtc::Call::Create).

Just adding //third_party/webrtc/call to the dependency list there gives lots
more errors. The "webrtc/call" target depends on lots of other parts of WebRtc
that it lacks proper dependencies for (e.g. Call::CreateVideoReceiveStream calls
webrtc::CallStats::CallStats in webrtc/video).

This also links libpeerconnection into all targets that link libjingle_webrtc
because (at least on Windows) libpeerconnection and libjingle_webrtc are both
required to link at the same time. I don't fully understand the webrtc build
here (I tried and failed to fix the internal deps) but I can tell that
third_party/libjingle/source/talk/app/webrtc/peerconnectionfactory.cc in the
libjingle_webrtc target uses MediaEngineInterface from
third_party/libjingle/source/talk/media/webrtc/webrtcmediaengine.cc in the
libpeerconnection target.

libjingle_webrtc_common because
third_party/libjingle/source/talk/app/webrtc/peerconnectionfactory.cc uses
==========

to

==========
Link Blink into the browser process less.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be
enabled when the bug is fixed.

Fix the set of fixable dependencies.

One big one is that content/browser depends on libjingle_webrtc.

This also links libpeerconnection into content/browser because (at least on
Windows) libpeerconnection and libjingle_webrtc are both required to link at the
same time. I don't fully understand the webrtc build here (I tried and failed to
fix the internal deps) but I can tell that
third_party/libjingle/source/talk/app/webrtc/peerconnectionfactory.cc in the
libjingle_webrtc target uses MediaEngineInterface from
third_party/libjingle/source/talk/media/webrtc/webrtcmediaengine.cc in the
libpeerconnection target.
==========

[brettw, 2016-02-04 21:59:03]

The CQ bit was checked by brettw@chromium.org to run a CQ dry run

[brettw, 2016-02-04 21:59:30]

Okay, I'm going to try to land this set of fixes just with the assertions
commented out. I fixed the webrtc linking issues (I hope). PTAL if you care.

[commit-bot: I haz the power, 2016-02-04 22:03:41]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1643633003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1643633003/100001

[Dirk Pranke, 2016-02-04 22:04:09]

still lgtm.

[brettw, 2016-02-04 22:17:00]

Description was changed from

==========
Link Blink into the browser process less.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be
enabled when the bug is fixed.

Fix the set of fixable dependencies.

One big one is that content/browser depends on libjingle_webrtc.

This also links libpeerconnection into content/browser because (at least on
Windows) libpeerconnection and libjingle_webrtc are both required to link at the
same time. I don't fully understand the webrtc build here (I tried and failed to
fix the internal deps) but I can tell that
third_party/libjingle/source/talk/app/webrtc/peerconnectionfactory.cc in the
libjingle_webrtc target uses MediaEngineInterface from
third_party/libjingle/source/talk/media/webrtc/webrtcmediaengine.cc in the
libpeerconnection target.
==========

to

==========
Link Blink into the browser process less.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be
enabled when the bug is fixed.

Fix the set of fixable dependencies.

One big one is that content/browser depends on libjingle_webrtc.

This also links libpeerconnection into content/browser because (at least on
Windows) libpeerconnection and libjingle_webrtc are both required to link at the
same time. I don't fully understand the webrtc build here (I tried and failed to
fix the internal deps) but I can tell that
third_party/libjingle/source/talk/app/webrtc/peerconnectionfactory.cc in the
libjingle_webrtc target uses MediaEngineInterface from
third_party/libjingle/source/talk/media/webrtc/webrtcmediaengine.cc in the
libpeerconnection target.

BUG=582206, 581766
==========

[brettw, 2016-02-04 22:17:10]

The CQ bit was unchecked by brettw@chromium.org

[brettw, 2016-02-04 22:17:13]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2016-02-04 22:17:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1643633003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1643633003/100001

[commit-bot: I haz the power, 2016-02-04 23:45:14]

Description was changed from

==========
Link Blink into the browser process less.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be
enabled when the bug is fixed.

Fix the set of fixable dependencies.

One big one is that content/browser depends on libjingle_webrtc.

This also links libpeerconnection into content/browser because (at least on
Windows) libpeerconnection and libjingle_webrtc are both required to link at the
same time. I don't fully understand the webrtc build here (I tried and failed to
fix the internal deps) but I can tell that
third_party/libjingle/source/talk/app/webrtc/peerconnectionfactory.cc in the
libjingle_webrtc target uses MediaEngineInterface from
third_party/libjingle/source/talk/media/webrtc/webrtcmediaengine.cc in the
libpeerconnection target.

BUG=582206, 581766
==========

to

==========
Link Blink into the browser process less.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be
enabled when the bug is fixed.

Fix the set of fixable dependencies.

One big one is that content/browser depends on libjingle_webrtc.

This also links libpeerconnection into content/browser because (at least on
Windows) libpeerconnection and libjingle_webrtc are both required to link at the
same time. I don't fully understand the webrtc build here (I tried and failed to
fix the internal deps) but I can tell that
third_party/libjingle/source/talk/app/webrtc/peerconnectionfactory.cc in the
libjingle_webrtc target uses MediaEngineInterface from
third_party/libjingle/source/talk/media/webrtc/webrtcmediaengine.cc in the
libpeerconnection target.

BUG=582206, 581766
==========

[commit-bot: I haz the power, 2016-02-04 23:45:15]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-02-04 23:46:03]

Description was changed from

==========
Link Blink into the browser process less.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be
enabled when the bug is fixed.

Fix the set of fixable dependencies.

One big one is that content/browser depends on libjingle_webrtc.

This also links libpeerconnection into content/browser because (at least on
Windows) libpeerconnection and libjingle_webrtc are both required to link at the
same time. I don't fully understand the webrtc build here (I tried and failed to
fix the internal deps) but I can tell that
third_party/libjingle/source/talk/app/webrtc/peerconnectionfactory.cc in the
libjingle_webrtc target uses MediaEngineInterface from
third_party/libjingle/source/talk/media/webrtc/webrtcmediaengine.cc in the
libpeerconnection target.

BUG=582206, 581766
==========

to

==========
Link Blink into the browser process less.

Adds a commented-out assert_no_deps from chrome/browser to Blink. This should be
enabled when the bug is fixed.

Fix the set of fixable dependencies.

One big one is that content/browser depends on libjingle_webrtc.

This also links libpeerconnection into content/browser because (at least on
Windows) libpeerconnection and libjingle_webrtc are both required to link at the
same time. I don't fully understand the webrtc build here (I tried and failed to
fix the internal deps) but I can tell that
third_party/libjingle/source/talk/app/webrtc/peerconnectionfactory.cc in the
libjingle_webrtc target uses MediaEngineInterface from
third_party/libjingle/source/talk/media/webrtc/webrtcmediaengine.cc in the
libpeerconnection target.

BUG=582206, 581766

Committed: https://crrev.com/e9ce0c5eedfc73b15404b5c0a5e737cce94670bf
Cr-Commit-Position: refs/heads/master@{#373666}
==========

[commit-bot: I haz the power, 2016-02-04 23:46:04]

Patchset 6 (id:??) landed as
https://crrev.com/e9ce0c5eedfc73b15404b5c0a5e737cce94670bf
Cr-Commit-Position: refs/heads/master@{#373666}