Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2855953003:
Handle long press in PDF documents. (Closed)
DescriptionHandle long press in PDF documents.
This Cl updates the touch handlers for PDF documents to better support
long press. The long press context menu is suppressed and the wonder
under the press is of sufficient time.
BUG=chromium:490184
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2855953003
Cr-Commit-Position: refs/heads/master@{#469849}
Committed: https://chromium.googlesource.com/chromium/src/+/6e51653e06d1740b5fe70b3ef9c2e6fe959e3968
Patch Set 1 #
Total comments: 13
Patch Set 2 : Review feedback #Patch Set 3 : Rebase to master #Patch Set 4 : Rebase to master #
Total comments: 4
Patch Set 5 : Add long press test #
Total comments: 16
Patch Set 6 : Review feedback #
Total comments: 6
Patch Set 7 : Review feedback #Patch Set 8 : Rebase to master #
Messages
Total messages: 45 (25 generated)
Description was changed from ========== Handle long press in PDF documents. This Cl updates the touch handlers for PDF documents to better support long press. The long press context menu is suppressed and the wonder under the press is of sufficient time. BUG=chromium:490184 ========== to ========== Handle long press in PDF documents. This Cl updates the touch handlers for PDF documents to better support long press. The long press context menu is suppressed and the wonder under the press is of sufficient time. BUG=chromium:490184 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Handle long press in PDF documents. This Cl updates the touch handlers for PDF documents to better support long press. The long press context menu is suppressed and the wonder under the press is of sufficient time. BUG=chromium:490184 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Handle long press in PDF documents. DO NOT COMMIT... This Cl updates the touch handlers for PDF documents to better support long press. The long press context menu is suppressed and the wonder under the press is of sufficient time. BUG=chromium:490184 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
dsinclair@chromium.org changed reviewers: + mcnee@chromium.org, raymes@chromium.org, thestig@chromium.org
PTAL. This is a first pass at getting long press handling working which makes text selection by press a touch a bit more consistent with Chromium. This should land after the CL mentioned in the comments. Does this seem like the right way to handle this? We still need to determine how to plumb the selection size back out to CC on order to get the handles for expanding the touch selection. https://codereview.chromium.org/2855953003/diff/1/chrome/browser/resources/pd... File chrome/browser/resources/pdf/gesture_detector.js (right): https://codereview.chromium.org/2855953003/diff/1/chrome/browser/resources/pd... chrome/browser/resources/pdf/gesture_detector.js:99: event.preventDefault(); This is very similar to https://codereview.chromium.org/2847893005/ and will be updated on top of that CL when it lands.
https://codereview.chromium.org/2855953003/diff/1/chrome/browser/resources/pd... File chrome/browser/resources/pdf/gesture_detector.js (right): https://codereview.chromium.org/2855953003/diff/1/chrome/browser/resources/pd... chrome/browser/resources/pdf/gesture_detector.js:20: 'touchstart', this.onTouchStart_.bind(this), { passive: false }); Since we're no longer calling preventDefault in this case, this should no longer be non-passive. This is updated in my CL, so you can just merge that. https://codereview.chromium.org/2855953003/diff/1/chrome/browser/resources/pd... chrome/browser/resources/pdf/gesture_detector.js:51: * @return {!bool} True if touch start was two finger. The ! is redundant since booleans are non-nullable. You can just use {boolean}. https://codereview.chromium.org/2855953003/diff/1/chrome/browser/resources/pd... chrome/browser/resources/pdf/gesture_detector.js:54: return this.touchStartEvent_ && this.touchStartEvent_.touches.length == 2; I notice that |touchStartEvent_| is never cleared even after the gesture ends. I take it by the time you call this, the current state of the gesture may have changed. But then we get that logic at the beginning of onTouch_ which is a bit unintuitive (there could be a |touchStartEvent_| without a |lastEvent_|). Perhaps it would be clearer if we just recorded the most recent number of fingers separately and just stored the events for the gesture detection logic. https://codereview.chromium.org/2855953003/diff/1/chrome/browser/resources/pd... File chrome/browser/resources/pdf/pdf.js (right): https://codereview.chromium.org/2855953003/diff/1/chrome/browser/resources/pd... chrome/browser/resources/pdf/pdf.js:417: // make sure the start event had 2 touches (which corresponds to our Don't you mean "the start event did not have 2 touches"?
https://codereview.chromium.org/2855953003/diff/1/pdf/out_of_process_instance.h File pdf/out_of_process_instance.h (right): https://codereview.chromium.org/2855953003/diff/1/pdf/out_of_process_instance... pdf/out_of_process_instance.h:241: pp::Point scroll_offset_; BTW, there's another scroll offset below that's a pp::FloatPoint. Haven't looked at if we need both, or if this needs to be a FloatPoint. https://codereview.chromium.org/2855953003/diff/1/pdf/pdfium/pdfium_engine.h File pdf/pdfium/pdfium_engine.h (right): https://codereview.chromium.org/2855953003/diff/1/pdf/pdfium/pdfium_engine.h#... pdf/pdfium/pdfium_engine.h:683: int next_touch_timer_id_; This isn't initialized?
https://codereview.chromium.org/2855953003/diff/1/chrome/browser/resources/pd... File chrome/browser/resources/pdf/gesture_detector.js (right): https://codereview.chromium.org/2855953003/diff/1/chrome/browser/resources/pd... chrome/browser/resources/pdf/gesture_detector.js:20: 'touchstart', this.onTouchStart_.bind(this), { passive: false }); On 2017/05/02 23:18:18, Kevin McNee wrote: > Since we're no longer calling preventDefault in this case, this should no longer > be non-passive. > This is updated in my CL, so you can just merge that. Yea, I'm going to use your Cl for this bit, I'd done this before I saw your change. https://codereview.chromium.org/2855953003/diff/1/chrome/browser/resources/pd... chrome/browser/resources/pdf/gesture_detector.js:51: * @return {!bool} True if touch start was two finger. On 2017/05/02 23:18:18, Kevin McNee wrote: > The ! is redundant since booleans are non-nullable. You can just use {boolean}. Done. https://codereview.chromium.org/2855953003/diff/1/chrome/browser/resources/pd... chrome/browser/resources/pdf/gesture_detector.js:54: return this.touchStartEvent_ && this.touchStartEvent_.touches.length == 2; On 2017/05/02 23:18:19, Kevin McNee wrote: > I notice that |touchStartEvent_| is never cleared even after the gesture ends. I > take it by the time you call this, the current state of the gesture may have > changed. > > But then we get that logic at the beginning of onTouch_ which is a bit > unintuitive (there could be a |touchStartEvent_| without a |lastEvent_|). > Perhaps it would be clearer if we just recorded the most recent number of > fingers separately and just stored the events for the gesture detection logic. Done. https://codereview.chromium.org/2855953003/diff/1/chrome/browser/resources/pd... File chrome/browser/resources/pdf/pdf.js (right): https://codereview.chromium.org/2855953003/diff/1/chrome/browser/resources/pd... chrome/browser/resources/pdf/pdf.js:417: // make sure the start event had 2 touches (which corresponds to our On 2017/05/02 23:18:19, Kevin McNee wrote: > Don't you mean "the start event did not have 2 touches"? Done. https://codereview.chromium.org/2855953003/diff/1/pdf/out_of_process_instance.h File pdf/out_of_process_instance.h (right): https://codereview.chromium.org/2855953003/diff/1/pdf/out_of_process_instance... pdf/out_of_process_instance.h:241: pp::Point scroll_offset_; On 2017/05/03 01:52:53, Lei Zhang wrote: > BTW, there's another scroll offset below that's a pp::FloatPoint. Haven't looked > at if we need both, or if this needs to be a FloatPoint. The one below is used specifically for pinch zooming. It doesn't get it's size from pepper, it gets it from the pinch message. This one could be a float, but we use the number as an int later anyway, so just storing as an int seems fine. https://codereview.chromium.org/2855953003/diff/1/pdf/pdfium/pdfium_engine.h File pdf/pdfium/pdfium_engine.h (right): https://codereview.chromium.org/2855953003/diff/1/pdf/pdfium/pdfium_engine.h#... pdf/pdfium/pdfium_engine.h:683: int next_touch_timer_id_; On 2017/05/03 01:52:53, Lei Zhang wrote: > This isn't initialized? Done.
The CQ bit was checked by dsinclair@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by dsinclair@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
https://codereview.chromium.org/2855953003/diff/60001/chrome/test/data/pdf/ge... File chrome/test/data/pdf/gesture_detector_test.js (right): https://codereview.chromium.org/2855953003/diff/60001/chrome/test/data/pdf/ge... chrome/test/data/pdf/gesture_detector_test.js:222: {clientX: 0, clientY: 0, touches: []} The array passed to MockTouchEvent's constructor is the |touches| array. e.g. Send a 1 finger touch start: stubElement.sendEvent(new MockTouchEvent('touchstart', [ {clientX: 0, clientY: 0}, ])); Send a 2 finger touch start: stubElement.sendEvent(new MockTouchEvent('touchstart', [ {clientX: 0, clientY: 0}, {clientX: 0, clientY: 2} ])); https://codereview.chromium.org/2855953003/diff/60001/chrome/test/data/pdf/ge... chrome/test/data/pdf/gesture_detector_test.js:238: } If you're relying on the value of |wasTwoFingerTouch()| to stick around after the end of the gesture, you might want to assert that |wasTwoFingerTouch()| stays true after a touchend. stubElement.sendEvent(new MockTouchEvent('touchend', []));
https://codereview.chromium.org/2855953003/diff/60001/chrome/test/data/pdf/ge... File chrome/test/data/pdf/gesture_detector_test.js (right): https://codereview.chromium.org/2855953003/diff/60001/chrome/test/data/pdf/ge... chrome/test/data/pdf/gesture_detector_test.js:222: {clientX: 0, clientY: 0, touches: []} On 2017/05/03 17:25:43, Kevin McNee wrote: > The array passed to MockTouchEvent's constructor is the |touches| array. > > e.g. > Send a 1 finger touch start: > stubElement.sendEvent(new MockTouchEvent('touchstart', [ > {clientX: 0, clientY: 0}, > ])); > > Send a 2 finger touch start: > stubElement.sendEvent(new MockTouchEvent('touchstart', [ > {clientX: 0, clientY: 0}, > {clientX: 0, clientY: 2} > ])); Awesome, thanks. I'd guessed and was going to start tracking down the failure after lunch. (Didn't mean to commit this yet, got pulled in by accident when I rebased) https://codereview.chromium.org/2855953003/diff/60001/chrome/test/data/pdf/ge... chrome/test/data/pdf/gesture_detector_test.js:238: } On 2017/05/03 17:25:43, Kevin McNee wrote: > If you're relying on the value of |wasTwoFingerTouch()| to stick around after > the end of the gesture, you might want to assert that |wasTwoFingerTouch()| > stays true after a touchend. > > stubElement.sendEvent(new MockTouchEvent('touchend', [])); Ah, good idea.
Description was changed from ========== Handle long press in PDF documents. DO NOT COMMIT... This Cl updates the touch handlers for PDF documents to better support long press. The long press context menu is suppressed and the wonder under the press is of sufficient time. BUG=chromium:490184 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Handle long press in PDF documents. This Cl updates the touch handlers for PDF documents to better support long press. The long press context menu is suppressed and the wonder under the press is of sufficient time. BUG=chromium:490184 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
PTAL. I think this is ready to go, I added a test to make sure long press worked correctly.
The CQ bit was checked by dsinclair@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Just some nits. Mainly style. https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/ge... File chrome/test/data/pdf/gesture_detector_test.js (right): https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/ge... chrome/test/data/pdf/gesture_detector_test.js:225: stubElement.sendEvent(new MockTouchEvent('touchstart', [])); nit: AFAIK I don't think it's possible to have a touchstart with no touches. I'd do a 1 finger touchstart here, especially since a 1 finger touchstart followed by a 2 finger touchstart is what you would see with an actual 2 finger tap. https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/ge... chrome/test/data/pdf/gesture_detector_test.js:231: {clientX: 2, clientY: 2}])); nit: style. Let's be consistent in this file and use block formatting for all of these touch arrays. 2 space indent, line break before "]" https://google.github.io/styleguide/jsguide.html#formatting-array-literals https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/ge... chrome/test/data/pdf/gesture_detector_test.js:233: "Should have a two finger touch.");; nit: double semicolon https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/to... File chrome/test/data/pdf/touch_handling_test.js (right): https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/to... chrome/test/data/pdf/touch_handling_test.js:7: let touch_list = touches.map(function(xy) { nit: touchList https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/to... chrome/test/data/pdf/touch_handling_test.js:21: changedtouches: touch_list})); nit: formatting https://google.github.io/styleguide/jsguide.html#formatting-object-literals https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/to... chrome/test/data/pdf/touch_handling_test.js:26: cancelable: true, nit: formatting https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/to... chrome/test/data/pdf/touch_handling_test.js:40: "Should have called preventDefault() for single touch."); nit: formatting https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/to... chrome/test/data/pdf/touch_handling_test.js:50: "Should not have called preventDefault() for double touch."); nit: formatting
https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/ge... File chrome/test/data/pdf/gesture_detector_test.js (right): https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/ge... chrome/test/data/pdf/gesture_detector_test.js:225: stubElement.sendEvent(new MockTouchEvent('touchstart', [])); On 2017/05/04 15:38:55, Kevin McNee wrote: > nit: AFAIK I don't think it's possible to have a touchstart with no touches. I'd > do a 1 finger touchstart here, especially since a 1 finger touchstart followed > by a 2 finger touchstart is what you would see with an actual 2 finger tap. Done. https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/ge... chrome/test/data/pdf/gesture_detector_test.js:231: {clientX: 2, clientY: 2}])); On 2017/05/04 15:38:55, Kevin McNee wrote: > nit: style. Let's be consistent in this file and use block formatting for all of > these touch arrays. > > 2 space indent, line break before "]" > https://google.github.io/styleguide/jsguide.html#formatting-array-literals Done. https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/ge... chrome/test/data/pdf/gesture_detector_test.js:233: "Should have a two finger touch.");; On 2017/05/04 15:38:55, Kevin McNee wrote: > nit: double semicolon Done. https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/to... File chrome/test/data/pdf/touch_handling_test.js (right): https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/to... chrome/test/data/pdf/touch_handling_test.js:7: let touch_list = touches.map(function(xy) { On 2017/05/04 15:38:55, Kevin McNee wrote: > nit: touchList Done. https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/to... chrome/test/data/pdf/touch_handling_test.js:21: changedtouches: touch_list})); On 2017/05/04 15:38:55, Kevin McNee wrote: > nit: formatting > https://google.github.io/styleguide/jsguide.html#formatting-object-literals Done. https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/to... chrome/test/data/pdf/touch_handling_test.js:26: cancelable: true, On 2017/05/04 15:38:55, Kevin McNee wrote: > nit: formatting Done. https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/to... chrome/test/data/pdf/touch_handling_test.js:40: "Should have called preventDefault() for single touch."); On 2017/05/04 15:38:55, Kevin McNee wrote: > nit: formatting Done. https://codereview.chromium.org/2855953003/diff/80001/chrome/test/data/pdf/to... chrome/test/data/pdf/touch_handling_test.js:50: "Should not have called preventDefault() for double touch."); On 2017/05/04 15:38:55, Kevin McNee wrote: > nit: formatting Done.
https://codereview.chromium.org/2855953003/diff/100001/chrome/test/data/pdf/g... File chrome/test/data/pdf/gesture_detector_test.js (right): https://codereview.chromium.org/2855953003/diff/100001/chrome/test/data/pdf/g... chrome/test/data/pdf/gesture_detector_test.js:229: "Should not have a two finger touch with no touches."); nit: update reason as well
dsinclair@chromium.org changed reviewers: - raymes@chromium.org
thestig@ can you remove the c++ side of this CL?
On 2017/05/04 19:18:17, dsinclair wrote: > thestig@ can you remove the c++ side of this CL? Review, you mean? :)
BTW, I landed some C++ side cleanups. You may need to rebase.
On 2017/05/04 19:20:15, Lei Zhang wrote: > On 2017/05/04 19:18:17, dsinclair wrote: > > thestig@ can you remove the c++ side of this CL? > > Review, you mean? :) Hah, I'm stuck in 'Remove IFX_Pause' mode apparently. Yes, review, thanks.
lgtm https://codereview.chromium.org/2855953003/diff/100001/pdf/out_of_process_ins... File pdf/out_of_process_instance.h (right): https://codereview.chromium.org/2855953003/diff/100001/pdf/out_of_process_ins... pdf/out_of_process_instance.h:240: // The scroll offset Can you mention the units? https://codereview.chromium.org/2855953003/diff/100001/pdf/pdf_engine.h File pdf/pdf_engine.h (right): https://codereview.chromium.org/2855953003/diff/100001/pdf/pdf_engine.h#newco... pdf/pdf_engine.h:145: // Calls the client's OnTouchTimerCallback() function in delay_in_ms with Refer to |variable_name| per style guide. Fix the incorrect one above.
https://codereview.chromium.org/2855953003/diff/100001/chrome/test/data/pdf/g... File chrome/test/data/pdf/gesture_detector_test.js (right): https://codereview.chromium.org/2855953003/diff/100001/chrome/test/data/pdf/g... chrome/test/data/pdf/gesture_detector_test.js:229: "Should not have a two finger touch with no touches."); On 2017/05/04 18:29:34, Kevin McNee wrote: > nit: update reason as well Done. https://codereview.chromium.org/2855953003/diff/100001/pdf/out_of_process_ins... File pdf/out_of_process_instance.h (right): https://codereview.chromium.org/2855953003/diff/100001/pdf/out_of_process_ins... pdf/out_of_process_instance.h:240: // The scroll offset On 2017/05/04 19:29:26, Lei Zhang wrote: > Can you mention the units? Done. https://codereview.chromium.org/2855953003/diff/100001/pdf/pdf_engine.h File pdf/pdf_engine.h (right): https://codereview.chromium.org/2855953003/diff/100001/pdf/pdf_engine.h#newco... pdf/pdf_engine.h:145: // Calls the client's OnTouchTimerCallback() function in delay_in_ms with On 2017/05/04 19:29:26, Lei Zhang wrote: > Refer to |variable_name| per style guide. Fix the incorrect one above. Done.
The CQ bit was checked by dsinclair@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by dsinclair@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org Link to the patchset: https://codereview.chromium.org/2855953003/#ps140001 (title: "Rebase to master")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 140001, "attempt_start_ts": 1494069956137090,
"parent_rev": "8c4edd07d3cf96c44d1995e491f8789a7d38a5fd", "commit_rev":
"6e51653e06d1740b5fe70b3ef9c2e6fe959e3968"}
Message was sent while issue was closed.
Description was changed from ========== Handle long press in PDF documents. This Cl updates the touch handlers for PDF documents to better support long press. The long press context menu is suppressed and the wonder under the press is of sufficient time. BUG=chromium:490184 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Handle long press in PDF documents. This Cl updates the touch handlers for PDF documents to better support long press. The long press context menu is suppressed and the wonder under the press is of sufficient time. BUG=chromium:490184 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2855953003 Cr-Commit-Position: refs/heads/master@{#469849} Committed: https://chromium.googlesource.com/chromium/src/+/6e51653e06d1740b5fe70b3ef9c2... ==========
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as https://chromium.googlesource.com/chromium/src/+/6e51653e06d1740b5fe70b3ef9c2...
Message was sent while issue was closed.
Findit (https://goo.gl/kROfz5) identified this CL at revision 469849 as the culprit for failures in the build cycles as shown on: https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb...
Message was sent while issue was closed.
A revert of this CL (patchset #8 id:140001) has been created in https://codereview.chromium.org/2864603006/ by avi@chromium.org. The reason for reverting is: Breaks Mac bots. See https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb... https://build.chromium.org/p/chromium.mac/builders/Mac10.11%20Tests/builds/12082 [ RUN ] PDFExtensionTest.TouchHandling [21973:14375:0506/122844.659425:WARNING:notification_platform_bridge_mac.mm(493)] AlertNotificationService: XPC connection invalidated. 2017-05-06 12:28:44.811 browser_tests[21973:153848] NSWindow warning: adding an unknown subview: <FullSizeContentView: 0x7fb0f2d756b0>. Break on NSLog to debug. 2017-05-06 12:28:44.812 browser_tests[21973:153848] Call stack: ( "+callStackSymbols disabled for performance reasons" ) [21973:87319:0506/122847.139773:WARNING:embedded_test_server.cc(219)] Request not handled. Returning 404: /favicon.ico [21973:35843:0506/122850.985727:ERROR:service_manager.cc(137)] Connection InterfaceProviderSpec prevented service: content_plugin from binding interface: memory_instrumentation::mojom::Coordinator exposed by: content_browser [21973:775:0506/122851.245843:INFO:CONSOLE(0)] "[SUCCESS] testContextMenuSingleTouch", source: chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/index.html (0) [21973:775:0506/122851.307976:INFO:CONSOLE(0)] "[SUCCESS] testContextMenuDoubleTouch", source: chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/index.html (0) [21973:775:0506/122851.669523:INFO:CONSOLE(0)] "[SUCCESS] testLongPressSelectsText", source: chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/index.html (0) [21973:775:0506/122851.674726:INFO:CONSOLE(0)] "[FAIL] testLongPressSelectsText: API Test Error in testLongPressSelectsText Actual: Expected: some Error at Object.handleRequest (extensions::binding:63:27) at Object.<anonymous> (extensions::binding:422:32) at Object.<anonymous> (extensions::test:215:18) at Object.handleRequest (extensions::binding:63:27) at Object.<anonymous> (extensions::binding:422:32) at <anonymous>:192:21 at safeFunctionApply (extensions::test:260:26) at PDFScriptingAPI.selectedTextCallback_ (extensions::test:285:18) at PDFScriptingAPI.<anonymous> (chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/pdf_scripting_api.js:81:16)", source: chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/index.html (0) [21973:775:0506/122851.676537:INFO:CONSOLE(263)] "Uncaught chrome.test.failure", source: extensions::test (263) ../../chrome/browser/pdf/pdf_extension_test.cc:165: Failure Failed Failed 1 of 3 tests [ FAILED ] PDFExtensionTest.TouchHandling, where TypeParam = and GetParam() = (8405 ms) . |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
