Move GuestView layer in browser to components

Move GuestView layer in browser to components

In addition to moving the browser-side of the GuestView layer into components, this CL also:

1. Replaces some loops to the top level web contents with GuestViewBase::GetTopLevelWebContents.
2. Installs the ExtensionsGuestViewMessageFilter in appshell.
3. Move some files out of extensions/common/guest_view to components/guest_view/common

BUG=444869
Committed: https://crrev.com/8dfa19acbffebaeafd38e8c3f4ef136c3fc83734
Cr-Commit-Position: refs/heads/master@{#328243}

[Fady Samuel, 2015-04-25 20:31:30]

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

[commit-bot: I haz the power, 2015-04-25 20:31:37]

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

[commit-bot: I haz the power, 2015-04-25 20:37:55]

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

[commit-bot: I haz the power, 2015-04-25 20:37:57]

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

[Fady Samuel, 2015-04-25 21:16:39]

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

[commit-bot: I haz the power, 2015-04-25 21:16:45]

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

[commit-bot: I haz the power, 2015-04-25 21:23:03]

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

[commit-bot: I haz the power, 2015-04-25 21:23:04]

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

[Fady Samuel, 2015-04-25 22:04:14]

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

[commit-bot: I haz the power, 2015-04-25 22:04:17]

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

[commit-bot: I haz the power, 2015-04-25 22:12:14]

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

[commit-bot: I haz the power, 2015-04-25 22:12:15]

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

[Fady Samuel, 2015-04-25 22:29:19]

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

[commit-bot: I haz the power, 2015-04-25 22:29:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/60001

[commit-bot: I haz the power, 2015-04-25 22:36:33]

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

[commit-bot: I haz the power, 2015-04-25 22:36:34]

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

[Fady Samuel, 2015-04-26 02:16:14]

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

[commit-bot: I haz the power, 2015-04-26 02:16:25]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/80001

[commit-bot: I haz the power, 2015-04-26 02:23:24]

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

[commit-bot: I haz the power, 2015-04-26 02:23:25]

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

[Fady Samuel, 2015-04-26 02:44:58]

Notes to self

https://codereview.chromium.org/1102173002/diff/80001/chrome/browser/printing...
File chrome/browser/printing/print_preview_dialog_controller.cc (left):

https://codereview.chromium.org/1102173002/diff/80001/chrome/browser/printing...
chrome/browser/printing/print_preview_dialog_controller.cc:107: while
(guest_view && guest_view->attached()) {
Note: Use GuestViewBase::GetTopMostWebContents

https://codereview.chromium.org/1102173002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/chrome_constrained_window_views_client.cc (left):

https://codereview.chromium.org/1102173002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/chrome_constrained_window_views_client.cc:23:
content::WebContents* initiator_web_contents) override {
Get rid of the call into chrome.

https://codereview.chromium.org/1102173002/diff/80001/components/components_t...
File components/components_tests.gyp (right):

https://codereview.chromium.org/1102173002/diff/80001/components/components_t...
components/components_tests.gyp:221:
'guest_view/browser/guest_view_manager_unittest.cc'
try running guest_view_manager_unittest

[Fady Samuel, 2015-04-26 04:29:17]

Addressed my past self's comments.

https://codereview.chromium.org/1102173002/diff/80001/chrome/browser/printing...
File chrome/browser/printing/print_preview_dialog_controller.cc (left):

https://codereview.chromium.org/1102173002/diff/80001/chrome/browser/printing...
chrome/browser/printing/print_preview_dialog_controller.cc:107: while
(guest_view && guest_view->attached()) {
On 2015/04/26 02:44:58, Fady Samuel wrote:
> Note: Use GuestViewBase::GetTopMostWebContents

Done.

https://codereview.chromium.org/1102173002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/chrome_constrained_window_views_client.cc (left):

https://codereview.chromium.org/1102173002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/chrome_constrained_window_views_client.cc:23:
content::WebContents* initiator_web_contents) override {
On 2015/04/26 02:44:58, Fady Samuel wrote:
> Get rid of the call into chrome.

Done.

https://codereview.chromium.org/1102173002/diff/80001/components/components_t...
File components/components_tests.gyp (right):

https://codereview.chromium.org/1102173002/diff/80001/components/components_t...
components/components_tests.gyp:221:
'guest_view/browser/guest_view_manager_unittest.cc'
On 2015/04/26 02:44:58, Fady Samuel wrote:
> try running guest_view_manager_unittest

Done.

[Fady Samuel, 2015-04-26 04:29:27]

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

[commit-bot: I haz the power, 2015-04-26 04:29:32]

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

[commit-bot: I haz the power, 2015-04-26 04:36:03]

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

[commit-bot: I haz the power, 2015-04-26 04:36:04]

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

[Fady Samuel, 2015-04-26 04:45:28]

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

[commit-bot: I haz the power, 2015-04-26 04:45:37]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/120001

[commit-bot: I haz the power, 2015-04-26 04:53:48]

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

[commit-bot: I haz the power, 2015-04-26 04:53:49]

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

[Fady Samuel, 2015-04-26 12:27:00]

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

[commit-bot: I haz the power, 2015-04-26 12:27:05]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/140001

[commit-bot: I haz the power, 2015-04-26 12:34:05]

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

[commit-bot: I haz the power, 2015-04-26 12:34:06]

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

[Fady Samuel, 2015-04-26 21:14:51]

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

[commit-bot: I haz the power, 2015-04-26 21:14:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/160001

[commit-bot: I haz the power, 2015-04-26 21:19:13]

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

[commit-bot: I haz the power, 2015-04-26 21:19:15]

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

[Fady Samuel, 2015-04-26 22:00:10]

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

[commit-bot: I haz the power, 2015-04-26 22:00:15]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/180001

[commit-bot: I haz the power, 2015-04-26 22:07:14]

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

[commit-bot: I haz the power, 2015-04-26 22:07:15]

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

[Fady Samuel, 2015-04-26 22:20:46]

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

[commit-bot: I haz the power, 2015-04-26 22:20:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/200001

[commit-bot: I haz the power, 2015-04-26 22:27:57]

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

[commit-bot: I haz the power, 2015-04-26 22:28:01]

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

[Fady Samuel, 2015-04-26 22:46:37]

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

[commit-bot: I haz the power, 2015-04-26 22:46:48]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/220001

[commit-bot: I haz the power, 2015-04-26 22:53:55]

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

[commit-bot: I haz the power, 2015-04-26 22:53:56]

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

[Fady Samuel, 2015-04-27 02:44:07]

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

[commit-bot: I haz the power, 2015-04-27 02:44:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/240001

[commit-bot: I haz the power, 2015-04-27 02:51:14]

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

[commit-bot: I haz the power, 2015-04-27 02:51:15]

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

[Fady Samuel, 2015-04-27 04:41:14]

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

[commit-bot: I haz the power, 2015-04-27 04:41:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/260001

[commit-bot: I haz the power, 2015-04-27 04:48:50]

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

[commit-bot: I haz the power, 2015-04-27 04:48:52]

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

[Fady Samuel, 2015-04-27 05:47:06]

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

[commit-bot: I haz the power, 2015-04-27 05:47:14]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/280001

[commit-bot: I haz the power, 2015-04-27 05:53:53]

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

[commit-bot: I haz the power, 2015-04-27 05:53:55]

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

[Fady Samuel, 2015-04-27 23:08:38]

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

[commit-bot: I haz the power, 2015-04-27 23:08:57]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/300001

[commit-bot: I haz the power, 2015-04-27 23:24:11]

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

[commit-bot: I haz the power, 2015-04-27 23:24:12]

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

[Fady Samuel, 2015-04-28 22:48:10]

fsamuel@chromium.org changed reviewers:
+ lazyboy@chromium.org

[lazyboy, 2015-04-30 19:31:07]

Looks OK
Haven't looked at BUILD/DEPS/gn/gypi closely.

https://chromiumcodereview.appspot.com/1102173002/diff/300001/components/gues...
File components/guest_view/BUILD.gn (right):

https://chromiumcodereview.appspot.com/1102173002/diff/300001/components/gues...
components/guest_view/BUILD.gn:1: # Copyright 2014 The Chromium Authors. All
rights reserved.
2015

https://chromiumcodereview.appspot.com/1102173002/diff/300001/components/gues...
File components/guest_view/browser/guest_view.h (right):

https://chromiumcodereview.appspot.com/1102173002/diff/300001/components/gues...
components/guest_view/browser/guest_view.h:12: namespace guestview {
based on the directory, this should be "namespace guest_view"

https://chromiumcodereview.appspot.com/1102173002/diff/300001/components/gues...
File components/guest_view/browser/guest_view_manager_factory.h (right):

https://chromiumcodereview.appspot.com/1102173002/diff/300001/components/gues...
components/guest_view/browser/guest_view_manager_factory.h:16: namespace
guestview {
This namespace should be merged with previous one.

https://chromiumcodereview.appspot.com/1102173002/diff/300001/extensions/brow...
File extensions/browser/guest_view/extensions_guest_view_manager_delegate.h
(right):

https://chromiumcodereview.appspot.com/1102173002/diff/300001/extensions/brow...
extensions/browser/guest_view/extensions_guest_view_manager_delegate.h:25: bool
DispatchEvent(const std::string& event_name,
Are we using the returned value in this CL? (I didn't notice any usage)
If not, can this change be left out of the CL?

https://chromiumcodereview.appspot.com/1102173002/diff/300001/extensions/shel...
File extensions/shell/browser/shell_content_browser_client.cc (right):

https://chromiumcodereview.appspot.com/1102173002/diff/300001/extensions/shel...
extensions/shell/browser/shell_content_browser_client.cc:99: host->AddFilter(
Add this change to CL description

[Fady Samuel, 2015-04-30 23:26:05]

PTAL

https://codereview.chromium.org/1102173002/diff/300001/components/guest_view/...
File components/guest_view/BUILD.gn (right):

https://codereview.chromium.org/1102173002/diff/300001/components/guest_view/...
components/guest_view/BUILD.gn:1: # Copyright 2014 The Chromium Authors. All
rights reserved.
On 2015/04/30 19:31:07, lazyboy wrote:
> 2015

Done.

https://codereview.chromium.org/1102173002/diff/300001/components/guest_view/...
File components/guest_view/browser/guest_view.h (right):

https://codereview.chromium.org/1102173002/diff/300001/components/guest_view/...
components/guest_view/browser/guest_view.h:12: namespace guestview {
On 2015/04/30 19:31:07, lazyboy wrote:
> based on the directory, this should be "namespace guest_view"

Done.

https://codereview.chromium.org/1102173002/diff/300001/components/guest_view/...
File components/guest_view/browser/guest_view_manager_factory.h (right):

https://codereview.chromium.org/1102173002/diff/300001/components/guest_view/...
components/guest_view/browser/guest_view_manager_factory.h:16: namespace
guestview {
On 2015/04/30 19:31:07, lazyboy wrote:
> This namespace should be merged with previous one.

Done.

https://codereview.chromium.org/1102173002/diff/300001/extensions/browser/gue...
File extensions/browser/guest_view/extensions_guest_view_manager_delegate.h
(right):

https://codereview.chromium.org/1102173002/diff/300001/extensions/browser/gue...
extensions/browser/guest_view/extensions_guest_view_manager_delegate.h:25: bool
DispatchEvent(const std::string& event_name,
On 2015/04/30 19:31:07, lazyboy wrote:
> Are we using the returned value in this CL? (I didn't notice any usage)
> If not, can this change be left out of the CL?

Done.

https://codereview.chromium.org/1102173002/diff/300001/extensions/shell/brows...
File extensions/shell/browser/shell_content_browser_client.cc (right):

https://codereview.chromium.org/1102173002/diff/300001/extensions/shell/brows...
extensions/shell/browser/shell_content_browser_client.cc:99: host->AddFilter(
On 2015/04/30 19:31:07, lazyboy wrote:
> Add this change to CL description

Done.

[Fady Samuel, 2015-04-30 23:26:24]

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

[commit-bot: I haz the power, 2015-04-30 23:27:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/320001

[commit-bot: I haz the power, 2015-04-30 23:37:46]

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

[commit-bot: I haz the power, 2015-04-30 23:37:48]

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

[lazyboy, 2015-05-01 00:05:03]

*guest_view* LGTM

[Fady Samuel, 2015-05-01 00:29:56]

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

[Fady Samuel, 2015-05-01 00:29:57]

The patchset sent to the CQ was uploaded after l-g-t-m from lazyboy@chromium.org
Link to the patchset: https://codereview.chromium.org/1102173002/#ps340001
(title: "Make bots happy (hopefully)")

[commit-bot: I haz the power, 2015-05-01 00:30:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/340001

[commit-bot: I haz the power, 2015-05-01 00:41:54]

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

[commit-bot: I haz the power, 2015-05-01 00:41:56]

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

[Fady Samuel, 2015-05-01 03:02:01]

fsamuel@chromium.org changed reviewers:
+ kalman@chromium.org, sky@chromium.org

[Fady Samuel, 2015-05-01 03:02:02]

This CL isn't as scary as it looks. It's mostly just moving guest_view into
components. Includes getting updated.

+kalman@ for extensions/ OWNER review (not guest_view)
+sky@ for chrome/ OWNER review (not guest_view)

[sky, 2015-05-01 15:08:47]

LGTM

[not at google - send to devlin, 2015-05-01 15:33:45]

lgtm with some comments as I explore the gyp/gn changes - but you may need a
separate owner for the DEPS files. I can't lg those.

https://codereview.chromium.org/1102173002/diff/340001/extensions/BUILD.gn
File extensions/BUILD.gn (right):

https://codereview.chromium.org/1102173002/diff/340001/extensions/BUILD.gn#ne...
extensions/BUILD.gn:152: "//components/guest_view:test_support",
Should this just go into the test_support set, and not into the individual
unittests/browsertests? Seems like there's some other duplication going on in
this file, too.

https://codereview.chromium.org/1102173002/diff/340001/extensions/extensions_...
File extensions/extensions_tests.gyp (right):

https://codereview.chromium.org/1102173002/diff/340001/extensions/extensions_...
extensions/extensions_tests.gyp:72:
'../components/components.gyp:guest_view_test_support',
I don't know why some of these use ".." and others use "<(DEPTH)", but in this
rule at least, the latter seems preferred.

[Fady Samuel, 2015-05-01 21:20:26]

fsamuel@chromium.org changed reviewers:
+ jam@chromium.org, jochen@chromium.org, kenrb@chromium.org

[Fady Samuel, 2015-05-01 21:20:29]

+jam@ for guest_view DEPS on content and ipc.
+jochen@ for guest_view DEPS on third_party/WebKit overall components changes:

components/BUILD.gn
components/components.gyp
components/constrained_window/BUILD.gn
components/constrained_window/DEPS
components/constrained_window/constrained_window_views.cc
components/constrained_window/constrained_window_views_client.h
components/guest_view.gypi

kenrb@ for move of
components/guest_view/common/guest_view_messages.h

https://codereview.chromium.org/1102173002/diff/340001/extensions/BUILD.gn
File extensions/BUILD.gn (right):

https://codereview.chromium.org/1102173002/diff/340001/extensions/BUILD.gn#ne...
extensions/BUILD.gn:152: "//components/guest_view:test_support",
On 2015/05/01 15:33:45, kalman wrote:
> Should this just go into the test_support set, and not into the individual
> unittests/browsertests? Seems like there's some other duplication going on in
> this file, too.

Done.

https://codereview.chromium.org/1102173002/diff/340001/extensions/extensions_...
File extensions/extensions_tests.gyp (right):

https://codereview.chromium.org/1102173002/diff/340001/extensions/extensions_...
extensions/extensions_tests.gyp:72:
'../components/components.gyp:guest_view_test_support',
On 2015/05/01 15:33:45, kalman wrote:
> I don't know why some of these use ".." and others use "<(DEPTH)", but in this
> rule at least, the latter seems preferred.

Done.

[Fady Samuel, 2015-05-01 21:20:45]

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

[Fady Samuel, 2015-05-01 21:20:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
lazyboy@chromium.org, sky@chromium.org, kalman@chromium.org
Link to the patchset: https://codereview.chromium.org/1102173002/#ps380001
(title: "Removed unnecessary dependency")

[commit-bot: I haz the power, 2015-05-01 21:24:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/380001

[commit-bot: I haz the power, 2015-05-01 21:35:41]

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

[commit-bot: I haz the power, 2015-05-01 21:35:43]

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

[kenrb, 2015-05-02 03:37:24]

lgtm

[jochen (gone - plz use gerrit), 2015-05-04 07:26:03]

https://codereview.chromium.org/1102173002/diff/380001/components/guest_view/...
File components/guest_view/browser/DEPS (right):

https://codereview.chromium.org/1102173002/diff/380001/components/guest_view/...
components/guest_view/browser/DEPS:7: '+third_party/WebKit/public/web',
ideally, browser/ stuff should explicitly list the WebKit headers it depends on,
as we don't link webkit into the browser process (so you can only use POD and
enum headers)

[Fady Samuel, 2015-05-04 13:00:56]

PTAL jochen@ I've addressed your comments.
PTAL jam@ for guest_view DEPS on content and ipc.

[jochen (gone - plz use gerrit), 2015-05-04 13:12:58]

lgtm

[Fady Samuel, 2015-05-04 14:15:05]

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

[Fady Samuel, 2015-05-04 14:15:06]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
kenrb@chromium.org, lazyboy@chromium.org, kalman@chromium.org
Link to the patchset: https://codereview.chromium.org/1102173002/#ps400001
(title: "Addressed Jochen's comment + GuestViewBase cleanup")

[commit-bot: I haz the power, 2015-05-04 14:15:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/400001

[commit-bot: I haz the power, 2015-05-04 14:23:03]

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

[commit-bot: I haz the power, 2015-05-04 14:23:07]

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

[jam, 2015-05-04 16:11:10]

https://codereview.chromium.org/1102173002/diff/400001/components/guest_view/...
File components/guest_view/browser/DEPS (right):

https://codereview.chromium.org/1102173002/diff/400001/components/guest_view/...
components/guest_view/browser/DEPS:4: "+content/public",
dont add content/public, but instead add content/public/common

https://codereview.chromium.org/1102173002/diff/400001/components/guest_view/...
File components/guest_view/browser/guest_view_manager_unittest.cc (right):

https://codereview.chromium.org/1102173002/diff/400001/components/guest_view/...
components/guest_view/browser/guest_view_manager_unittest.cc:10: #include
"content/test/test_render_view_host.h"
you can't include from inside content. why did you change this?

[Fady Samuel, 2015-05-04 20:24:29]

fsamuel@chromium.org changed reviewers:
+ miket@chromium.org

[Fady Samuel, 2015-05-04 20:24:32]

PTAL John! Thanks!

+miket@ for extensions/browser/DEPS review (added guest_view as dependency).

https://codereview.chromium.org/1102173002/diff/400001/components/guest_view/...
File components/guest_view/browser/DEPS (right):

https://codereview.chromium.org/1102173002/diff/400001/components/guest_view/...
components/guest_view/browser/DEPS:4: "+content/public",
On 2015/05/04 16:11:09, jam wrote:
> dont add content/public, but instead add content/public/common 

Done. Also, added content/public/test

https://codereview.chromium.org/1102173002/diff/400001/components/guest_view/...
File components/guest_view/browser/guest_view_manager_unittest.cc (right):

https://codereview.chromium.org/1102173002/diff/400001/components/guest_view/...
components/guest_view/browser/guest_view_manager_unittest.cc:10: #include
"content/test/test_render_view_host.h"
On 2015/05/04 16:11:09, jam wrote:
> you can't include from inside content. why did you change this?

Doh! Meant to include: #include "content/public/test/test_renderer_host.h"

Fixed, thanks!

[Fady Samuel, 2015-05-04 20:25:20]

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

[Fady Samuel, 2015-05-04 20:25:22]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
sky@chromium.org, kenrb@chromium.org, lazyboy@chromium.org, kalman@chromium.org
Link to the patchset: https://codereview.chromium.org/1102173002/#ps420001
(title: "Addressed John's comments")

[commit-bot: I haz the power, 2015-05-04 20:25:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/420001

[commit-bot: I haz the power, 2015-05-04 20:35:27]

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

[commit-bot: I haz the power, 2015-05-04 20:35:30]

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

[jam, 2015-05-04 21:50:41]

On 2015/05/04 20:24:32, Fady Samuel wrote:
> PTAL John! Thanks!
> 
> +miket@ for extensions/browser/DEPS review (added guest_view as dependency).
> 
>
https://codereview.chromium.org/1102173002/diff/400001/components/guest_view/...
> File components/guest_view/browser/DEPS (right):
> 
>
https://codereview.chromium.org/1102173002/diff/400001/components/guest_view/...
> components/guest_view/browser/DEPS:4: "+content/public",
> On 2015/05/04 16:11:09, jam wrote:
> > dont add content/public, but instead add content/public/common 
> 
> Done. Also, added content/public/test
> 
>
https://codereview.chromium.org/1102173002/diff/400001/components/guest_view/...
> File components/guest_view/browser/guest_view_manager_unittest.cc (right):
> 
>
https://codereview.chromium.org/1102173002/diff/400001/components/guest_view/...
> components/guest_view/browser/guest_view_manager_unittest.cc:10: #include
> "content/test/test_render_view_host.h"
> On 2015/05/04 16:11:09, jam wrote:
> > you can't include from inside content. why did you change this?
> 
> Doh! Meant to include: #include "content/public/test/test_renderer_host.h"
> 
> Fixed, thanks!

lgtm

[Fady Samuel, 2015-05-04 22:00:16]

fsamuel@chromium.org changed reviewers:
+ benwells@chromium.org
- miket@chromium.org

[Fady Samuel, 2015-05-04 22:00:18]

-miket@ +benwells, since Mike doesn't do code reviews.

[benwells, 2015-05-04 23:00:13]

On 2015/05/04 22:00:18, Fady Samuel wrote:
> -miket@ +benwells, since Mike doesn't do code reviews.

Is it just the one deps file you want me to look at?

[Fady Samuel, 2015-05-04 23:03:19]

On 2015/05/04 23:00:13, benwells wrote:
> On 2015/05/04 22:00:18, Fady Samuel wrote:
> > -miket@ +benwells, since Mike doesn't do code reviews.
> 
> Is it just the one deps file you want me to look at?

Yes please! :-) extensions/browser/DEPS

[benwells, 2015-05-05 00:47:39]

deps lgtm

[Fady Samuel, 2015-05-05 00:51:40]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2015-05-05 00:53:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1102173002/420001

[commit-bot: I haz the power, 2015-05-05 01:00:54]

Committed patchset #22 (id:420001)

[commit-bot: I haz the power, 2015-05-05 01:01:50]

Patchset 22 (id:??) landed as
https://crrev.com/8dfa19acbffebaeafd38e8c3f4ef136c3fc83734
Cr-Commit-Position: refs/heads/master@{#328243}