Fix crash in Chromium memory test.

• Component: New Bugs
• Status: RESOLVED FIXED

[Shishir Agrawal]

Chromium stack trace:
InvalidRead
Invalid read of size 4
  WebKit::WebViewImpl::WebViewImpl(WebKit::WebViewClient*) (third_party/WebKit/Source/WebKit/chromium/src/WebViewImpl.cpp:371)
  WebKit::WebView::create(WebKit::WebViewClient*) (third_party/WebKit/Source/WebKit/chromium/src/WebViewImpl.cpp:225)
  WebKit::WebWorkerBase::initializeLoader(WebKit::WebURL const&) (third_party/WebKit/Source/WebKit/chromium/src/WebWorkerBase.cpp:240)
  WebKit::WebWorkerImpl::startWorkerContext(WebKit::WebURL const&, WebKit::WebString const&, WebKit::WebString const&) (third_party/WebKit/Source/WebKit/chromium/src/WebWorkerImpl.cpp:105)
  void DispatchToMethod<WebKit::WebWorker, void (WebKit::WebWorker::*)(WebKit::WebURL const&, WebKit::WebString const&, WebKit::WebString const&), GURL, std::basic_string<unsigned short, base::string16_char_traits, std::allocator<unsigned short> >, std::basic_string<unsigned short, base::string16_char_traits, std::allocator<unsigned short> > >(WebKit::WebWorker*, void (WebKit::WebWorker::*)(WebKit::WebURL const&, WebKit::WebString const&, WebKit::WebString const&), Tuple3<GURL, std::basic_string<unsigned short, base::string16_char_traits, std::allocator<unsigned short> >, std::basic_string<unsigned short, base::string16_char_traits, std::allocator<unsigned short> > > const&) (./base/tuple.h:564)
  bool IPC::MessageWithTuple<Tuple3<GURL, std::basic_string<unsigned short, base::string16_char_traits, std::allocator<unsigned short> >, std::basic_string<unsigned short, base::string16_char_traits, std::allocator<unsigned short> > > >::Dispatch<WebKit::WebWorker, WebWorkerStub, void (WebKit::WebWorker::*)(WebKit::WebURL const&, WebKit::WebString const&, WebKit::WebString const&)>(IPC::Message const*, WebKit::WebWorker*, WebWorkerStub*, void (WebKit::WebWorker::*)(WebKit::WebURL const&, WebKit::WebString const&, WebKit::WebString const&)) (./ipc/ipc_message_utils.h:963)
  WebWorkerStub::OnMessageReceived(IPC::Message const&) (content/worker/webworker_stub.cc:43)
  MessageRouter::RouteMessage(IPC::Message const&) (content/common/message_router.cc:46)
  MessageRouter::OnMessageReceived(IPC::Message const&) (content/common/message_router.cc:38)
  ChildThread::OnMessageReceived(IPC::Message const&) (content/common/child_thread.cc:175)
  IPC::ChannelProxy::Context::OnDispatchMessage(IPC::Message const&) (ipc/ipc_channel_proxy.cc:256)
  void DispatchToMethod<IPC::ChannelProxy::Context, void (IPC::ChannelProxy::Context::*)(IPC::Message const&), IPC::Message>(IPC::ChannelProxy::Context*, void (IPC::ChannelProxy::Context::*)(IPC::Message const&), Tuple1<IPC::Message> const&) (./base/tuple.h:551)
  RunnableMethod<IPC::ChannelProxy::Context, void (IPC::ChannelProxy::Context::*)(IPC::Message const&), Tuple1<IPC::Message> >::Run() (./base/task.h:338)
  (anonymous namespace)::TaskClosureAdapter::Run() (base/message_loop.cc:102)
  base::internal::Invoker1<false, base::internal::InvokerStorage1<void ((anonymous namespace)::TaskClosureAdapter::*)(), (anonymous namespace)::TaskClosureAdapter*>, void ((anonymous namespace)::TaskClosureAdapter::*)()>::DoInvoke(base::internal::InvokerStorageBase*) (./base/bind_internal.h:547)
  base::Callback<void ()()>::Run() const (./base/callback.h:261)
  MessageLoop::RunTask(MessageLoop::PendingTask const&) (base/message_loop.cc:482)
  MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) (base/message_loop.cc:500)
  MessageLoop::DoWork() (base/message_loop.cc:691)
  base::MessagePumpDefault::Run(base::MessagePump::Delegate*) (base/message_pump_default.cc:23)
  MessageLoop::RunInternal() (base/message_loop.cc:449)
  MessageLoop::RunHandler() (base/message_loop.cc:422)
  MessageLoop::Run() (base/message_loop.cc:346)
  WorkerMain(MainFunctionParams const&) (content/worker/worker_main.cc:52)
  (anonymous namespace)::RunZygote(MainFunctionParams const&) (chrome/app/chrome_main.cc:448)
  (anonymous namespace)::RunNamedProcessTypeMain(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, MainFunctionParams const&) (chrome/app/chrome_main.cc:494)
  ChromeMain (chrome/app/chrome_main.cc:815)
  main (chrome/app/chrome_exe_main_gtk.cc:46)

This is likely due to the client being null at the WebViewImpl constructor.

[Shishir Agrawal]

Created attachment 94814 [details]
Patch

[Shishir Agrawal]

Created attachment 94815 [details]
Patch

Minor style fix.

[Dimitri Glazkov (Google)]

Comment on attachment 94815 [details]
Patch

no changelog!

[Shishir Agrawal]

Created attachment 94835 [details]
Patch

Added changelog.

[Dimitri Glazkov (Google)]

Comment on attachment 94835 [details]
Patch

How can we write a regression test for this?

[Shishir Agrawal]

I am not sure what we want to test in the regression. Should we test that passing in a null client will not cause failures in the constructor of WebViewClient? Also there is already other code that does m_client checks before using it so this should get caught by existing tests?

[WebKit Commit Bot]

Comment on attachment 94835 [details]
Patch

Clearing flags on attachment: 94835

Committed r87342: <http://trac.webkit.org/changeset/87342>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.