Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Issue 12519 Crash - views::DialogClientView::AcceptWindow()
Starred by 1 user Project Member Reported by lafo...@chromium.org, May 22 2009 Back to list
Status: Fixed
Owner:
Last visit 20 days ago
Closed: Jun 2009
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression
M-3

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
This crash was detected in 2.0.172.28 and appears to be a regression from 2.0.172.27.
It is currently ranked #18 (based on the relative number of reports in the release).  There have been 24 reports from 24 clients.
Search query: http://crash/search?query=Chrome+2.0.172.28+views%3A%3ADialogClientView%3A%3AAcceptWindow%28%29
----------------------------
*       Summary Data       *
----------------------------
Report Link: http://crash/reportdetail?reportid=311ad0300aba91b5
Mini Dump Link: http://crash/file?reportid=311ad0300aba91b5&name=upload_file_minidump

Uptime: 956 sec
User Comments: null
OS: Windows Vista or Windows Server 2008 Service Pack 1
CPU Architecture: x86
CPU Info: GenuineIntel family 15 model 4 stepping 10
rept: null
ptype: browser
plat: Win32

----------------------------
*      Loaded Modules      *
----------------------------
    chrome.dll
    chrome.exe
    kernel32.dll
    ntdll.dll
    user32.dll
    comctl32.dll

----------------------------
*        Crash Trace       *
----------------------------
[dialog_client_view.cc:209] - views::DialogClientView::AcceptWindow()
[dialog_client_view.cc:343] - views::DialogClientView::ButtonPressed(views::Button *)
             [button.cc:69] - views::Button::NotifyClick(int)
      [native_button.cc:89] - views::NativeButton::ButtonPressed()
  [native_button_win.cc:80] - views::NativeButtonWin::ProcessMessage(unsigned int,unsigned int,long,long *)
        [widget_win.cc:932] - views::ProcessNativeControlMessage(unsigned int,unsigned int,long,long *)
        [widget_win.cc:960] - views::WidgetWin::WndProc(HWND__ *,unsigned int,unsigned int,long)
    [user32.dll+0x0001f8d1] - InternalCallWinProc
    [user32.dll+0x0001f793] - UserCallWinProcCheckWow
    [user32.dll+0x000206f5] - CallWindowProcAorW
    [user32.dll+0x0002069b] - CallWindowProcW
      [focus_manager.cc:91] - views::FocusWindowCallback
    [user32.dll+0x0001f8d1] - InternalCallWinProc
    [user32.dll+0x0001f793] - UserCallWinProcCheckWow
    [user32.dll+0x00020816] - DispatchClientMessage
    [user32.dll+0x00020a64] - __fnDWORD
     [ntdll.dll+0x000599cd] - KiUserCallbackDispatcher
     [ntdll.dll+0x0005997f] - KiUserApcDispatcher
    [user32.dll+0x00020af9] - SendMessageW
  [comctl32.dll+0x0001f94e] - Button_NotifyParent(tagBUTN *,unsigned int)
  [comctl32.dll+0x0001f9b0] - Button_NotifyParent(tagBUTN *,unsigned int)
  [comctl32.dll+0x0001fc96] - AlterWindowStyle
    [user32.dll+0x0001f8d1] - InternalCallWinProc
    [user32.dll+0x0001f793] - UserCallWinProcCheckWow
    [user32.dll+0x000206f5] - CallWindowProcAorW
    [user32.dll+0x0002069b] - CallWindowProcW
[native_control_win.cc:198] - views::NativeControlWin::NativeControlWndProc(HWND__ *,unsigned int,unsigned int,long)
    [user32.dll+0x0001f8d1] - InternalCallWinProc
    [user32.dll+0x0001f793] - UserCallWinProcCheckWow
    [user32.dll+0x000206f5] - CallWindowProcAorW
    [user32.dll+0x0002069b] - CallWindowProcW
      [focus_manager.cc:91] - views::FocusWindowCallback
    [user32.dll+0x0001f8d1] - InternalCallWinProc
    [user32.dll+0x0001f793] - UserCallWinProcCheckWow
    [user32.dll+0x00020007] - DispatchMessageWorker
    [user32.dll+0x0002005f] - DispatchMessageW
