Break cycles between views, content and webview.

Break cycles between views, content and webview.

When running gyp_chromium with the following diff:

diff --git a/build/gyp_chromium b/build/gyp_chromium
index 63e8671..ca9b6a4 100755
--- a/build/gyp_chromium
+++ b/build/gyp_chromium
@@ -509,8 +509,6 @@ if __name__ == '__main__':
   # option.  http://crbug.com/35878.
   # TODO(tc): Fix circular dependencies in ChromiumOS then add linux2
   # list.
-  if sys.platform not in ('darwin',):
-    args.append('--no-circular-check')

These cycles are found:

gyp: Cycles in .gyp file dependency graph detected:
Cycle: content/content_shell_and_tests.gyp ->
ui/views/controls/webview/webview.gyp -> ui/views/views.gyp ->
content/content_shell_and_tests.gyp
Cycle: ui/views/controls/webview/webview.gyp -> ui/views/views.gyp ->
content/content_shell_and_tests.gyp ->
ui/views/controls/webview/webview.gyp
Cycle: ui/views/views.gyp -> content/content_shell_and_tests.gyp ->
ui/views/controls/webview/webview.gyp -> ui/views/views.gyp
Cycle: ui/views/views.gyp -> content/content_shell_and_tests.gyp ->
ui/views/views.gyp
Cycle: ui/views/controls/webview/webview.gyp -> ui/views/views.gyp ->
ui/views/controls/webview/webview.gyp
Cycle: ui/views/views.gyp -> ui/views/controls/webview/webview.gyp ->
ui/views/views.gyp
Cycle: content/content_shell_and_tests.gyp -> ui/views/views.gyp ->
content/content_shell_and_tests.gyp

By moving '*examples*' targets from views.gyp to examples.gyp we break
most of these cycles.

Then it remains the cycle:
Cycle: content/content_shell_and_tests.gyp -> ui/views/controls/webview/webview.gyp -> content/content_shell_and_tests.gyp

To fix that we introduced a webview_tests.gyp to which we moved the include of
content_shell_and_tests.gyp from webview.gyp, and thus breaking that
cycle and fixing all the circlar dependencies found above.

BUG=331669, 35878
TEST=run gyp_chromium with the above diff, gyp should not throw any
cycles output.

R=ben@chromium.org, harrym@chromium.org, tapted@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=258758

[tapted, 2014-03-16 23:45:48]

https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp
File ash/ash.gyp (right):

https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp#newcode845
ash/ash.gyp:845:
'../ui/views/examples/examples.gyp:views_examples_with_content_lib',
Having ash_unittests depending on examples.gyp feels a bit odd - how much
explodes if this is just removed? Maybe some transitive dependencies just need
to be moved in here.

[tfarina, 2014-03-17 01:33:09]

Harry, could you comment on the ash dependency on views' examples?

https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp
File ash/ash.gyp (right):

https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp#newcode845
ash/ash.gyp:845:
'../ui/views/examples/examples.gyp:views_examples_with_content_lib',
On 2014/03/16 23:45:49, tapted wrote:
> Having ash_unittests depending on examples.gyp feels a bit odd - how much
> explodes if this is just removed? Maybe some transitive dependencies just need
> to be moved in here.
I will try removing it and see what happens.

This was added by Harry -
https://chromiumcodereview.appspot.com/10834185/diff/7002/ash/ash.gyp

[Harry McCleave, 2014-03-17 21:27:46]

On 2014/03/17 01:33:09, tfarina wrote:
> Harry, could you comment on the ash dependency on views' examples?
> 
> https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp
> File ash/ash.gyp (right):
> 
> https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp#newcode845
> ash/ash.gyp:845:
> '../ui/views/examples/examples.gyp:views_examples_with_content_lib',
> On 2014/03/16 23:45:49, tapted wrote:
> > Having ash_unittests depending on examples.gyp feels a bit odd - how much
> > explodes if this is just removed? Maybe some transitive dependencies just
need
> > to be moved in here.
> I will try removing it and see what happens.
> 
> This was added by Harry -
> https://chromiumcodereview.appspot.com/10834185/diff/7002/ash/ash.gyp

