-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Reland "Move implementation of Mojo method GetTextSurroundingSelectio…
…n() to Blink" We can't construct a WTF::String using the default constructor when running the GetTextSurroundingSelectionCallback directly for those cases where the SurroundingText is empty, since that will construct a null string instead of an empty string, causing a crash at the Mojo level due to trying to send a null message over the wire, which is not permitted. To avoid this crash in Mojo, make sure we create a WTF::String out of an empty string, instead of using the default constructor. Last, this CL also incorporates the required changes to address Daniel's comments after the original patch landed in CL 1696968 [1], to rename the newly added Mojo interface to become blink::mojom::Frame. [1] https://crrev.com/c/1696968/11/third_party/blink/public/mojom/editing/editing.mojom#9 Original change's description: > Move this method from the content.mojom.Frame Mojo interface into Blink, > as part of a new blink.mojom.SurroundingText Mojo interface, and add a new > class there to implement such interface (i.e. blink::SurroundingTextImpl), > to replace usages of content.mojom.Frame's GetTextSurroundingSelection() > from the browser process. > > Note that this SurroundingTextImpl class still relies on WebSurroundingText > and WebLocalFrame to keep the change smaller, but this is a temporary step > that will be corrected once WebSurroundingText has been moved out of the > public API and into renderer/core, which will happen on a follow-up CL. > > Bug: 980151 > Change-Id: I62fb2d5b24a98773c1acddbe1c58217c1a2d53e4 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1696968 > Commit-Queue: Mario Sanchez Prada <mario@igalia.com> > Reviewed-by: Mike West <mkwst@chromium.org> > Reviewed-by: Ken Buchanan <kenrb@chromium.org> > Reviewed-by: Kentaro Hara <haraken@chromium.org> > Cr-Commit-Position: refs/heads/master@{#679981} Bug: 980151, 987214, 987191, 989438 Change-Id: Ia32be36c1a7524930ddcecc79e7fac5bcb514a13 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1735459 Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Mario Sanchez Prada <mario@igalia.com> Cr-Commit-Position: refs/heads/master@{#686348}
- Loading branch information
Showing
15 changed files
with
159 additions
and
31 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| // Copyright 2019 The Chromium Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| module blink.mojom; | ||
|
|
||
| import "mojo/public/mojom/base/string16.mojom"; | ||
|
|
||
| // Implemented in Blink, this interface defines frame-specific methods that will | ||
| // be invoked from the browser process (e.g. content::RenderFrameHostImpl). | ||
| // | ||
| // Note that this is different than content/common/frame.mojom in that the | ||
| // methods defined here are handled directly in Blink without passing through | ||
| // content. In the future this interface will likely host more methods as the | ||
| // Onion Soup project advances, which can potentially leading to the removal of | ||
| // content/common/frame.mojom in the future if enough code is moved to Blink. | ||
| interface Frame { | ||
| // Retrieves the text surrounding the current selection for the frame up to | ||
| // the length specified by |max_length|, along with its start and end offsets. | ||
| GetTextSurroundingSelection(uint32 max_length) | ||
| => (mojo_base.mojom.String16 content, uint32 start_offset, | ||
| uint32 end_offset); | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| // Copyright 2019 The Chromium Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #include "third_party/blink/renderer/core/frame/frame_impl.h" | ||
|
|
||
| #include <utility> | ||
|
|
||
| #include "third_party/blink/public/platform/task_type.h" | ||
| #include "third_party/blink/public/web/web_surrounding_text.h" | ||
| #include "third_party/blink/renderer/core/frame/local_frame.h" | ||
| #include "third_party/blink/renderer/core/frame/web_local_frame_impl.h" | ||
| #include "third_party/blink/renderer/platform/heap/persistent.h" | ||
| #include "third_party/blink/renderer/platform/wtf/functional.h" | ||
| #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h" | ||
|
|
||
| namespace blink { | ||
|
|
||
| FrameImpl::FrameImpl(WebLocalFrameImpl& frame, | ||
| InterfaceRegistry* interface_registry) | ||
| : frame_(&frame) { | ||
| if (!interface_registry) | ||
| return; | ||
| // TODO(crbug.com/800641): Use InterfaceValidator when it works for associated | ||
| // interfaces. | ||
| interface_registry->AddAssociatedInterface( | ||
| WTF::BindRepeating(&FrameImpl::BindToReceiver, WrapWeakPersistent(this))); | ||
| } | ||
|
|
||
| void FrameImpl::BindToReceiver( | ||
| mojo::PendingAssociatedReceiver<mojom::blink::Frame> receiver) { | ||
| receiver_.Bind(std::move(receiver), | ||
| frame_->GetTaskRunner(blink::TaskType::kInternalDefault)); | ||
| } | ||
|
|
||
| void FrameImpl::GetTextSurroundingSelection( | ||
| uint32_t max_length, | ||
| GetTextSurroundingSelectionCallback callback) { | ||
| blink::WebSurroundingText surrounding_text(frame_, max_length); | ||
|
|
||
| // |surrounding_text| might not be correctly initialized, for example if | ||
| // |frame_->SelectionRange().IsNull()|, in other words, if there was no | ||
| // selection. | ||
| if (surrounding_text.IsEmpty()) { | ||
| // Don't use WTF::String's default constructor so that we make sure that we | ||
| // always send a valid empty string over the wire instead of a null pointer. | ||
| std::move(callback).Run(g_empty_string, 0, 0); | ||
| return; | ||
| } | ||
|
|
||
| std::move(callback).Run(surrounding_text.TextContent(), | ||
| surrounding_text.StartOffsetInTextContent(), | ||
| surrounding_text.EndOffsetInTextContent()); | ||
| } | ||
|
|
||
| } // namespace blink |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| // Copyright 2019 The Chromium Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_FRAME_FRAME_IMPL_H_ | ||
| #define THIRD_PARTY_BLINK_RENDERER_CORE_FRAME_FRAME_IMPL_H_ | ||
|
|
||
| #include "mojo/public/cpp/bindings/associated_receiver.h" | ||
| #include "mojo/public/cpp/bindings/pending_associated_receiver.h" | ||
| #include "third_party/blink/public/mojom/frame/frame.mojom-blink.h" | ||
| #include "third_party/blink/public/platform/interface_registry.h" | ||
| #include "third_party/blink/public/web/web_local_frame.h" | ||
| #include "third_party/blink/renderer/core/core_export.h" | ||
| #include "third_party/blink/renderer/platform/heap/garbage_collected.h" | ||
|
|
||
| namespace blink { | ||
|
|
||
| class WebLocalFrameImpl; | ||
|
|
||
| // Implementation of blink::mojom::Frame | ||
| class CORE_EXPORT FrameImpl final : public GarbageCollectedFinalized<FrameImpl>, | ||
| public mojom::blink::Frame { | ||
| public: | ||
| // TODO(crbug.com/980151): Construct FrameImpl this way only while we need to | ||
| // rely in blink::WebSurroundingText to implement GetTextSurroundingSelection, | ||
| // and remove the dependency once WebSurroundingText is out of the public API. | ||
| FrameImpl(WebLocalFrameImpl& frame, InterfaceRegistry* interface_registry); | ||
|
|
||
| void BindToReceiver( | ||
| mojo::PendingAssociatedReceiver<mojom::blink::Frame> receiver); | ||
|
|
||
| void GetTextSurroundingSelection( | ||
| uint32_t max_length, | ||
| GetTextSurroundingSelectionCallback callback) final; | ||
|
|
||
| void Trace(blink::Visitor* visitor) { visitor->Trace(frame_); } | ||
|
|
||
| private: | ||
| const Member<WebLocalFrameImpl> frame_; | ||
|
|
||
| mojo::AssociatedReceiver<mojom::blink::Frame> receiver_{this}; | ||
|
|
||
| DISALLOW_COPY_AND_ASSIGN(FrameImpl); | ||
| }; | ||
|
|
||
| } // namespace blink | ||
|
|
||
| #endif // THIRD_PARTY_BLINK_RENDERER_CORE_FRAME_FRAME_IMPL_H_ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters