Move navigation and frame tree classes to a new frame_host/ directory.

Move navigation and frame tree classes to a new frame_host/ directory.

This CL moves all frame tree classes, Navigation* and its dependencies from web_contents/ to a separate frame_host/ directory.

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

[nasko, 2013-10-28 21:35:32]

Hey guys,
This is initial stab at moving all relevant files to frame_host/. This CL does
allow InterstitialPageImpl to include web_contents* stuff, which is gated on
results of discussions in https://codereview.chromium.org/45453002/.
Thanks,
Nasko

[jam, 2013-10-29 01:25:59]

lgtm

https://codereview.chromium.org/49823002/diff/1/content/browser/frame_host/DEPS
File content/browser/frame_host/DEPS (right):

https://codereview.chromium.org/49823002/diff/1/content/browser/frame_host/DE...
content/browser/frame_host/DEPS:9: 
nit: extra blank line

https://codereview.chromium.org/49823002/diff/1/content/browser/frame_host/DE...
content/browser/frame_host/DEPS:14: # investigate how to remove it.
nit: it's ok if tests depend on other layers, we have this with renderer_host
for example

https://codereview.chromium.org/49823002/diff/1/content/browser/frame_host/DE...
content/browser/frame_host/DEPS:16:
"+content/browser/web_contents/web_contents_impl",
nit: this isn't necessary given the line above?

https://codereview.chromium.org/49823002/diff/1/content/browser/frame_host/DE...
content/browser/frame_host/DEPS:24:
"+content/browser/web_contents/web_contents_impl",
ditto

[nasko, 2013-10-29 13:19:53]

Fixed DEPS.

https://codereview.chromium.org/49823002/diff/1/content/browser/frame_host/DEPS
File content/browser/frame_host/DEPS (right):

https://codereview.chromium.org/49823002/diff/1/content/browser/frame_host/DE...
content/browser/frame_host/DEPS:9: 
On 2013/10/29 01:26:00, jam wrote:
> nit: extra blank line

Done.

https://codereview.chromium.org/49823002/diff/1/content/browser/frame_host/DE...
content/browser/frame_host/DEPS:14: # investigate how to remove it.
On 2013/10/29 01:26:00, jam wrote:
> nit: it's ok if tests depend on other layers, we have this with renderer_host
> for example

Done.

https://codereview.chromium.org/49823002/diff/1/content/browser/frame_host/DE...
content/browser/frame_host/DEPS:16:
"+content/browser/web_contents/web_contents_impl",
On 2013/10/29 01:26:00, jam wrote:
> nit: this isn't necessary given the line above?

Done.

https://codereview.chromium.org/49823002/diff/1/content/browser/frame_host/DE...
content/browser/frame_host/DEPS:24:
"+content/browser/web_contents/web_contents_impl",
On 2013/10/29 01:26:00, jam wrote:
> ditto

Done.

