Extension/Platform App window isolation

Extension/Platform App window isolation

Hide platform app windows from extensions, and hide all windows that don't
belong to the current app from apps.

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

[Mihai Parparita -not on Chrome, 2012-05-18 01:17:00]

Steven: Review the ExtensionWindowController, etc. changes, to check that
they're in the spirit of your design for them.
Aaron: General extensions API review
Jeremy: Just as a FYI. I'm assuming that your windowing API will still be built
on top of the ExtensionWindowController/BaseWindow/ExtensionWindowList
contructs.

[Aaron Boodman, 2012-05-21 19:19:57]

lgtm

[stevenjb, 2012-05-21 19:22:52]

I like the profile matching cleanup, but I would prefer to avoid a dependency on
UIThreadExtensionFunction in ExtensionWindowController.

https://chromiumcodereview.appspot.com/10407035/diff/8002/chrome/browser/exte...
File chrome/browser/extensions/extension_window_controller.cc (right):

https://chromiumcodereview.appspot.com/10407035/diff/8002/chrome/browser/exte...
chrome/browser/extensions/extension_window_controller.cc:30:
UIThreadExtensionFunction* function) const {
It seems like this might be better if this were in UIThreadExtensionFunction
instead of here.

https://chromiumcodereview.appspot.com/10407035/diff/8002/chrome/browser/exte...
chrome/browser/extensions/extension_window_controller.cc:33: if (extension !=
NULL && !IsVisibleToExtension(function->GetExtension()))
This would become window->IsVisibleToExtension(GetExtension())
(IsVisibleToExtension would need to be public)

https://chromiumcodereview.appspot.com/10407035/diff/8002/chrome/browser/exte...
chrome/browser/extensions/extension_window_controller.cc:36: if
(function->profile() == profile_)
profile() == window->profile()

https://chromiumcodereview.appspot.com/10407035/diff/8002/chrome/browser/exte...
chrome/browser/extensions/extension_window_controller.cc:43:
function->profile()->GetOffTheRecordProfile() == profile_;
The above has more to do with function than with window, so makes more sense in
UIThreadExtensionFunction.

[Mihai Parparita -not on Chrome, 2012-05-21 21:10:05]

https://chromiumcodereview.appspot.com/10407035/diff/8002/chrome/browser/exte...
File chrome/browser/extensions/extension_window_controller.cc (right):

https://chromiumcodereview.appspot.com/10407035/diff/8002/chrome/browser/exte...
chrome/browser/extensions/extension_window_controller.cc:30:
UIThreadExtensionFunction* function) const {
On 2012/05/21 19:22:52, stevenjb (chromium) wrote:
> It seems like this might be better if this were in UIThreadExtensionFunction
> instead of here.

Done.

[stevenjb, 2012-05-21 21:55:42]

lgtm with nits

https://chromiumcodereview.appspot.com/10407035/diff/9002/chrome/browser/exte...
File chrome/browser/extensions/extension_window_list.cc (right):

https://chromiumcodereview.appspot.com/10407035/diff/9002/chrome/browser/exte...
chrome/browser/extensions/extension_window_list.cc:41: const
UIThreadExtensionFunction* function,
nit: Maybe we should rename this something like FindWindowForFunction?

https://chromiumcodereview.appspot.com/10407035/diff/9002/chrome/browser/exte...
chrome/browser/extensions/extension_window_list.cc:52: const
UIThreadExtensionFunction* function) const {
nit: Maybe CurrentWindowForFunction?

[Mihai Parparita -not on Chrome, 2012-05-21 22:05:28]

https://chromiumcodereview.appspot.com/10407035/diff/9002/chrome/browser/exte...
File chrome/browser/extensions/extension_window_list.cc (right):

https://chromiumcodereview.appspot.com/10407035/diff/9002/chrome/browser/exte...
chrome/browser/extensions/extension_window_list.cc:41: const
UIThreadExtensionFunction* function,
On 2012/05/21 21:55:42, stevenjb (chromium) wrote:
> nit: Maybe we should rename this something like FindWindowForFunction?

Done.

https://chromiumcodereview.appspot.com/10407035/diff/9002/chrome/browser/exte...
chrome/browser/extensions/extension_window_list.cc:52: const
UIThreadExtensionFunction* function) const {
On 2012/05/21 21:55:42, stevenjb (chromium) wrote:
> nit: Maybe CurrentWindowForFunction?

Done.

[commit-bot: I haz the power, 2012-05-21 22:06:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mihaip@chromium.org/10407035/20001

[commit-bot: I haz the power, 2012-05-21 22:06:35]

Presubmit check for 10407035-20001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/browser/ui/views/ash

Presubmit checks took 1.9s to calculate.

[Mihai Parparita -not on Chrome, 2012-05-21 22:09:49]

+Scott for panel_view_aura.cc chnages

[sky, 2012-05-21 22:37:31]

LGTM

[commit-bot: I haz the power, 2012-05-21 22:40:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mihaip@chromium.org/10407035/20001

[commit-bot: I haz the power, 2012-05-22 04:32:50]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is win_rel, revision is 138212, job name
was 10407035-20001 (retry) (retry) (retry) (retry).

[commit-bot: I haz the power, 2012-05-22 04:34:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mihaip@chromium.org/10407035/20001

[commit-bot: I haz the power, 2012-05-22 07:02:40]

Try job failure for 10407035-20001 (retry) on win_rel for steps "base_unittests,
sync_unit_tests".
It's a second try, previously, steps "base_unittests, sync_unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-05-22 15:27:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mihaip@chromium.org/10407035/20001

[commit-bot: I haz the power, 2012-05-22 15:52:30]

Try job failure for 10407035-20001 (retry) on linux_chromeos for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2012-05-22 17:02:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mihaip@chromium.org/10407035/20001

[commit-bot: I haz the power, 2012-05-22 19:12:49]

Try job failure for 10407035-20001 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...