This was added for a test in ash_unittests so long as that is not broken
(evidenced by try bots) lgtm.

[Ben Goodger (Google), 2014-03-18 15:59:02]

https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp
File ash/ash.gyp (right):

https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp#newcode845
ash/ash.gyp:845:
'../ui/views/examples/examples.gyp:views_examples_with_content_lib',
Should work without it. Only ash_shell needs examples.

[tfarina, 2014-03-19 04:34:18]

https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp
File ash/ash.gyp (right):

https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp#newcode845
ash/ash.gyp:845:
'../ui/views/examples/examples.gyp:views_examples_with_content_lib',
On 2014/03/18 15:59:03, Ben Goodger (Google) wrote:
> Should work without it. Only ash_shell needs examples.

Done.

[tfarina, 2014-03-19 14:18:06]

Harry, Ben, see below why ash_unittests needs (for now?) examples to compile.

I think making it not depend on examples library is beyond the scope of this CL.

wyt?

https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp
File ash/ash.gyp (right):

https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp#newcode845
ash/ash.gyp:845:
'../ui/views/examples/examples.gyp:views_examples_with_content_lib',
On 2014/03/19 04:34:18, tfarina wrote:
> On 2014/03/18 15:59:03, Ben Goodger (Google) wrote:
> > Should work without it. Only ash_shell needs examples.
> 
> Done.
Actually I reverted this change, see below why:

If we remove this dep, then it does not compile:

../../ash/shell/app_list.cc:129: error: undefined reference to
'views::examples::ShowExamplesWindowWithContent(views::examples::Operation,
content::BrowserContext*, aura::Window*)'
../../ash/shell/window_type_launcher.cc:357: error: undefined reference to
'views::examples::ShowExamplesWindowWithContent(views::examples::Operation,
content::BrowserContext*, aura::Window*)'

This is because we are compiling ash/shell files in ash_unittests in order to
support ash/shell/window_watcher_unittest.cc.

If we remove the ash/shell/ files from ash_unittests, then
window_watcher_unittest.cc won't compile (of course).

../../ash/session_state_delegate_stub.cc:51: error: undefined reference to
'ash::shell::CreateLockScreen()'
../../ash/shell/window_watcher_unittest.cc:25: error: undefined reference to
'ash::shell::ShellDelegateImpl::ShellDelegateImpl()'
../../ash/shell/window_watcher_unittest.cc:32: error: undefined reference to
'ash::shell::WindowWatcher::WindowWatcher()'
../../ash/shell/window_watcher_unittest.cc:34: error: undefined reference to
'ash::shell::ShellDelegateImpl::SetWatcher(ash::shell::WindowWatcher*)'
../../ash/shell/window_watcher_unittest.cc:37: error: undefined reference to
'ash::shell::ShellDelegateImpl::SetWatcher(ash::shell::WindowWatcher*)'

I'm happy to address this in a follow up CL though...

[Harry McCleave, 2014-03-20 19:46:29]

On 2014/03/19 14:18:06, tfarina wrote:
> Harry, Ben, see below why ash_unittests needs (for now?) examples to compile.
> 
> I think making it not depend on examples library is beyond the scope of this
CL.
> 
> wyt?
> 
> https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp
> File ash/ash.gyp (right):
> 
> https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp#newcode845
> ash/ash.gyp:845:
> '../ui/views/examples/examples.gyp:views_examples_with_content_lib',
> On 2014/03/19 04:34:18, tfarina wrote:
> > On 2014/03/18 15:59:03, Ben Goodger (Google) wrote:
> > > Should work without it. Only ash_shell needs examples.
> > 
> > Done.
> Actually I reverted this change, see below why:
> 
> If we remove this dep, then it does not compile:
> 
> ../../ash/shell/app_list.cc:129: error: undefined reference to
> 'views::examples::ShowExamplesWindowWithContent(views::examples::Operation,
> content::BrowserContext*, aura::Window*)'
> ../../ash/shell/window_type_launcher.cc:357: error: undefined reference to
> 'views::examples::ShowExamplesWindowWithContent(views::examples::Operation,
> content::BrowserContext*, aura::Window*)'
> 
> This is because we are compiling ash/shell files in ash_unittests in order to
> support ash/shell/window_watcher_unittest.cc.
> 
> If we remove the ash/shell/ files from ash_unittests, then
> window_watcher_unittest.cc won't compile (of course).
> 
> ../../ash/session_state_delegate_stub.cc:51: error: undefined reference to
> 'ash::shell::CreateLockScreen()'
> ../../ash/shell/window_watcher_unittest.cc:25: error: undefined reference to
> 'ash::shell::ShellDelegateImpl::ShellDelegateImpl()'
> ../../ash/shell/window_watcher_unittest.cc:32: error: undefined reference to
> 'ash::shell::WindowWatcher::WindowWatcher()'
> ../../ash/shell/window_watcher_unittest.cc:34: error: undefined reference to
> 'ash::shell::ShellDelegateImpl::SetWatcher(ash::shell::WindowWatcher*)'
> ../../ash/shell/window_watcher_unittest.cc:37: error: undefined reference to
> 'ash::shell::ShellDelegateImpl::SetWatcher(ash::shell::WindowWatcher*)'
> 
> I'm happy to address this in a follow up CL though...

fine by me, Ben?

[Ben Goodger (Google), 2014-03-21 19:42:58]

On 2014/03/20 19:46:29, Harry McCleave wrote:
> On 2014/03/19 14:18:06, tfarina wrote:
> > Harry, Ben, see below why ash_unittests needs (for now?) examples to
compile.
> > 
> > I think making it not depend on examples library is beyond the scope of this
> CL.
> > 
> > wyt?
> > 
> > https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp
> > File ash/ash.gyp (right):
> > 
> > https://codereview.chromium.org/201093002/diff/60001/ash/ash.gyp#newcode845
> > ash/ash.gyp:845:
> > '../ui/views/examples/examples.gyp:views_examples_with_content_lib',
> > On 2014/03/19 04:34:18, tfarina wrote:
> > > On 2014/03/18 15:59:03, Ben Goodger (Google) wrote:
> > > > Should work without it. Only ash_shell needs examples.
> > > 
> > > Done.
> > Actually I reverted this change, see below why:
> > 
> > If we remove this dep, then it does not compile:
> > 
> > ../../ash/shell/app_list.cc:129: error: undefined reference to
> > 'views::examples::ShowExamplesWindowWithContent(views::examples::Operation,
> > content::BrowserContext*, aura::Window*)'
> > ../../ash/shell/window_type_launcher.cc:357: error: undefined reference to
> > 'views::examples::ShowExamplesWindowWithContent(views::examples::Operation,
> > content::BrowserContext*, aura::Window*)'
> > 
> > This is because we are compiling ash/shell files in ash_unittests in order
to
> > support ash/shell/window_watcher_unittest.cc.
> > 
> > If we remove the ash/shell/ files from ash_unittests, then
> > window_watcher_unittest.cc won't compile (of course).
> > 
> > ../../ash/session_state_delegate_stub.cc:51: error: undefined reference to
> > 'ash::shell::CreateLockScreen()'
> > ../../ash/shell/window_watcher_unittest.cc:25: error: undefined reference to
> > 'ash::shell::ShellDelegateImpl::ShellDelegateImpl()'
> > ../../ash/shell/window_watcher_unittest.cc:32: error: undefined reference to
> > 'ash::shell::WindowWatcher::WindowWatcher()'
> > ../../ash/shell/window_watcher_unittest.cc:34: error: undefined reference to
> > 'ash::shell::ShellDelegateImpl::SetWatcher(ash::shell::WindowWatcher*)'
> > ../../ash/shell/window_watcher_unittest.cc:37: error: undefined reference to
> > 'ash::shell::ShellDelegateImpl::SetWatcher(ash::shell::WindowWatcher*)'
> > 
> > I'm happy to address this in a follow up CL though...
> 
> fine by me, Ben?

OK. LGTM to do in a followup cl. That seems like a bad dep. ash/shell should be
a cul de sac.

[tfarina, 2014-03-22 03:12:36]

Committed patchset #4 manually as r258758 (presubmit successful).