Second attempt to land change to remove NativeViewportService and

Second attempt to land change to remove NativeViewportService and
ViewManager from mojo_shell. First attempt here:
https://codereview.chromium.org/477923004/

Failed the linux gn dbg build after cq landed:
http://build.chromium.org/p/chromium.linux/builders/Linux%20GN%20%28dbg%29/builds/12251

BUG=
TBR=ben@chromium.org

Committed: https://crrev.com/d3e622b0bb31f20c274bc9ad634b8ee7bae99ac1
Cr-Commit-Position: refs/heads/master@{#295230}

[DaveMoore, 2014-09-15 20:58:21]

Get GN component build working

[DaveMoore, 2014-09-15 21:24:14]

Clean up gn

[DaveMoore, 2014-09-15 23:33:32]

Fix extra export decl

[DaveMoore, 2014-09-15 23:33:45]

The CQ bit was checked by davemoore@chromium.org

[DaveMoore, 2014-09-15 23:33:57]

The CQ bit was unchecked by davemoore@chromium.org

[DaveMoore, 2014-09-16 03:46:45]

Fix Windows component build

[DaveMoore, 2014-09-16 03:47:56]

davemoore@chromium.org changed reviewers:
+ jamesr@chromium.org

[DaveMoore, 2014-09-16 03:47:57]

The code is the same from the previous attempt but I've made changes to the GN
build. PTAL.

[jamesr, 2014-09-16 19:19:21]

https://codereview.chromium.org/568883003/diff/80001/mojo/mojo.gyp
File mojo/mojo.gyp (right):

https://codereview.chromium.org/568883003/diff/80001/mojo/mojo.gyp#newcode246
mojo/mojo.gyp:246: ['component=="shared_library"', {
why is this dependency component build only?  if shell/desktop/mojo_main.cc uses
things from ui/gfx/, it should declare the dependency on ui/gfx/ in the static
and component builds. if it doesn't, it shouldn't have a dependency in either

https://codereview.chromium.org/568883003/diff/80001/mojo/shell/desktop/mojo_...
File mojo/shell/desktop/mojo_main.cc (right):

https://codereview.chromium.org/568883003/diff/80001/mojo/shell/desktop/mojo_...
mojo/shell/desktop/mojo_main.cc:14: #include "ui/gfx/switches.h"
looks like we do use ui/gfx/ so we should have a dependency on it declared in
the build system

[DaveMoore, 2014-09-16 19:59:24]

Get BUILD.gn files to pass gn check

[DaveMoore, 2014-09-16 20:11:37]

Get BUILD.gn files to pass gn check

[DaveMoore, 2014-09-16 20:12:37]

https://codereview.chromium.org/568883003/diff/80001/mojo/mojo.gyp
File mojo/mojo.gyp (right):

https://codereview.chromium.org/568883003/diff/80001/mojo/mojo.gyp#newcode246
mojo/mojo.gyp:246: ['component=="shared_library"', {
On 2014/09/16 19:19:20, jamesr wrote:
> why is this dependency component build only?  if shell/desktop/mojo_main.cc
uses
> things from ui/gfx/, it should declare the dependency on ui/gfx/ in the static
> and component builds. if it doesn't, it shouldn't have a dependency in either

I was protecting the call with an #ifdef but not the include. Changed to do that
now.

https://codereview.chromium.org/568883003/diff/80001/mojo/shell/desktop/mojo_...
File mojo/shell/desktop/mojo_main.cc (right):

https://codereview.chromium.org/568883003/diff/80001/mojo/shell/desktop/mojo_...
mojo/shell/desktop/mojo_main.cc:14: #include "ui/gfx/switches.h"
On 2014/09/16 19:19:20, jamesr wrote:
> looks like we do use ui/gfx/ so we should have a dependency on it declared in
> the build system

Done.

[DaveMoore, 2014-09-16 20:42:46]

Rebase

[jamesr, 2014-09-16 22:56:55]

lgtm

[DaveMoore, 2014-09-16 22:59:09]

The CQ bit was checked by davemoore@chromium.org

[commit-bot: I haz the power, 2014-09-16 23:01:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/568883003/140001

[commit-bot: I haz the power, 2014-09-17 00:51:27]

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

[commit-bot: I haz the power, 2014-09-17 00:51:29]

Failed to apply patch for mojo/services/BUILD.gn:
While running git apply --index -p1;
  error: patch failed: mojo/services/BUILD.gn:8
  error: mojo/services/BUILD.gn: patch does not apply

Patch:       mojo/services/BUILD.gn
Index: mojo/services/BUILD.gn
diff --git a/mojo/services/BUILD.gn b/mojo/services/BUILD.gn
index
ae16a08414f0728283c6f2295276a54fc9555ca6..0c8796d8790c7beda645ea638aaa338d5ee76a1a
100644
--- a/mojo/services/BUILD.gn
+++ b/mojo/services/BUILD.gn
@@ -8,7 +8,6 @@ group("services") {
   deps = [
     "//mojo/services/gles2:bindings",
     "//mojo/services/html_viewer",
-    "//mojo/services/native_viewport",
     "//mojo/services/network",
     "//mojo/services/public/interfaces/content_handler",
     "//mojo/services/public/interfaces/geometry",
@@ -20,10 +19,14 @@ group("services") {
     "//mojo/services/surfaces",
     "//mojo/services/test_service:bindings",
   ]
+  if (!is_android) {
+    deps += ["//mojo/services/native_viewport"]
+  }
   if (use_aura) {
     deps += [
       "//mojo/services/public/interfaces/view_manager",
       "//mojo/services/public/interfaces/window_manager",
+      "//mojo/services/view_manager",
       "//mojo/services/window_manager",
     ]
   }

[DaveMoore, 2014-09-17 01:47:27]

The CQ bit was checked by davemoore@chromium.org

[commit-bot: I haz the power, 2014-09-17 01:48:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/568883003/140001

[commit-bot: I haz the power, 2014-09-17 01:49:44]

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

[commit-bot: I haz the power, 2014-09-17 01:49:45]

Failed to apply patch for mojo/services/BUILD.gn:
While running git apply --index -p1;
  error: patch failed: mojo/services/BUILD.gn:8
  error: mojo/services/BUILD.gn: patch does not apply

Patch:       mojo/services/BUILD.gn
Index: mojo/services/BUILD.gn
diff --git a/mojo/services/BUILD.gn b/mojo/services/BUILD.gn
index
ae16a08414f0728283c6f2295276a54fc9555ca6..0c8796d8790c7beda645ea638aaa338d5ee76a1a
100644
--- a/mojo/services/BUILD.gn
+++ b/mojo/services/BUILD.gn
@@ -8,7 +8,6 @@ group("services") {
   deps = [
     "//mojo/services/gles2:bindings",
     "//mojo/services/html_viewer",
-    "//mojo/services/native_viewport",
     "//mojo/services/network",
     "//mojo/services/public/interfaces/content_handler",
     "//mojo/services/public/interfaces/geometry",
@@ -20,10 +19,14 @@ group("services") {
     "//mojo/services/surfaces",
     "//mojo/services/test_service:bindings",
   ]
+  if (!is_android) {
+    deps += ["//mojo/services/native_viewport"]
+  }
   if (use_aura) {
     deps += [
       "//mojo/services/public/interfaces/view_manager",
       "//mojo/services/public/interfaces/window_manager",
+      "//mojo/services/view_manager",
       "//mojo/services/window_manager",
     ]
   }

[DaveMoore, 2014-09-17 04:02:46]

Rebase

[DaveMoore, 2014-09-17 04:03:12]

The CQ bit was checked by davemoore@chromium.org

[commit-bot: I haz the power, 2014-09-17 04:04:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/568883003/160001

[commit-bot: I haz the power, 2014-09-17 05:01:54]

Committed patchset #9 (id:160001) as 8efe4a0f5a610b04b1370fa9a97bfd06a713291e

[commit-bot: I haz the power, 2014-09-17 05:02:33]

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

[horo, 2014-09-17 08:20:49]

A revert of this CL (patchset #9 id:160001) has been created in
https://codereview.chromium.org/578793002/ by horo@chromium.org.

The reason for reverting is: ViewManagerTest failure.

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#master=ch....

[battre, 2014-09-17 10:25:30]

A revert of this CL (patchset #9 id:160001) has been created in
https://codereview.chromium.org/574993002/ by battre@chromium.org.

The reason for reverting is: This CL caused timeouts on the following tests:

ViewManagerTest.SetViewVisibility
ViewManagerTest.SecondEmbedRoot_Service
ViewManagerTest.EmbedWithSameViewId2
ViewManagerTest.EmbedWithSameViewId
ViewManagerTest.CantGetViewTreeOfOtherRoots
ViewManagerTest.SetViewVisibilityNotifications
ViewManagerTest.SetViewBounds
ViewManagerTest.ViewHierarchyChangedAddingKnownToUnknown
ViewManagerTest.GetViewTree
ViewManagerTest.MultipleEmbedRootsBeforeWTHReady
ViewManagerTest.ReuseDeletedViewId
ViewManagerTest.CantRemoveViewsInOtherRoots
ViewManagerTest.ReorderView
ViewManagerTest.DeleteViewFromAnotherConnectionDisallowed
ViewManagerTest.DeleteView
ViewManagerTest.OnViewInput
ViewManagerTest.CantMoveViewsFromOtherRoot
ViewManagerTest.SecondEmbedRoot_InitService
ViewManagerTest.AddToRoot
ViewManagerTest.ViewHierarchyChangedViews

on Linux and Windows builders..