Skip to content

Commit

Permalink
[unseasoned-pdf] Fix PDFExtensionJSText.BasicPlugin
Browse files Browse the repository at this point in the history
Focuses the frame containing the plugin before trying to change the text
selection. This avoids a DCHECK when changing the text selection on that
frame, but the OnTextSelectionChanged() event subsequently gets sent to
the MimeHandlerView's frame, which isn't ready for it.

Fixed: 1229631
Change-Id: Ief54f4337271591d41313acd11b695f20c89daa2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3061429
Auto-Submit: K. Moon <kmoon@chromium.org>
Commit-Queue: Hui Yingst <nigi@chromium.org>
Reviewed-by: Hui Yingst <nigi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#906945}
  • Loading branch information
kmoon-work authored and Chromium LUCI CQ committed Jul 30, 2021
1 parent 2391737 commit 8b2f9b8
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 3 deletions.
3 changes: 1 addition & 2 deletions chrome/browser/pdf/pdf_extension_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -912,8 +912,7 @@ IN_PROC_BROWSER_TEST_P(PDFExtensionJSTest, Basic) {
EXPECT_EQ(1, CountPDFProcesses());
}

IN_PROC_BROWSER_TEST_F(PDFExtensionJSTestWithoutUnseasonedOverride,
BasicPlugin) {
IN_PROC_BROWSER_TEST_P(PDFExtensionJSTest, BasicPlugin) {
RunTestsInJsModule("basic_plugin_test.js", "test.pdf");
}

Expand Down
10 changes: 9 additions & 1 deletion pdf/pdf_view_web_plugin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
#include "third_party/blink/public/web/web_plugin_container.h"
#include "third_party/blink/public/web/web_plugin_params.h"
#include "third_party/blink/public/web/web_print_preset_options.h"
#include "third_party/blink/public/web/web_view.h"
#include "third_party/blink/public/web/web_widget.h"
#include "third_party/skia/include/core/SkRect.h"
#include "third_party/skia/include/core/SkRefCnt.h"
Expand Down Expand Up @@ -177,7 +178,14 @@ class BlinkContainerWrapper final : public PdfViewWebPlugin::ContainerWrapper {
void TextSelectionChanged(const blink::WebString& selection_text,
uint32_t offset,
const gfx::Range& range) override {
GetFrame()->TextSelectionChanged(selection_text, offset, range);
// Focus the plugin's containing frame before changing the text selection.
// TODO(crbug.com/1234559): Would it make more sense not to change the text
// selection at all in this case? Maybe we only have this problem because we
// support a "selectAll" message.
blink::WebLocalFrame* frame = GetFrame();
frame->View()->SetFocusedFrame(frame);

frame->TextSelectionChanged(selection_text, offset, range);
}

std::unique_ptr<blink::WebAssociatedURLLoader> CreateAssociatedURLLoader(
Expand Down

0 comments on commit 8b2f9b8

Please sign in to comment.