[chromium-dev] Re: ResourceMessageFilter::OnGet(Root)WindowRect and NULL windows

2009-03-23 Thread Darin Fisher
Thanks for the info!  Could you dump this into a bug report?

Thanks,
-Darin


On Mon, Mar 23, 2009 at 9:01 AM, Craig Schlenter
craig.schlen...@gmail.comwrote:

 On Sun, Mar 22, 2009 at 1:01 AM, Darin Fisher da...@chromium.org wrote:
  It would be nice to track down the source of the null NativeViewId.  I
 bet
  that corresponds to a real bug.

 Here's a backtrace from the renderer with host_window_ == NULL
 obtained from clicking on a link in an email in gmail:

 (gdb) bt
 #0  RenderWidget::GetRootWindowRect (this=0x9330530,
 webwidget=0x932ffc0, rect=0xb77b4508) at renderer/render_widget.cc:602
 #1  0x011808f1 in ChromeClientImpl::windowRect (this=0x90b76b8) at
 /home/craig/chromium.git/src/webkit/glue/chrome_client_impl.cc:100
 #2  0x07692c3c in WebCore::Chrome::windowRect (this=0x9338000) at
 /home/craig/chromium.git/src/third_party/WebKit/WebCore/page/Chrome.cpp:119
 #3  0x07a435e7 in WebCore::FrameLoader::createWindow (this=0x8b3715c,
 frameLoaderForFrameLookup=0x8a6caac, reque...@0xb77b47b4,
 featur...@0xb77b4b54,
creat...@0xb77b4abb) at

 /home/craig/chromium.git/src/third_party/WebKit/WebCore/loader/FrameLoader.cpp:380
 #4  0x0444ec13 in createWindow (openerFrame=0x8a6ca80,
 u...@0xb77b4bcc, framena...@0xb77b4bc8, windowfeatur...@0xb77b4b54,
 dialogArgs=
{v8::Handlev8::Value = {val_ = 0x0}, No data fields})
at
 /home/craig/chromium.git/src/third_party/WebKit/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:270
 #5  0x0444fee0 in WebCore::V8Custom::v8DOMWindowOpenCallback
 (ar...@0xb77b4ce4)
at
 /home/craig/chromium.git/src/third_party/WebKit/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:492
 #6  0x0806e20d in Builtin_HandleApiCall (__argc__=4,
 __argv__=0xb77b4da0) at
 /home/craig/chromium.git/src/v8/src/builtins.cc:380

 After poking at some code and inserting the odd printf, it seems as if
 RenderView::CreateWebView is calling RenderView::Create and passing a
 _hardcoded_ NULL as the parent_hwnd which is then passed to
 RenderView::Init where host_window_ is set to parent_hwnd which is
 NULL. That NULL would seem to me to be wrong. If it's not wrong, the
 other  initialisation path for host_window_ is
 RenderWidget::CompleteInit which is called from
 RenderWidget::OnCreatingNewAck. The CreatingNewAck message should be
 sent by RenderWidgetHost::Init() but I never see that being called
 when creating the error.

 Hopefully that's enough detail for someone who actually understands
 what these things do to propose a fix :)

 It looks trivial to squash sending the NULL to the browser to in
 RenderWidget::GetRootWindowRect but that is probably not the right
 fix.

 Thank you,

 --Craig


--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---



[chromium-dev] Re: ResourceMessageFilter::OnGet(Root)WindowRect and NULL windows

2009-03-23 Thread Craig Schlenter

On Mon, Mar 23, 2009 at 6:45 PM, Darin Fisher da...@chromium.org wrote:
 Thanks for the info!  Could you dump this into a bug report?

Added to http://crbug.com/9060 ...

--Craig

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---



[chromium-dev] Re: ResourceMessageFilter::OnGet(Root)WindowRect and NULL windows

2009-03-21 Thread Darin Fisher
It would be nice to track down the source of the null NativeViewId.  I bet
that corresponds to a real bug.
-Darin



On Fri, Mar 20, 2009 at 9:36 AM, Adam Langley a...@chromium.org wrote:


 On Fri, Mar 20, 2009 at 8:11 AM, Avi Drissman a...@google.com wrote:
  http://codereview.chromium.org/47002
  http://crbug.com/9060

 There's several parts here:
  * Why is the renderer asking questions about a NULL NativeViewId?
  * Why does that result in a NULL pointer in the browser?

 I don't know the answer to the first. It's probably some assumption in
 WebKit which assumes that it has a valid window at all times and we
 break that with multiprocess. However, it seems to be harmless.

 As for the second: we currently take pointers, raw from the renderer.
 This is just a hack. At some point, we need to make NativeViewIds be
 something else and have a proper translation layer. But we don't yet.



 AGL

 


--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---



