D9852: Fix crash due to dangling pointer in MouseAnnotation

[Tobias Deiminger]

tobiasdeiminger created this revision.
tobiasdeiminger added a reviewer: Okular.
tobiasdeiminger added a project: Okular.
tobiasdeiminger requested review of this revision.

REVISION SUMMARY
  Fixes BUG: 388228. Diff applies to Applications/17.12, and should be easy to merge to master. It's kept quite minimal as suggested by Albert.
  
  Albert also suggested to add a dedicated unit test and I'd agree, but am not yet sure how to do it. The original bug involves several classes, including UI: Document, Page, AddAnnotationCommand, PageView, PageViewAnnotator, MouseAnnotation - to name a few. So a test for the exact bug scenario would become a bigger integration test rather than an isolated unit test. The other approach would be to do a real unit test on MouseAnnotation. But again, MouseAnnotation has nasty dependencies (e.g., needs a parent PageView) which I'd have to mock. Any ideas? I'd be interested in a discussion on this topic.

TEST PLAN
  1. Load a document (e.g. linked PDF from bug report <http://www.philipebert.info/resources/WhatMathematicalKnowledgeCouldNotBe.pdf>) and enable highlight toolbar (F6).
  2. Create highlight annotation.
  3. Move the view port so that the annotation position is right beside the highlight tool icon.
  4. Move the mouse over the annotation, and then horizontally left until you reach the tool icon; it's important to stay over the highlight annotation as long as in viewport.
  5. Press ctrl-z for undo.
  6. Click on highlight tool, move right into the document, create new highlight annotation.
  7. Okular doesn't crash.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D9852

AFFECTED FILES
  ui/pageview.cpp
  ui/pageviewmouseannotation.cpp
  ui/pageviewmouseannotation.h

To: tobiasdeiminger, #okular
Cc: michaelweghorn, gassaf, ngraham, aacid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20180113/65d0f354/attachment-0001.html>