[chromium-dev] Re: ResourceMessageFilter::OnGet(Root)WindowRect and NULL windows
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
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
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
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
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
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
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
#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 -~--~~~~--~~--~--~---