Changeset 240342 in webkit


Ignore:
Timestamp:
Jan 23, 2019 9:30:05 AM (5 years ago)
Author:
Wenson Hsieh
Message:

Introduce UndoStep::label() and adopt it in WebKitLegacy and WebKit
​https://bugs.webkit.org/show_bug.cgi?id=193706
<rdar://problem/44807048>

Reviewed by Ryosuke Niwa.

Source/WebCore:

Refactors some existing logic when registering undoable actions, such that we propagate the undoable action's
label string instead of the EditAction to the client layer. This will help make handling of CustomUndoStep's
undo/redo label simpler, as the client layer won't need to worry about managing an EditAction and undo/redo
label simultaneously. There should be no change in behavior.

  • editing/CompositeEditCommand.cpp:

(WebCore::EditCommandComposition::label const):

  • editing/CompositeEditCommand.h:
  • editing/CustomUndoStep.cpp:

(WebCore::CustomUndoStep::label const):

  • editing/CustomUndoStep.h:
  • editing/EditAction.cpp:

(WebCore::undoRedoLabel):
(WebCore::nameForUndoRedo): Deleted.

  • editing/EditAction.h:

Rename nameForUndoRedo to undoRedoLabel, and remove the WEBCORE_EXPORT since it's no longer needed in WebKit or
WebKitLegacy.

  • editing/UndoStep.h:

Add UndoStep::label(). While EditCommandComposition implements this by mapping its EditAction to a
localized string, CustomUndoStep implements this by returning the undoable action label provided by script.

Source/WebKit:

  • UIProcess/Cocoa/WebViewImpl.mm:

(WebKit::WebViewImpl::registerEditCommand):

  • UIProcess/WebEditCommandProxy.cpp:

(WebKit::WebEditCommandProxy::WebEditCommandProxy):

  • UIProcess/WebEditCommandProxy.h:

Drive-by tweak: make WebEditCommandProxy's backpointer to its WebPageProxy a WeakPtr instead of a raw pointer.
Additionally, call clear() instead of setting m_page to 0 upon invalidation. Also, turn the WebPageProxy*
argument into a WebPageProxy&, since the WebPageProxy must exist when it creates a new WebEditCommandProxy.

(WebKit::WebEditCommandProxy::create):
(WebKit::WebEditCommandProxy::label const):
(WebKit::WebEditCommandProxy::invalidate):
(WebKit::WebEditCommandProxy::editAction const): Deleted.

Adjust UI-side logic to just handle the undo/redo label, rather than map the edit action to a localized string.

  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::registerEditCommandForUndo):
(WebKit::WebPageProxy::resetState):

Tweak this to use std::exchange instead of copying all the WebEditCommandProxy RefPtrs into a separate Vector
and then iterating over the Vector.

  • UIProcess/WebPageProxy.h:
  • UIProcess/WebPageProxy.messages.in:

Adjust this so that we only send the undo/redo label over IPC, rather than the edit action type.

  • UIProcess/ios/PageClientImplIOS.mm:

(WebKit::PageClientImpl::registerEditCommand):

  • WebProcess/WebCoreSupport/WebEditorClient.cpp:

(WebKit::WebEditorClient::registerUndoStep):

Source/WebKitLegacy/mac:

Use UndoStep::label().

  • WebCoreSupport/WebEditorClient.h:
  • WebCoreSupport/WebEditorClient.mm:

(-[WebUndoStep initWithUndoStep:]):
(+[WebUndoStep stepWithUndoStep:]):
(WebEditorClient::registerUndoOrRedoStep):
(WebEditorClient::registerUndoStep):
(WebEditorClient::registerRedoStep):

