Move compositor InputHandler from RenderViewImpl to RenderWidget.

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 901 matching lines @@
   String message = String("Blocked ") + dialogTypeToString(dialogType) + "('" +
                    dialogMessage + "') during " +
                    dismissalTypeToString(dismissalType) + ".";
   m_webView->mainFrame()->addMessageToConsole(
       WebConsoleMessage(WebConsoleMessage::LevelError, message));
 
   return false;
 }
 
 void ChromeClientImpl::setEventListenerProperties(
+    LocalFrame* frame,
     WebEventListenerClass eventClass,
     WebEventListenerProperties properties) {
-  if (WebLayerTreeView* treeView = m_webView->layerTreeView()) {
+  WebLocalFrameImpl* webFrame = WebLocalFrameImpl::fromFrame(frame);
+  if (!webFrame || webFrame->localRoot() == m_webView->mainFrame()) {
+    setEventListenerProperties(m_webView->layerTreeView(), eventClass,
+                               properties);
+  } else {
+    WebFrameWidgetBase* widget = webFrame->localRoot()->frameWidget();
+    // The widget may be nullptr in some unit tests.

[dcheng, 2016/11/10 07:46:11]

How many tests does this break? If possible, we should fix the unit tests (a
followup is OK for this)

[wjmaclean, 2016/11/10 15:57:15]

I realize I missed a case, this isn't just broken in tests. The problem also
occurs when swapping out the main frame. We get the following call stack:

#1 0x7f860487d3f2 blink::ChromeClientImpl::setEventListenerProperties()
#2 0x7f8602266b5f blink::FrameLoader::commitProvisionalLoad()
#3 0x7f860224cd1d blink::DocumentLoader::finishedLoading()
#4 0x7f860224ecf0 blink::DocumentLoader::maybeLoadEmpty()
#5 0x7f860224ede6 blink::DocumentLoader::startLoadingMainResource()
#6 0x7f8602261c0d blink::FrameLoader::init()
#7 0x7f860490d27a blink::WebLocalFrameImpl::createProvisional()
#8 0x7f860490cf86 blink::WebLocalFrame::createProvisional()

Frame #1 is where we encounter the null widget. Frame #2 only calls
setEvenListenerProperties() in order to initialize the value for all three
handler classes to WebEventListenerProperties::Nothing, which in fact is the
*implicit* initialization when the LayerTree for the as-yet-to-be-created
widget, when it is finally created.

It would seem our safe options are:
1) Make the initialization *explicit* so we can rely on it, or
2) Don't allow WebLocalFrameImpl::createProvisional() to call
FrameLoader::init() without first creating a valid widget.

The first option seems easier, though perhaps is better done as a follow on.
This whole issue stems from the fact that event properties seem to have always
been set on the WebViewImpl's LayerTreeView, regardless of which RenderWidget's
LayerTreeView is really being used to draw the page content.

[dcheng, 2016/11/11 08:51:52]

Can you clarify what you mean?
We can't do that =(
I think I'd be OK with just special-casing this to skip this block for
provisional frames.
Also, now that we're tracking this on a per-local root basis, doesn't the
FrameLoader logic need to be updated?

Finally, it's actually not clear to me what the purpose of the call in
FrameLoader is: I would kind of expect the EventHandlerRegistry to end up in the
Nothing state just by virtue of detaching the previously attached documents.
Does that not happen?

[wjmaclean, 2016/11/11 15:40:51]

The event listener properties are stored as an array of EventListenerProperties,
which is itself an enum with a first value (0) of 'None'

https://cs.chromium.org/chromium/src/cc/trees/layer_tree.h?rcl=1478852525&l=275

This is initialized to zero, which means 'None' in terms of the enum. See
https://cs.chromium.org/chromium/src/cc/trees/layer_tree.cc?rcl=1478852525&l=58

But it would be nice if the initialization was a little clearer. Or maybe it
doesn't matter?

I guess in the provisional case, if we assume we are always going to create a
widget, and thus initialize a new set of event listener properties, then we can
just skip the block you mention below.
Sure, I'm not surprised.
Ok. But do you mean just delete this block entirely, or make it conditional on
something (and if so, what?).
I don't understand? We're just concerned about setting the event listener
properties, which is what we are attempting to update here.
I don't know anything about 'detaching' at this point in time. But yes, that
seems like it would be a reasonable thing to do on detach.

[dcheng, 2016/11/11 22:32:11]

I don't think we can expose that to the layer tree, unfortunately.
Right, but don't we need to track this on a per-local root basis now? The
current block is conditional on if it's a main frame.
Is it possible to verify if that already happens? Or is it possible to attach an
event listener somehow?

+    if (!widget) {
+      LOG(INFO) << "Null frameWidget.";

[dcheng, 2016/11/10 07:46:11]

Nit: remove this LOG

[wjmaclean, 2016/11/10 15:57:15]

Done.

+      return;
+    }
+
+    WebWidgetClient* client = widget->client();
+    // TODO(wjmaclean): Do we need to inform the WebView about these also?
+    setEventListenerProperties(client->layerTreeView(), eventClass, properties);
+  }
+}
+
+void ChromeClientImpl::setEventListenerProperties(
+    WebLayerTreeView* treeView,
+    WebEventListenerClass eventClass,
+    WebEventListenerProperties properties) {
+  if (treeView) {
     treeView->setEventListenerProperties(eventClass, properties);
     if (eventClass == WebEventListenerClass::TouchStartOrMove) {
       m_webView->hasTouchEventHandlers(
           properties != WebEventListenerProperties::Nothing ||
-          eventListenerProperties(WebEventListenerClass::TouchEndOrCancel) !=
+          eventListenerProperties(treeView,
+                                  WebEventListenerClass::TouchEndOrCancel) !=
               WebEventListenerProperties::Nothing);
     } else if (eventClass == WebEventListenerClass::TouchEndOrCancel) {
       m_webView->hasTouchEventHandlers(
           properties != WebEventListenerProperties::Nothing ||
-          eventListenerProperties(WebEventListenerClass::TouchStartOrMove) !=
+          eventListenerProperties(treeView,
+                                  WebEventListenerClass::TouchStartOrMove) !=
               WebEventListenerProperties::Nothing);
     }
   } else {
     m_webView->hasTouchEventHandlers(true);
   }
 }
 
 void ChromeClientImpl::beginLifecycleUpdates() {
   if (WebLayerTreeView* treeView = m_webView->layerTreeView()) {
     treeView->setDeferCommits(false);
     treeView->setNeedsBeginFrame();
   }
 }
 
 WebEventListenerProperties ChromeClientImpl::eventListenerProperties(
+    LocalFrame* frame,
     WebEventListenerClass eventClass) const {
-  if (WebLayerTreeView* treeView = m_webView->layerTreeView())
+  WebLocalFrameImpl* webFrame = WebLocalFrameImpl::fromFrame(frame);
+  if (!webFrame || webFrame->localRoot() == m_webView->mainFrame())
+    return eventListenerProperties(m_webView->layerTreeView(), eventClass);
+
+  WebWidgetClient* client =
+      WebLocalFrameImpl::fromFrame(frame)->localRoot()->frameWidget()->client();
+  return eventListenerProperties(client->layerTreeView(), eventClass);
+}
+
+WebEventListenerProperties ChromeClientImpl::eventListenerProperties(
+    WebLayerTreeView* treeView,
+    WebEventListenerClass eventClass) const {
+  if (treeView)
     return treeView->eventListenerProperties(eventClass);
   return WebEventListenerProperties::Nothing;
 }
 
 void ChromeClientImpl::setHasScrollEventHandlers(bool hasEventHandlers) {
   if (WebLayerTreeView* treeView = m_webView->layerTreeView())
     treeView->setHaveScrollEventHandlers(hasEventHandlers);
 }
 
 bool ChromeClientImpl::hasScrollEventHandlers() const {
@@ skipping 197 matching lines @@
   if (RuntimeEnabledFeatures::presentationEnabled())
     PresentationController::provideTo(frame, client->presentationClient());
   if (RuntimeEnabledFeatures::audioOutputDevicesEnabled())
     provideAudioOutputDeviceClientTo(frame,
                                      AudioOutputDeviceClientImpl::create());
   if (RuntimeEnabledFeatures::installedAppEnabled())
     InstalledAppController::provideTo(frame, client->installedAppClient());
 }
 
 }  // namespace blink