Fix touch accessibility events in WebViews and iframes

third_party/WebKit/Source/web/WebViewImpl.cpp

 /*
  * Copyright (C) 2011, 2012 Google Inc. All rights reserved.
  *
  * 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.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 2138 matching lines @@
         if (overlay->handleInputEvent(inputEvent))
             return WebInputEventResult::HandledSuppressed;
     }
 
     if (inputEvent.modifiers & WebInputEvent::IsTouchAccessibility
         && WebInputEvent::isMouseEventType(inputEvent.type)) {
         PlatformMouseEventBuilder pme(mainFrameImpl()->frameView(), static_cast<const WebMouseEvent&>(inputEvent));
 
         // Find the right target frame. See issue 1186900.
         HitTestResult result = hitTestResultForRootFramePos(pme.position());
-        Frame* targetFrame;
-        if (result.innerNodeOrImageMapImage())
-            targetFrame = result.innerNodeOrImageMapImage()->document().frame();
-        else
-            targetFrame = m_page->focusController().focusedOrMainFrame();
-
-        if (targetFrame->isLocalFrame()) {
-            LocalFrame* targetLocalFrame = toLocalFrame(targetFrame);
-            Document* document = targetLocalFrame->document();
+        if (result.innerNodeFrame()) {

[dcheng, 2016/05/25 17:44:38]

I like this because it looks simpler, but I don't think I understand why this
new version is more correct. Do you mind enlightening me?

[dmazzoni, 2016/05/25 17:53:51]

I don't think result.innerNodeOrImageMapImage()->document().frame() was
incorrect - it seemed to return the same result for the cases I tried.
result.innerNodeFrame() does seem cleaner.

The critical bug was below, passing pme.position() rather than
result.roundedPointInInnerNodeFrame().

[dcheng, 2016/05/25 18:03:00]

Hmm. It looks like the original code is from the initial upstreaming of
WebViewImpl... +pdr who touched some hittesting stuff recently.

Intuitively this makes sense: I don't think you should be able to hittest a
detached document (which is what a null frame would imply), but I'm not sure if
there's an edge case I'm missing here.

[pdr., 2016/05/26 21:32:48]

This change seems reasonable to me. LGTM

+            Document* document = result.innerNodeFrame()->document();
             if (document) {
                 AXObjectCache* cache = document->existingAXObjectCache();
                 if (cache)
-                    cache->onTouchAccessibilityHover(pme.position());
+                    cache->onTouchAccessibilityHover(result.roundedPointInInnerNodeFrame());
             }
         }
     }
 
     // Report the event to be NOT processed by WebKit, so that the browser can handle it appropriately.
     if (m_ignoreInputEvents)
         return WebInputEventResult::NotHandled;
 
     TemporaryChange<const WebInputEvent*> currentEventChange(m_currentInputEvent, &inputEvent);
     UIEventWithKeyState::clearNewTabModifierSetFromIsolatedWorld();
@@ skipping 2342 matching lines @@
 {
     // TODO(oshima): Investigate if this should return the ScreenInfo's scale factor rather than
     // page's scale factor, which can be 1 in use-zoom-for-dsf mode.
     if (!page())
         return 1;
 
     return page()->deviceScaleFactor();
 }
 
 } // namespace blink