[chromium-dev] Re: ResourceMessageFilter::OnGet(Root)WindowRect and NULL windows

2009-03-20 Thread Avi Drissman
I don't disagree with the statement you shouldn't let the renderer crash
the browser but I'm trying to understand if a null window ref is an
expected value for those functions to handle.

If so, the Windows version should explicitly check rather than relying on
the fact that the Win32 APIs happen to handle that case by accident.

If not, we need to figure out why those null window refs are coming through.

Is there anyone who knows what's going on here and can answer this question?

Avi
/who just wrote this into code in http://codereview.chromium.org/47002

On Thu, Mar 19, 2009 at 1:46 PM, Craig Schlenter
craig.schlen...@gmail.comwrote:

 Hi Avi

 When I did the original change, that function wasn't being called with
 a null window.
 Clicking on a link in gmail opened the link in a new window as I
 recall. At some later
 point that changed possibly when some of the tab_contents stuff was hooked
 up. I
 think it's good practice to check for null since you don't want the
 renderer to be able
 to crash the browser but I do tend to think that it shouldn't be
 happening to begin with
 but I'm largely clueless about the code involved :(

 Perhaps someone with a windows build can put a breakpoint in
 OnGetRootWindowRect please and see if clicking on a link in an
 email in gmail passes a HWND of null at all?

 Thank you,

 --Craig

 On Thu, Mar 19, 2009 at 7:33 PM, Avi Drissman a...@google.com wrote:
  We've been seeing calls to ResourceMessageFilter::OnGet(Root)WindowRect
 for
  NULL windows. agl put in a fix for GTK with
  http://codereview.chromium.org/42356 and I'm seeing the same problem on
 the
  Mac.
 
  1. Why isn't Windows seeing this? What happens when you pass a null HWND
  into ::GetAncestor and ::GetWindowRect?
  2. Is this expected, or is this indicative of a bug?
 
  Avi
 


--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---



[chromium-dev] Re: ResourceMessageFilter::OnGet(Root)WindowRect and NULL windows

2009-03-20 Thread Avi Drissman
http://codereview.chromium.org/47002
http://crbug.com/9060

Avi

On Fri, Mar 20, 2009 at 10:58 AM, Avi Drissman a...@google.com wrote:

 I don't disagree with the statement you shouldn't let the renderer crash
 the browser but I'm trying to understand if a null window ref is an
 expected value for those functions to handle.

 If so, the Windows version should explicitly check rather than relying on
 the fact that the Win32 APIs happen to handle that case by accident.

 If not, we need to figure out why those null window refs are coming
 through.

 Is there anyone who knows what's going on here and can answer this
 question?

 Avi
 /who just wrote this into code in http://codereview.chromium.org/47002

 On Thu, Mar 19, 2009 at 1:46 PM, Craig Schlenter 
 craig.schlen...@gmail.com wrote:

 Hi Avi

 When I did the original change, that function wasn't being called with
 a null window.
 Clicking on a link in gmail opened the link in a new window as I
 recall. At some later
 point that changed possibly when some of the tab_contents stuff was hooked
 up. I
 think it's good practice to check for null since you don't want the
 renderer to be able
 to crash the browser but I do tend to think that it shouldn't be
 happening to begin with
 but I'm largely clueless about the code involved :(

 Perhaps someone with a windows build can put a breakpoint in
 OnGetRootWindowRect please and see if clicking on a link in an
 email in gmail passes a HWND of null at all?

 Thank you,

 --Craig

 On Thu, Mar 19, 2009 at 7:33 PM, Avi Drissman a...@google.com wrote:
  We've been seeing calls to ResourceMessageFilter::OnGet(Root)WindowRect
 for
  NULL windows. agl put in a fix for GTK with
  http://codereview.chromium.org/42356 and I'm seeing the same problem on
 the
  Mac.
 
  1. Why isn't Windows seeing this? What happens when you pass a null HWND
  into ::GetAncestor and ::GetWindowRect?
  2. Is this expected, or is this indicative of a bug?
 
  Avi
 




--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---



[chromium-dev] Re: ResourceMessageFilter::OnGet(Root)WindowRect and NULL windows

2009-03-20 Thread Adam Langley

On Fri, Mar 20, 2009 at 8:11 AM, Avi Drissman a...@google.com wrote:
 http://codereview.chromium.org/47002
 http://crbug.com/9060

There's several parts here:
  * Why is the renderer asking questions about a NULL NativeViewId?
  * Why does that result in a NULL pointer in the browser?

I don't know the answer to the first. It's probably some assumption in
WebKit which assumes that it has a valid window at all times and we
break that with multiprocess. However, it seems to be harmless.

As for the second: we currently take pointers, raw from the renderer.
This is just a hack. At some point, we need to make NativeViewIds be
something else and have a proper translation layer. But we don't yet.