[commit-bot: I haz the power, 2013-10-29 13:20:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nasko@chromium.org/49823002/60001

[commit-bot: I haz the power, 2013-10-29 13:20:42]

Failed to apply patch for content/content_browser.gypi:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/content_browser.gypi
  Hunk #2 FAILED at 810.
  Hunk #4 succeeded at 1159 (offset 4 lines).
  1 out of 4 hunks FAILED -- saving rejects to file
content/content_browser.gypi.rej

Patch:       content/content_browser.gypi
Index: content/content_browser.gypi
diff --git a/content/content_browser.gypi b/content/content_browser.gypi
index
33341b92250305e95bc607c2ff5393533544a013..c6254bb3b447fddad5cddfe75a18d18462f83115
100644
--- a/content/content_browser.gypi
+++ b/content/content_browser.gypi
@@ -501,6 +501,27 @@
     'browser/fileapi/fileapi_message_filter.cc',
     'browser/fileapi/fileapi_message_filter.h',
     'browser/font_list_async.cc',
+    'browser/frame_host/debug_urls.cc',
+    'browser/frame_host/debug_urls.h',
+    'browser/frame_host/frame_tree.cc',
+    'browser/frame_host/frame_tree.h',
+    'browser/frame_host/frame_tree_node.cc',
+    'browser/frame_host/frame_tree_node.h',
+    'browser/frame_host/interstitial_page_impl.cc',
+    'browser/frame_host/interstitial_page_impl.h',
+    'browser/frame_host/navigation_controller_delegate.h',
+    'browser/frame_host/navigation_controller_impl.cc',
+    'browser/frame_host/navigation_controller_impl.h',
+    'browser/frame_host/navigation_entry_impl.cc',
+    'browser/frame_host/navigation_entry_impl.h',
+    'browser/frame_host/render_frame_host_impl.cc',
+    'browser/frame_host/render_frame_host_impl.h',
+    'browser/frame_host/render_frame_message_filter.cc',
+    'browser/frame_host/render_frame_message_filter.h',
+    'browser/frame_host/render_view_host_manager.cc',
+    'browser/frame_host/render_view_host_manager.h',
+    'browser/frame_host/web_contents_screenshot_manager.cc',
+    'browser/frame_host/web_contents_screenshot_manager.h',
     'browser/gamepad/gamepad_data_fetcher.h',
     'browser/gamepad/gamepad_platform_data_fetcher.h',
     'browser/gamepad/gamepad_platform_data_fetcher_linux.cc',
@@ -789,10 +810,6 @@
     'browser/renderer_host/file_utilities_message_filter.h',
     'browser/renderer_host/software_frame_manager.cc',
     'browser/renderer_host/software_frame_manager.h',
-    'browser/renderer_host/frame_tree.cc',
-    'browser/renderer_host/frame_tree.h',
-    'browser/renderer_host/frame_tree_node.cc',
-    'browser/renderer_host/frame_tree_node.h',
     'browser/renderer_host/gamepad_browser_message_filter.cc',
     'browser/renderer_host/gamepad_browser_message_filter.h',
     'browser/renderer_host/gpu_message_filter.cc',
@@ -974,10 +991,6 @@
     'browser/renderer_host/pepper/ssl_context_helper.h',
     'browser/renderer_host/popup_menu_helper_mac.h',
     'browser/renderer_host/popup_menu_helper_mac.mm',
-    'browser/renderer_host/render_frame_host_impl.cc',
-    'browser/renderer_host/render_frame_host_impl.h',
-    'browser/renderer_host/render_frame_message_filter.cc',
-    'browser/renderer_host/render_frame_message_filter.h',
     'browser/renderer_host/render_message_filter.cc',
     'browser/renderer_host/render_message_filter.h',
     'browser/renderer_host/render_process_host_impl.cc',
@@ -1138,27 +1151,14 @@
     'browser/web_contents/aura/shadow_layer_delegate.h',
     'browser/web_contents/aura/window_slider.cc',
     'browser/web_contents/aura/window_slider.h',
-    'browser/web_contents/debug_urls.cc',
-    'browser/web_contents/debug_urls.h',
     'browser/web_contents/drag_utils_gtk.cc',
     'browser/web_contents/drag_utils_gtk.h',
-    'browser/web_contents/interstitial_page_impl.cc',
-    'browser/web_contents/interstitial_page_impl.h',
-    'browser/web_contents/navigation_controller_delegate.h',
-    'browser/web_contents/navigation_controller_impl.cc',
-    'browser/web_contents/navigation_controller_impl.h',
-    'browser/web_contents/navigation_entry_impl.cc',
-    'browser/web_contents/navigation_entry_impl.h',
-    'browser/web_contents/render_view_host_manager.cc',
-    'browser/web_contents/render_view_host_manager.h',
     'browser/web_contents/touch_editable_impl_aura.cc',
     'browser/web_contents/touch_editable_impl_aura.h',
     'browser/web_contents/web_contents_drag_win.cc',
     'browser/web_contents/web_contents_drag_win.h',
     'browser/web_contents/web_contents_impl.cc',
     'browser/web_contents/web_contents_impl.h',
-    'browser/web_contents/web_contents_screenshot_manager.cc',
-    'browser/web_contents/web_contents_screenshot_manager.h',
     'browser/web_contents/web_contents_view_android.cc',
     'browser/web_contents/web_contents_view_android.h',
     'browser/web_contents/web_contents_view_aura.cc',

[commit-bot: I haz the power, 2013-10-29 13:24:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nasko@chromium.org/49823002/110001

[commit-bot: I haz the power, 2013-10-29 13:57:55]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-10-29 15:09:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nasko@chromium.org/49823002/50003

[Charlie Reis, 2013-10-29 15:53:59]

Perhaps we should prevent renderer_host from depending on frame_host, with some
exceptions / TODOs for the things that still need to change (RenderViewHost,
RenderProcessHost, etc).

https://codereview.chromium.org/49823002/diff/50003/content/browser/renderer_...
File content/browser/renderer_host/render_view_host_impl.cc (right):

https://codereview.chromium.org/49823002/diff/50003/content/browser/renderer_...
content/browser/renderer_host/render_view_host_impl.cc:30: #include
"content/browser/frame_host/frame_tree.h"
Hmm, do we want renderer_host to depend on frame_host?

https://codereview.chromium.org/49823002/diff/50003/content/browser/renderer_...
File content/browser/renderer_host/render_view_host_impl.h (right):

https://codereview.chromium.org/49823002/diff/50003/content/browser/renderer_...
content/browser/renderer_host/render_view_host_impl.h:17: #include
"content/browser/frame_host/render_frame_host_impl.h"
Ditto.

https://codereview.chromium.org/49823002/diff/50003/content/browser/renderer_...
File content/browser/renderer_host/render_view_host_unittest.cc (right):

https://codereview.chromium.org/49823002/diff/50003/content/browser/renderer_...
content/browser/renderer_host/render_view_host_unittest.cc:8: #include
"content/browser/frame_host/navigation_controller_impl.h"
Ditto.

[commit-bot: I haz the power, 2013-10-29 15:57:24]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[nasko, 2013-10-29 16:14:21]

Charlie,
I added exceptions to the DEPS for renderer_host/, since we can't break the
dependencies just yet.
Thanks,
Nasko

https://codereview.chromium.org/49823002/diff/50003/content/browser/renderer_...
File content/browser/renderer_host/render_view_host_impl.cc (right):

https://codereview.chromium.org/49823002/diff/50003/content/browser/renderer_...
content/browser/renderer_host/render_view_host_impl.cc:30: #include
"content/browser/frame_host/frame_tree.h"
On 2013/10/29 15:54:00, creis wrote:
> Hmm, do we want renderer_host to depend on frame_host?

We don't want it long term, but we cannot prevent it until we have refactored
code such that the main RenderFrameHost is created by the FrameTree and not the
RVH.

https://codereview.chromium.org/49823002/diff/50003/content/browser/renderer_...
File content/browser/renderer_host/render_view_host_impl.h (right):

https://codereview.chromium.org/49823002/diff/50003/content/browser/renderer_...
content/browser/renderer_host/render_view_host_impl.h:17: #include
"content/browser/frame_host/render_frame_host_impl.h"
On 2013/10/29 15:54:00, creis wrote:
> Ditto.

Done.

https://codereview.chromium.org/49823002/diff/50003/content/browser/renderer_...
File content/browser/renderer_host/render_view_host_unittest.cc (right):

https://codereview.chromium.org/49823002/diff/50003/content/browser/renderer_...
content/browser/renderer_host/render_view_host_unittest.cc:8: #include
"content/browser/frame_host/navigation_controller_impl.h"
On 2013/10/29 15:54:00, creis wrote:
> Ditto.

It isn't even needed : )

[Charlie Reis, 2013-10-29 16:36:38]

LGTM with comments below.

https://codereview.chromium.org/49823002/diff/230002/content/browser/frame_ho...
File content/browser/frame_host/web_contents_screenshot_manager.h (right):

https://codereview.chromium.org/49823002/diff/230002/content/browser/frame_ho...
content/browser/frame_host/web_contents_screenshot_manager.h:26: class
CONTENT_EXPORT WebContentsScreenshotManager {
Let's add a TODO to rename this to something like
NavigationEntryScreenshotManager, since "WebContents" no longer makes sense in
the frame_host directory.

https://codereview.chromium.org/49823002/diff/230002/content/content_browser....
File content/content_browser.gypi (right):

https://codereview.chromium.org/49823002/diff/230002/content/content_browser....
content/content_browser.gypi:814:
'browser/renderer_host/software_frame_manager.h',
Merge conflict?

[nasko, 2013-10-29 16:47:21]

https://codereview.chromium.org/49823002/diff/230002/content/browser/frame_ho...
File content/browser/frame_host/web_contents_screenshot_manager.h (right):

https://codereview.chromium.org/49823002/diff/230002/content/browser/frame_ho...
content/browser/frame_host/web_contents_screenshot_manager.h:26: class
CONTENT_EXPORT WebContentsScreenshotManager {
On 2013/10/29 16:36:39, creis wrote:
> Let's add a TODO to rename this to something like
> NavigationEntryScreenshotManager, since "WebContents" no longer makes sense in
> the frame_host directory.

Done.

https://codereview.chromium.org/49823002/diff/230002/content/content_browser....
File content/content_browser.gypi (right):

https://codereview.chromium.org/49823002/diff/230002/content/content_browser....
content/content_browser.gypi:814:
'browser/renderer_host/software_frame_manager.h',
On 2013/10/29 16:36:39, creis wrote:
> Merge conflict?

Wrong resolution of one, yes. Thanks for catching this one!

[commit-bot: I haz the power, 2013-10-30 17:15:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nasko@chromium.org/49823002/620001

[commit-bot: I haz the power, 2013-10-30 21:06:42]

Change committed as 231921