devtools: avoid relying on the internal variables for browser

devtools: avoid relying on the internal variables for browser

When the devtools are undocked, they live in their own browser. The
devtools could be moved to a different browser through the chrome.tabs
extension. This makes variables of the DevtoolsWindows out of sync
with the actual browser containing the devtools.

This CL removes the internal Browser* pointer and introduce methods to
retrieve the information from the browser list.

BUG=394341, 427226

[llandwerlin-old, 2015-07-14 09:09:50]

lionel.g.landwerlin@intel.com changed reviewers:
+ dgozman@chromium.org, kalman@chromium.org

[llandwerlin-old, 2015-07-14 09:09:50]

dgozman@: Could review this change to the devtools? Thanks.

Adding kalman@ who is reviewer on the other CL
(https://codereview.chromium.org/1099553002) needing this change.

[dgozman, 2015-07-14 12:47:16]

Why do you do this? If I remember correctly, in issue 427226 I specifically
disabled extensions access to devtools. I don't think we should expose it yet
even complicate devtools window code.

[llandwerlin-old, 2015-07-14 13:11:51]

On 2015/07/14 12:47:16, dgozman wrote:
> Why do you do this? If I remember correctly, in issue 427226 I specifically
> disabled extensions access to devtools. I don't think we should expose it yet
> even complicate devtools window code.

In 394341 we're trying to list a broader set of windows so extensions using the
chrome.windows API are able to move all the windows within a given profile. That
includes application windows as well as devtools windows. This is mainly to let
an extension layout windows on a ChromeOS.

It seems the issue in 427226 is that DevtoolWindows couldn't know that its
WebContents was being reattached to a different browser. This CL is about
removing that assumption, by always figuring out what browser the devtools are
attached to, when the information is needed.

[dgozman, 2015-07-14 13:19:21]

On 2015/07/14 13:11:51, llandwerlin wrote:
> On 2015/07/14 12:47:16, dgozman wrote:
> > Why do you do this? If I remember correctly, in issue 427226 I specifically
> > disabled extensions access to devtools. I don't think we should expose it
yet
> > even complicate devtools window code.
> 
> In 394341 we're trying to list a broader set of windows so extensions using
the
> chrome.windows API are able to move all the windows within a given profile.
That
> includes application windows as well as devtools windows. This is mainly to
let
> an extension layout windows on a ChromeOS.
> 
> It seems the issue in 427226 is that DevtoolWindows couldn't know that its
> WebContents was being reattached to a different browser. This CL is about
> removing that assumption, by always figuring out what browser the devtools are
> attached to, when the information is needed.

We can't afford to place DevTools in a regular browser. Browser class even has
is_devtools() method which is checked around.
I'd rather we ban DevTools from extension APIs. I can't imagine a lot of users
which both have extension managing windows _and_ DevTools open.

[not at google - send to devlin, 2015-07-14 17:18:06]

On 2015/07/14 13:19:21, dgozman wrote:
> On 2015/07/14 13:11:51, llandwerlin wrote:
> > On 2015/07/14 12:47:16, dgozman wrote:
> > > Why do you do this? If I remember correctly, in issue 427226 I
specifically
> > > disabled extensions access to devtools. I don't think we should expose it
> yet
> > > even complicate devtools window code.
> > 
> > In 394341 we're trying to list a broader set of windows so extensions using
> the
> > chrome.windows API are able to move all the windows within a given profile.
> That
> > includes application windows as well as devtools windows. This is mainly to
> let
> > an extension layout windows on a ChromeOS.
> > 
> > It seems the issue in 427226 is that DevtoolWindows couldn't know that its
> > WebContents was being reattached to a different browser. This CL is about
> > removing that assumption, by always figuring out what browser the devtools
are
> > attached to, when the information is needed.
> 
> We can't afford to place DevTools in a regular browser. Browser class even has
> is_devtools() method which is checked around.
> I'd rather we ban DevTools from extension APIs. I can't imagine a lot of users
> which both have extension managing windows _and_ DevTools open.

I don't mind exposing devtools windows to extensions, so long as it works. I
don't see a reason not to.
However we should set an appropriate WindowType:
https://developer.chrome.com/extensions/tabs#type-WindowType

[llandwerlin-old, 2015-07-14 17:33:04]

On 2015/07/14 17:18:06, kalman wrote:
> On 2015/07/14 13:19:21, dgozman wrote:
> > On 2015/07/14 13:11:51, llandwerlin wrote:
> > > On 2015/07/14 12:47:16, dgozman wrote:
> > > > Why do you do this? If I remember correctly, in issue 427226 I
> specifically
> > > > disabled extensions access to devtools. I don't think we should expose
it
> > yet
> > > > even complicate devtools window code.
> > > 
> > > In 394341 we're trying to list a broader set of windows so extensions
using
> > the
> > > chrome.windows API are able to move all the windows within a given
profile.
> > That
> > > includes application windows as well as devtools windows. This is mainly
to
> > let
> > > an extension layout windows on a ChromeOS.
> > > 
> > > It seems the issue in 427226 is that DevtoolWindows couldn't know that its
> > > WebContents was being reattached to a different browser. This CL is about
> > > removing that assumption, by always figuring out what browser the devtools
> are
> > > attached to, when the information is needed.
> > 
> > We can't afford to place DevTools in a regular browser. Browser class even
has
> > is_devtools() method which is checked around.
> > I'd rather we ban DevTools from extension APIs. I can't imagine a lot of
users
> > which both have extension managing windows _and_ DevTools open.
> 
> I don't mind exposing devtools windows to extensions, so long as it works. I
> don't see a reason not to.
> However we should set an appropriate WindowType:
> https://developer.chrome.com/extensions/tabs#type-WindowType

They're currently reported as "popup".
Browser::is_app() will return true for a devtools window, should we set their
type to "app" or create a new "devtools" type?

[not at google - send to devlin, 2015-07-14 17:35:46]

On 2015/07/14 17:33:04, llandwerlin wrote:
> On 2015/07/14 17:18:06, kalman wrote:
> > On 2015/07/14 13:19:21, dgozman wrote:
> > > On 2015/07/14 13:11:51, llandwerlin wrote:
> > > > On 2015/07/14 12:47:16, dgozman wrote:
> > > > > Why do you do this? If I remember correctly, in issue 427226 I
> > specifically
> > > > > disabled extensions access to devtools. I don't think we should expose
> it
> > > yet
> > > > > even complicate devtools window code.
> > > > 
> > > > In 394341 we're trying to list a broader set of windows so extensions
> using
> > > the
> > > > chrome.windows API are able to move all the windows within a given
> profile.
> > > That
> > > > includes application windows as well as devtools windows. This is mainly
> to
> > > let
> > > > an extension layout windows on a ChromeOS.
> > > > 
> > > > It seems the issue in 427226 is that DevtoolWindows couldn't know that
its
> > > > WebContents was being reattached to a different browser. This CL is
about
> > > > removing that assumption, by always figuring out what browser the
devtools
> > are
> > > > attached to, when the information is needed.
> > > 
> > > We can't afford to place DevTools in a regular browser. Browser class even
> has
> > > is_devtools() method which is checked around.
> > > I'd rather we ban DevTools from extension APIs. I can't imagine a lot of
> users
> > > which both have extension managing windows _and_ DevTools open.
> > 
> > I don't mind exposing devtools windows to extensions, so long as it works. I
> > don't see a reason not to.
> > However we should set an appropriate WindowType:
> > https://developer.chrome.com/extensions/tabs#type-WindowType
> 
> They're currently reported as "popup".
> Browser::is_app() will return true for a devtools window, should we set their
> type to "app" or create a new "devtools" type?

I think a separate devtools makes most sense. It's definitely not an app window
(I presume/hope these are reserved for windows created via the chrome.app.window
API). A popup also doesn't sound right.

[llandwerlin-old, 2015-07-16 10:16:20]

lionel.g.landwerlin@intel.com changed reviewers:
- dgozman@chromium.org, kalman@chromium.org