Remove WebViewClient::showVirtualKeyboardOnElementFocus()

third_party/WebKit/Source/web/ChromeClientImpl.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
@@ skipping 1049 matching lines @@
   if (WebViewClient* client = m_webView->client())
     client->draggableRegionsChanged();
 }
 
 void ChromeClientImpl::didAssociateFormControlsAfterLoad(LocalFrame* frame) {
   WebLocalFrameImpl* webframe = WebLocalFrameImpl::fromFrame(frame);
   if (webframe->autofillClient())
     webframe->autofillClient()->didAssociateFormControlsDynamically();
 }
 
-void ChromeClientImpl::showVirtualKeyboardOnElementFocus() {
-  if (m_webView->client())
-    m_webView->client()->showVirtualKeyboardOnElementFocus();
+void ChromeClientImpl::showVirtualKeyboardOnElementFocus(LocalFrame* frame) {
+  if (auto* frameWidget = WebLocalFrameImpl::fromFrame(frame)->frameWidget())

[dcheng, 2017/02/22 07:26:44]

Is this only null in tests? I would prefer not to null check this if it's not
necessary for production code.

[EhsanK, 2017/02/22 17:10:52]

Good point. But unfortunately (and I don't quite know why yet) it happens in
content shell as well.
To repro:
1- I put a DCHECK(false) when frameWidget is nullptr.
2- Created an html page with <input> which in its onload handler creates and
attaches another <iframe> to its window.location.href.
3- In content shell, focused address bar with the URL to the page explaiend and
spammed Enter (to navigate) + tab.

Sample Stack Trace:
#0 0x7ff8d3f784a7 base::debug::StackTrace::StackTrace()
#1 0x7ff8d3f7801f base::debug::(anonymous namespace)::StackDumpSignalHandler()
#2 0x7ff8d5d5e330 <unknown>
#3 0x7ff8ccf23c37 gsignal
#4 0x7ff8ccf27028 abort
#5 0x7ff8d3f762b2 base::debug::BreakDebugger()
#6 0x7ff8d3f9cb0d logging::LogMessage::~LogMessage()
#7 0x7ff8d0d18704 blink::ChromeClientImpl::showVirtualKeyboardOnElementFocus()
#8 0x7ff8cf9b4157 blink::Element::focus()
#9 0x7ff8d0150b4e blink::FocusController::advanceFocusInDocumentOrder()
#10 0x7ff8d015060d blink::FocusController::advanceFocus()
#11 0x7ff8cfe42f0e blink::KeyboardEventManager::defaultTabEventHandler()
#12 0x7ff8cfe42d22 blink::KeyboardEventManager::defaultKeyboardEventHandler()
#13 0x7ff8cfa02112 blink::Node::defaultEventHandler()
#14 0x7ff8cfca8496 blink::HTMLElement::defaultEventHandler()
#15 0x7ff8cfb9ac53 blink::EventDispatcher::dispatchEventPostProcess()
#16 0x7ff8cfb9a10d blink::EventDispatcher::dispatch()
#17 0x7ff8cfb995f1 blink::EventDispatchMediator::dispatchEvent()
#18 0x7ff8cfb996e1 blink::EventDispatcher::dispatchEvent()
#19 0x7ff8cfe428fd blink::KeyboardEventManager::keyEvent()
#20 0x7ff8d0de02b3 blink::WebViewImpl::handleKeyEvent()
#21 0x7ff8d0d47454 blink::PageWidgetDelegate::handleInputEvent()
#22 0x7ff8d0ddee23 blink::WebViewImpl::handleInputEvent()
#23 0x7ff8d5095969 content::RenderWidgetInputHandler::HandleInputEvent()
#24 0x7ff8d5134554 content::RenderWidget::OnHandleInputEvent()

Also, I tried focusing the <input> in the window.onload handler which added the
following variety of stack trace to the mix:

Sample Stack Trace #2:
#0 0x7ff8d3f784a7 base::debug::StackTrace::StackTrace()
#1 0x7ff8d3f7801f base::debug::(anonymous namespace)::StackDumpSignalHandler()
#2 0x7ff8d5d5e330 <unknown>
#3 0x7ff8ccf23c37 gsignal
#4 0x7ff8ccf27028 abort
#5 0x7ff8d3f762b2 base::debug::BreakDebugger()
#6 0x7ff8d3f9cb0d logging::LogMessage::~LogMessage()
#7 0x7ff8d0d18704 blink::ChromeClientImpl::showVirtualKeyboardOnElementFocus()
#8 0x7ff8cf9b4157 blink::Element::focus()
#9 0x7ff8d045fff4 blink::V8HTMLElement::focusMethodCallback()

So I believe it is safe to check for frameWidget. That being said, we were (and
still are in other places) checking for |m_webView|. Is it possible that
m_webView does not exist but we still have a |frameWidget|?

[dcheng, 2017/02/23 05:42:29]

Hmm, I think you might need to get the local root first, right? Otherwise, I
think the null deref is expected.

[EhsanK, 2017/02/23 17:24:16]

Yes thanks! I was confusing frameWidget() with GetRenderWidget() in content/
which
automatically gets it from the local root. I believe everything should be green
now.

+    frameWidget->client()->showVirtualKeyboardOnElementFocus();
 }
 
 void ChromeClientImpl::showUnhandledTapUIIfNeeded(
     IntPoint tappedPositionInViewport,
     Node* tappedNode,
     bool pageChanged) {
   if (m_webView->client())
     m_webView->client()->showUnhandledTapUIIfNeeded(
         WebPoint(tappedPositionInViewport), WebNode(tappedNode), pageChanged);
 }
@@ skipping 136 matching lines @@
     PresentationController::provideTo(frame, client->presentationClient());
   if (RuntimeEnabledFeatures::audioOutputDevicesEnabled()) {
     provideAudioOutputDeviceClientTo(frame,
                                      new AudioOutputDeviceClientImpl(frame));
   }
   if (RuntimeEnabledFeatures::installedAppEnabled())
     InstalledAppController::provideTo(frame, client->relatedAppsFetcher());
 }
 
 }  // namespace blink