[accelerator_handler.cc:29] - views::AcceleratorHandler::Dispatch(tagMSG const &)
  [message_pump_win.cc:356] - base::MessagePumpForUI::ProcessMessageHelper(tagMSG const &)
  [message_pump_win.cc:204] - base::MessagePumpForUI::DoRunLoop()
   [message_pump_win.cc:52] - base::MessagePumpWin::RunWithDispatcher(base::MessagePump::Delegate *,base::MessagePumpWin::Dispatcher *)
      [message_loop.cc:192] - MessageLoop::RunInternal()
      [message_loop.cc:180] - MessageLoop::RunHandler()
      [message_loop.cc:566] - MessageLoopForUI::Run(base::MessagePumpWin::Dispatcher *)
         [first_run.cc:391] - OpenFirstRunDialog(Profile *)
      [browser_main.cc:453] - BrowserMain(MainFunctionParams const &)
   [chrome_dll_main.cc:406] - ChromeMain
[google_update_client.cc:93] - google_update::GoogleUpdateClient::Launch(HINSTANCE__ *,sandbox::SandboxInterfaceInfo *,wchar_t *,char const *,int *)
    [chrome_exe_main.cc:67] - wWinMain

 
Labels: Mstone-3
Status: Assigned
Hey Finnur,

Could you take a look at this code please?
So... I can't get the source to line up with the crash dump for some reason. I 
usually can (wonder if the symbols for this file are messed up). However, looking at 
a somewhat recent copy of the file it looks like the crash occurs here...

void DialogClientView::AcceptWindow() {
  if (accepted_) {
    // We should only get into AcceptWindow once.
    NOTREACHED();
    return;
  }
  if (GetDialogDelegate()->Accept(false)) {
    accepted_ = true;
    Close();
  }
}

I suspect the failing code is GetDialogDelegate()->Accept()

I also see that r11132 removed this check: 

void DialogClientView::ButtonPressed(NativeButton* sender) {
  // We NULL check the delegate here since the buttons can receive WM_COMMAND
  // messages even after they (and the window containing us) are destroyed.
  if (!GetDialogDelegate())
    return;

... and below this check is a call to AcceptWindow.

The comment made with the checkin was: "Test to see if we can reduce some crashes by 
deferring delegate destruction until WM_NCDESTROY. This involves adding a specific 
method to allow delegates to destroy themselves to WindowDelegate, and moving all 
delete this calls into implementations of that method (to allow delegates to still 
respond to WM_DESTROY which is legit)."


My question to Ben is, should we re-add this check?

Status: Fixed
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=17651 

------------------------------------------------------------------------
r17651 | finnur@chromium.org | 2009-06-04 12:47:11 -0700 (Thu, 04 Jun 2009) | 22 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/views/window/dialog_client_view.cc?r1=17651&r2=17650

Fix crash 12519: in views::DialogClientView::AcceptWindow()

We don't have repro steps for this crash so this
is a blind fix.

But looking at the function that crashes and the
history of this file I see:

Ben's r11070 and r11047: Which add this check.
Ben's r11132: Which removes this check (according
to the comments it is a test to see if we can get
away with it).

It looks to me like the check would prevent the
crash and since this is still crashing, I am
proposing we re-add it.

BUG= 12519 
TEST=We'll need to verify this by checking the
crash dumps sent from the field.

Review URL: http://codereview.chromium.org/119183
------------------------------------------------------------------------

Project Member Comment 6 by bugdroid1@chromium.org, Feb 17 2011
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=75221

------------------------------------------------------------------------
r75221 | estade@chromium.org | Wed Feb 16 18:19:43 PST 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/resources/options/options_page.js?r1=75221&r2=75220&pathrev=75221
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/resources/options/options_page.css?r1=75221&r2=75220&pathrev=75221
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/resources/options/options.js?r1=75221&r2=75220&pathrev=75221

Use class instead of attribute for hide-menu

Works around webkit  bug 12519 

BUG= 73047 
TEST=manual

Review URL: http://codereview.chromium.org/6507019
------------------------------------------------------------------------
Labels: -Regression bulkmove Type-Regression
This crash was detected in 2.0.172.28 and appears to be a regression from 2.0.172.27.
It is currently ranked #18 (based on the relative number of reports in the release).  There have been 24 reports from 24 clients.
Search query: http://crash/search?query=Chrome+2.0.172.28+views%3A%3ADialogClientView%3A%3AAcceptWindow%28%29
----------------------------
*       Summary Data       *
----------------------------
Report Link: http://crash/reportdetail?reportid=311ad0300aba91b5
Mini Dump Link: http://crash/file?reportid=311ad0300aba91b5&name=upload_file_minidump

Uptime: 956 sec
User Comments: null
OS: Windows Vista or Windows Server 2008 Service Pack 1
CPU Architecture: x86
CPU Info: GenuineIntel family 15 model 4 stepping 10
rept: null
ptype: browser
plat: Win32