AGL

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---



[chromium-dev] Re: ResourceMessageFilter::OnGet(Root)WindowRect and NULL windows

2009-03-19 Thread Craig Schlenter

Hi

OK, that's interesting, thank you.  Does GetAncestor return something
valid when passed that null HWND or does it pass null through to
GetwindowRect? It would also be interesting to know what rect it
returns please. Maybe it doesn't matter ... I haven't looked at the
renderer code that creates this request to see if it actually cares
about this when the window is null.

Thank you,

--Craig

On Thu, Mar 19, 2009 at 7:56 PM, Mohamed Mansour
m0.interact...@gmail.com wrote:
 Craig, it returns back a 0.
 +               window  0x {unused=??? }        HWND__ *




 On Thu, Mar 19, 2009 at 1:46 PM, Craig Schlenter
 craig.schlen...@gmail.com wrote:

 Hi Avi

 When I did the original change, that function wasn't being called with
 a null window.
 Clicking on a link in gmail opened the link in a new window as I
 recall. At some later
 point that changed possibly when some of the tab_contents stuff was hooked 
 up. I
 think it's good practice to check for null since you don't want the
 renderer to be able
 to crash the browser but I do tend to think that it shouldn't be
 happening to begin with
 but I'm largely clueless about the code involved :(

 Perhaps someone with a windows build can put a breakpoint in
 OnGetRootWindowRect please and see if clicking on a link in an
 email in gmail passes a HWND of null at all?

 Thank you,

 --Craig

 On Thu, Mar 19, 2009 at 7:33 PM, Avi Drissman a...@google.com wrote:
 We've been seeing calls to ResourceMessageFilter::OnGet(Root)WindowRect for
 NULL windows. agl put in a fix for GTK with
 http://codereview.chromium.org/42356 and I'm seeing the same problem on the
 Mac.

 1. Why isn't Windows seeing this? What happens when you pass a null HWND
 into ::GetAncestor and ::GetWindowRect?
 2. Is this expected, or is this indicative of a bug?

 Avi


 



--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---



[chromium-dev] Re: ResourceMessageFilter::OnGet(Root)WindowRect and NULL windows

2009-03-19 Thread Lei Zhang

#include stdio.h
#include windows.h

int main()
{
  HWND hwnd = GetAncestor(NULL, GA_PARENT);
  printf(%p\n, hwnd);
  hwnd = GetAncestor(NULL, GA_ROOT);
  printf(%p\n, hwnd);
  hwnd = GetAncestor(NULL, GA_ROOTOWNER);
  printf(%p\n, hwnd);
  hwnd = GetAncestor(NULL, 0);
  printf(%p\n, hwnd);
  return 0;
}

printed four 0's for me on Vista.

On Thu, Mar 19, 2009 at 11:27 AM, Craig Schlenter
craig.schlen...@gmail.com wrote:

 Hi

 OK, that's interesting, thank you.  Does GetAncestor return something
 valid when passed that null HWND or does it pass null through to
 GetwindowRect? It would also be interesting to know what rect it
 returns please. Maybe it doesn't matter ... I haven't looked at the
 renderer code that creates this request to see if it actually cares
 about this when the window is null.

 Thank you,

 --Craig

 On Thu, Mar 19, 2009 at 7:56 PM, Mohamed Mansour
 m0.interact...@gmail.com wrote:
 Craig, it returns back a 0.
 +               window  0x {unused=??? }        HWND__ *




 On Thu, Mar 19, 2009 at 1:46 PM, Craig Schlenter
 craig.schlen...@gmail.com wrote:

 Hi Avi

 When I did the original change, that function wasn't being called with
 a null window.
 Clicking on a link in gmail opened the link in a new window as I
 recall. At some later
 point that changed possibly when some of the tab_contents stuff was hooked 
 up. I
 think it's good practice to check for null since you don't want the
 renderer to be able
 to crash the browser but I do tend to think that it shouldn't be
 happening to begin with
 but I'm largely clueless about the code involved :(

 Perhaps someone with a windows build can put a breakpoint in
 OnGetRootWindowRect please and see if clicking on a link in an
 email in gmail passes a HWND of null at all?

 Thank you,

 --Craig

 On Thu, Mar 19, 2009 at 7:33 PM, Avi Drissman a...@google.com wrote:
 We've been seeing calls to ResourceMessageFilter::OnGet(Root)WindowRect for
 NULL windows. agl put in a fix for GTK with
 http://codereview.chromium.org/42356 and I'm seeing the same problem on the
 Mac.

 1. Why isn't Windows seeing this? What happens when you pass a null HWND
 into ::GetAncestor and ::GetWindowRect?
 2. Is this expected, or is this indicative of a bug?

 Avi


 



 


--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---