Location:
trunk/Source
Files:
19 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r240340 r240342  
     12019-01-23  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        Introduce UndoStep::label() and adopt it in WebKitLegacy and WebKit
     4        https://bugs.webkit.org/show_bug.cgi?id=193706
     5        <rdar://problem/44807048>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        Refactors some existing logic when registering undoable actions, such that we propagate the undoable action's
     10        label string instead of the EditAction to the client layer. This will help make handling of CustomUndoStep's
     11        undo/redo label simpler, as the client layer won't need to worry about managing an EditAction and undo/redo
     12        label simultaneously. There should be no change in behavior.
     13
     14        * editing/CompositeEditCommand.cpp:
     15        (WebCore::EditCommandComposition::label const):
     16        * editing/CompositeEditCommand.h:
     17        * editing/CustomUndoStep.cpp:
     18        (WebCore::CustomUndoStep::label const):
     19        * editing/CustomUndoStep.h:
     20        * editing/EditAction.cpp:
     21        (WebCore::undoRedoLabel):
     22        (WebCore::nameForUndoRedo): Deleted.
     23        * editing/EditAction.h:
     24
     25        Rename nameForUndoRedo to undoRedoLabel, and remove the WEBCORE_EXPORT since it's no longer needed in WebKit or
     26        WebKitLegacy.
     27
     28        * editing/UndoStep.h:
     29
     30        Add UndoStep::label(). While EditCommandComposition implements this by mapping its EditAction to a
     31        localized string, CustomUndoStep implements this by returning the undoable action label provided by script.
     32
    1332019-01-23  Michael Catanzaro  <mcatanzaro@igalia.com>
    234

  • trunk/Source/WebCore/editing/CompositeEditCommand.cpp

    r240237 r240342  
    303303#endif
    304304
     305String EditCommandComposition::label() const
     306{
     307    return undoRedoLabel(m_editAction);
     308}
     309
    305310CompositeEditCommand::CompositeEditCommand(Document& document, EditAction editingAction)
    306311    : EditCommand(document, editingAction)

  • trunk/Source/WebCore/editing/CompositeEditCommand.h

    r239427 r240342  
    8888private:
    8989    EditCommandComposition(Document&, const VisibleSelection& startingSelection, const VisibleSelection& endingSelection, EditAction);
     90
     91    String label() const final;
    9092
    9193    RefPtr<Document> m_document;

  • trunk/Source/WebCore/editing/CustomUndoStep.cpp

    r240328 r240342  
    6666}
    6767
     68String CustomUndoStep::label() const
     69{
     70    if (!isValid()) {
     71        ASSERT_NOT_REACHED();
     72        return emptyString();
     73    }
     74    return m_undoItem->label();
     75}
     76
    6877} // namespace WebCore

  • trunk/Source/WebCore/editing/CustomUndoStep.h

    r240328 r240342  
    4848    void reapply() final;
    4949    EditAction editingAction() const final { return EditAction::Unspecified; }
     50    String label() const final;
    5051
    5152    bool isValid() const;

  • trunk/Source/WebCore/editing/EditAction.cpp

    r239627 r240342  
    3131namespace WebCore {
    3232
    33 String nameForUndoRedo(EditAction editAction)
     33String undoRedoLabel(EditAction editAction)
    3434{
    3535    switch (editAction) {

  • trunk/Source/WebCore/editing/EditAction.h

    r239627 r240342  
    9696};
    9797
    98 WEBCORE_EXPORT WTF::String nameForUndoRedo(EditAction);
     98WTF::String undoRedoLabel(EditAction);
    9999
    100100} // namespace WebCore

  • trunk/Source/WebCore/editing/UndoStep.h

    r223728 r240342  
    3333#include "EditAction.h"
    3434#include <wtf/RefCounted.h>
     35#include <wtf/text/WTFString.h>
    3536
    3637namespace WebCore {
    … …  
    4344    virtual void reapply() = 0;
    4445    virtual EditAction editingAction() const = 0;
     46    virtual String label() const = 0;
    4547};
    4648

  • trunk/Source/WebKit/ChangeLog

    r240341 r240342  
     12019-01-23  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        Introduce UndoStep::label() and adopt it in WebKitLegacy and WebKit
     4        https://bugs.webkit.org/show_bug.cgi?id=193706
     5        <rdar://problem/44807048>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        * UIProcess/Cocoa/WebViewImpl.mm:
     10        (WebKit::WebViewImpl::registerEditCommand):
     11        * UIProcess/WebEditCommandProxy.cpp:
     12        (WebKit::WebEditCommandProxy::WebEditCommandProxy):
     13        * UIProcess/WebEditCommandProxy.h:
     14
     15        Drive-by tweak: make WebEditCommandProxy's backpointer to its WebPageProxy a WeakPtr instead of a raw pointer.
     16        Additionally, call clear() instead of setting m_page to 0 upon invalidation. Also, turn the WebPageProxy*
     17        argument into a WebPageProxy&, since the WebPageProxy must exist when it creates a new WebEditCommandProxy.
     18
     19        (WebKit::WebEditCommandProxy::create):
     20        (WebKit::WebEditCommandProxy::label const):
     21        (WebKit::WebEditCommandProxy::invalidate):
     22        (WebKit::WebEditCommandProxy::editAction const): Deleted.
     23
     24        Adjust UI-side logic to just handle the undo/redo label, rather than map the edit action to a localized string.
     25
     26        * UIProcess/WebPageProxy.cpp:
     27        (WebKit::WebPageProxy::registerEditCommandForUndo):
     28        (WebKit::WebPageProxy::resetState):
     29
     30        Tweak this to use std::exchange instead of copying all the WebEditCommandProxy RefPtrs into a separate Vector
     31        and then iterating over the Vector.
     32
     33        * UIProcess/WebPageProxy.h:
     34        * UIProcess/WebPageProxy.messages.in:
     35
     36        Adjust this so that we only send the undo/redo label over IPC, rather than the edit action type.
     37
     38        * UIProcess/ios/PageClientImplIOS.mm:
     39        (WebKit::PageClientImpl::registerEditCommand):
     40        * WebProcess/WebCoreSupport/WebEditorClient.cpp:
     41        (WebKit::WebEditorClient::registerUndoStep):
     42
    1432019-01-23  Michael Catanzaro  <mcatanzaro@igalia.com>
    244

  • trunk/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm

    r240219 r240342  
    26932693void WebViewImpl::registerEditCommand(Ref<WebEditCommandProxy>&& command, UndoOrRedo undoOrRedo)
    26942694{
    2695     auto actionName = WebCore::nameForUndoRedo(command->editAction());
     2695    auto actionName = command->label();
    26962696    auto commandObjC = adoptNS([[WKEditCommand alloc] initWithWebEditCommandProxy:WTFMove(command)]);
    26972697

  • trunk/Source/WebKit/UIProcess/WebEditCommandProxy.cpp

    r239627 r240342  
    3737using namespace WebCore;
    3838
    39 WebEditCommandProxy::WebEditCommandProxy(WebUndoStepID commandID, WebCore::EditAction editAction, WebPageProxy* page)
     39WebEditCommandProxy::WebEditCommandProxy(WebUndoStepID commandID, const String& label, WebPageProxy& page)
    4040    : m_commandID(commandID)
    41     , m_editAction(editAction)
    42     , m_page(page)
     41    , m_label(label)
     42    , m_page(makeWeakPtr(page))
    4343{
    4444    m_page->addEditCommand(*this);

  • trunk/Source/WebKit/UIProcess/WebEditCommandProxy.h

    r239627 r240342  
    3232#include <wtf/RefCounted.h>
    3333#include <wtf/RefPtr.h>
     34#include <wtf/WeakPtr.h>
     35#include <wtf/text/WTFString.h>
    3436
    3537namespace WebKit {
    … …  
    3941class WebEditCommandProxy : public API::ObjectImpl<API::Object::Type::EditCommandProxy> {
    4042public:
    41     static Ref<WebEditCommandProxy> create(WebUndoStepID commandID, WebCore::EditAction editAction, WebPageProxy* page)
     43    static Ref<WebEditCommandProxy> create(WebUndoStepID commandID, const String& label, WebPageProxy& page)
    4244    {
    43         return adoptRef(*new WebEditCommandProxy(commandID, editAction, page));
     45        return adoptRef(*new WebEditCommandProxy(commandID, label, page));
    4446    }
    4547    ~WebEditCommandProxy();
    4648
    4749    WebUndoStepID commandID() const { return m_commandID; }
    48     WebCore::EditAction editAction() const { return m_editAction; }
     50    String label() const { return m_label; }
    4951
    50     void invalidate() { m_page = 0; }
     52    void invalidate() { m_page.clear(); }
    5153
    5254    void unapply();
    … …  
    5456
    5557private:
    56     WebEditCommandProxy(WebUndoStepID commandID, WebCore::EditAction, WebPageProxy*);
     58    WebEditCommandProxy(WebUndoStepID commandID, const String& label, WebPageProxy&);
    5759
    5860    WebUndoStepID m_commandID;
    59     WebCore::EditAction m_editAction;
    60     WebPageProxy* m_page;
     61    String m_label;
     62    WeakPtr<WebPageProxy> m_page;
    6163};
    6264

  • trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

    r240243 r240342  
    54405440// Undo management
    54415441
    5442 void WebPageProxy::registerEditCommandForUndo(WebUndoStepID commandID, uint32_t editAction)
    5443 {
    5444     registerEditCommand(WebEditCommandProxy::create(commandID, static_cast<EditAction>(editAction), this), UndoOrRedo::Undo);
     5442void WebPageProxy::registerEditCommandForUndo(WebUndoStepID commandID, const String& label)
     5443{
     5444    registerEditCommand(WebEditCommandProxy::create(commandID, label, *this), UndoOrRedo::Undo);
    54455445}
    54465446   
    … …  
    66536653    m_loadDependentStringCallbackIDs.clear();
    66546654
    6655     auto editCommandVector = copyToVector(m_editCommandSet);
    6656     m_editCommandSet.clear();
    6657 
    6658     for (auto& editCommand : editCommandVector)
     6655    for (auto& editCommand : std::exchange(m_editCommandSet, { }))
    66596656        editCommand->invalidate();
    66606657

  • trunk/Source/WebKit/UIProcess/WebPageProxy.h

    r240243 r240342  
    16451645
    16461646    // Undo management
    1647     void registerEditCommandForUndo(WebUndoStepID commandID, uint32_t editAction);
     1647    void registerEditCommandForUndo(WebUndoStepID commandID, const String& label);
    16481648    void registerInsertionUndoGrouping();
    16491649    void clearAllEditCommands();

  • trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in

    r240199 r240342  
    234234
    235235    # Undo/Redo messages
    236     RegisterEditCommandForUndo(uint64_t commandID, uint32_t editAction)
     236    RegisterEditCommandForUndo(uint64_t commandID, String label)
    237237    ClearAllEditCommands()
    238238    RegisterInsertionUndoGrouping()

  • trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.mm

    r240199 r240342  
    268268void PageClientImpl::registerEditCommand(Ref<WebEditCommandProxy>&& command, UndoOrRedo undoOrRedo)
    269269{
    270     auto actionName = WebCore::nameForUndoRedo(command->editAction());
     270    auto actionName = command->label();
    271271    auto commandObjC = adoptNS([[WKEditCommand alloc] initWithWebEditCommandProxy:WTFMove(command)]);
    272272   

  • trunk/Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.cpp

    r239589 r240342  
    312312    auto webStep = WebUndoStep::create(step);
    313313    auto stepID = webStep->stepID();
    314     auto editAction = static_cast<uint32_t>(webStep->step().editingAction());
    315314
    316315    m_page->addWebUndoStep(stepID, WTFMove(webStep));
    317     m_page->send(Messages::WebPageProxy::RegisterEditCommandForUndo(stepID, editAction), m_page->pageID(), IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply);
     316    m_page->send(Messages::WebPageProxy::RegisterEditCommandForUndo(stepID, step.label()), m_page->pageID(), IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply);
    318317}
    319318

  • trunk/Source/WebKitLegacy/mac/ChangeLog

    r240292 r240342  
     12019-01-23  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        Introduce UndoStep::label() and adopt it in WebKitLegacy and WebKit
     4        https://bugs.webkit.org/show_bug.cgi?id=193706
     5        <rdar://problem/44807048>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        Use UndoStep::label().
     10
     11        * WebCoreSupport/WebEditorClient.h:
     12        * WebCoreSupport/WebEditorClient.mm:
     13        (-[WebUndoStep initWithUndoStep:]):
     14        (+[WebUndoStep stepWithUndoStep:]):
     15        (WebEditorClient::registerUndoOrRedoStep):
     16        (WebEditorClient::registerUndoStep):
     17        (WebEditorClient::registerRedoStep):
     18
    1192019-01-22  Alex Christensen  <achristensen@webkit.org>
    220

  • trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebEditorClient.mm

    r240181 r240342  
    126126}
    127127
    128 + (WebUndoStep *)stepWithUndoStep:(UndoStep&)step;
     128+ (WebUndoStep *)stepWithUndoStep:(Ref<UndoStep>&&)step;
    129129- (UndoStep&)step;
    130130
    … …  
    142142}
    143143
    144 - (id)initWithUndoStep:(UndoStep&)step
     144- (id)initWithUndoStep:(Ref<UndoStep>&&)step
    145145{
    146146    self = [super init];
    147147    if (!self)
    148148        return nil;
    149     m_step = &step;
     149    m_step = WTFMove(step);
    150150    return self;
    151151}
    … …  
    159159}
    160160
    161 + (WebUndoStep *)stepWithUndoStep:(UndoStep&)step
    162 {
    163     return [[[WebUndoStep alloc] initWithUndoStep:step] autorelease];
     161+ (WebUndoStep *)stepWithUndoStep:(Ref<UndoStep>&&)step
     162{
     163    return [[[WebUndoStep alloc] initWithUndoStep:WTFMove(step)] autorelease];
    164164}
    165165
    … …  
    602602#endif
    603603
    604     NSString *actionName = WebCore::nameForUndoRedo(step.editingAction());
    605     WebUndoStep *webEntry = [WebUndoStep stepWithUndoStep:step];
    606     [undoManager registerUndoWithTarget:m_undoTarget.get() selector:(isRedo ? @selector(redoEditing:) : @selector(undoEditing:)) object:webEntry];
     604    NSString *actionName = step.label();
     605    [undoManager registerUndoWithTarget:m_undoTarget.get() selector:(isRedo ? @selector(redoEditing:) : @selector(undoEditing:)) object:[WebUndoStep stepWithUndoStep:step]];
    607606    if (actionName)
    608607        [undoManager setActionName:actionName];
    … …  
    630629}
    631630
    632 void WebEditorClient::registerUndoStep(UndoStep& cmd)
    633 {
    634     registerUndoOrRedoStep(cmd, false);
    635 }
    636 
    637 void WebEditorClient::registerRedoStep(UndoStep& cmd)
    638 {
    639     registerUndoOrRedoStep(cmd, true);
     631void WebEditorClient::registerUndoStep(UndoStep& command)
     632{
     633    registerUndoOrRedoStep(command, false);
     634}
     635
     636void WebEditorClient::registerRedoStep(UndoStep& command)
     637{
     638    registerUndoOrRedoStep(command, true);
    640639}
    641640
Note: See TracChangeset for help on using the changeset viewer.