----------------------------
*      Loaded Modules      *
----------------------------
    chrome.dll
    chrome.exe
    kernel32.dll
    ntdll.dll
    user32.dll
    comctl32.dll

----------------------------
*        Crash Trace       *
----------------------------
[dialog_client_view.cc:209] - views::DialogClientView::AcceptWindow()
[dialog_client_view.cc:343] - views::DialogClientView::ButtonPressed(views::Button *)
             [button.cc:69] - views::Button::NotifyClick(int)
      [native_button.cc:89] - views::NativeButton::ButtonPressed()
  [native_button_win.cc:80] - views::NativeButtonWin::ProcessMessage(unsigned int,unsigned int,long,long *)
        [widget_win.cc:932] - views::ProcessNativeControlMessage(unsigned int,unsigned int,long,long *)
        [widget_win.cc:960] - views::WidgetWin::WndProc(HWND__ *,unsigned int,unsigned int,long)
    [user32.dll+0x0001f8d1] - InternalCallWinProc
    [user32.dll+0x0001f793] - UserCallWinProcCheckWow
    [user32.dll+0x000206f5] - CallWindowProcAorW
    [user32.dll+0x0002069b] - CallWindowProcW
      [focus_manager.cc:91] - views::FocusWindowCallback
    [user32.dll+0x0001f8d1] - InternalCallWinProc
    [user32.dll+0x0001f793] - UserCallWinProcCheckWow
    [user32.dll+0x00020816] - DispatchClientMessage
    [user32.dll+0x00020a64] - __fnDWORD
     [ntdll.dll+0x000599cd] - KiUserCallbackDispatcher
     [ntdll.dll+0x0005997f] - KiUserApcDispatcher
    [user32.dll+0x00020af9] - SendMessageW
  [comctl32.dll+0x0001f94e] - Button_NotifyParent(tagBUTN *,unsigned int)
  [comctl32.dll+0x0001f9b0] - Button_NotifyParent(tagBUTN *,unsigned int)
  [comctl32.dll+0x0001fc96] - AlterWindowStyle
    [user32.dll+0x0001f8d1] - InternalCallWinProc
    [user32.dll+0x0001f793] - UserCallWinProcCheckWow
    [user32.dll+0x000206f5] - CallWindowProcAorW
    [user32.dll+0x0002069b] - CallWindowProcW
[native_control_win.cc:198] - views::NativeControlWin::NativeControlWndProc(HWND__ *,unsigned int,unsigned int,long)
    [user32.dll+0x0001f8d1] - InternalCallWinProc
    [user32.dll+0x0001f793] - UserCallWinProcCheckWow
    [user32.dll+0x000206f5] - CallWindowProcAorW
    [user32.dll+0x0002069b] - CallWindowProcW
      [focus_manager.cc:91] - views::FocusWindowCallback
    [user32.dll+0x0001f8d1] - InternalCallWinProc
    [user32.dll+0x0001f793] - UserCallWinProcCheckWow
    [user32.dll+0x00020007] - DispatchMessageWorker
    [user32.dll+0x0002005f] - DispatchMessageW
[accelerator_handler.cc:29] - views::AcceleratorHandler::Dispatch(tagMSG const &)
  [message_pump_win.cc:356] - base::MessagePumpForUI::ProcessMessageHelper(tagMSG const &)
  [message_pump_win.cc:204] - base::MessagePumpForUI::DoRunLoop()
   [message_pump_win.cc:52] - base::MessagePumpWin::RunWithDispatcher(base::MessagePump::Delegate *,base::MessagePumpWin::Dispatcher *)
      [message_loop.cc:192] - MessageLoop::RunInternal()
      [message_loop.cc:180] - MessageLoop::RunHandler()
      [message_loop.cc:566] - MessageLoopForUI::Run(base::MessagePumpWin::Dispatcher *)
         [first_run.cc:391] - OpenFirstRunDialog(Profile *)
      [browser_main.cc:453] - BrowserMain(MainFunctionParams const &)
   [chrome_dll_main.cc:406] - ChromeMain
[google_update_client.cc:93] - google_update::GoogleUpdateClient::Launch(HINSTANCE__ *,sandbox::SandboxInterfaceInfo *,wchar_t *,char const *,int *)
    [chrome_exe_main.cc:67] - wWinMain
Project Member Comment 8 by bugdroid1@chromium.org, Oct 13 2012
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member Comment 9 by bugdroid1@chromium.org, Mar 10 2013
Labels: -Mstone-3 -Type-Regression Type-Bug-Regression M-3
Project Member Comment 10 by bugdroid1@chromium.org, Mar 13 2013
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Sign in to add a comment