Finish removing FrameLoader::Clear()

Finish removing FrameLoader::Clear()

BUG=

Review-Url: https://codereview.chromium.org/2862293002
Cr-Commit-Position: refs/heads/master@{#474444}
Committed: https://chromium.googlesource.com/chromium/src/+/0cca324ec82d5b59ba8b917f810d652e010b4e72

[Nate Chapin, 2017-05-05 17:19:45]

The CQ bit was checked by japhet@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-05 17:20:33]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-05 20:29:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-05 20:29:25]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Nate Chapin, 2017-05-05 20:30:11]

The CQ bit was checked by japhet@chromium.org

[commit-bot: I haz the power, 2017-05-05 20:30:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-05 20:30:48]

This CL has an open dependency (Issue 2854853006 Patch 20001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[Nate Chapin, 2017-05-05 20:34:52]

The CQ bit was checked by japhet@chromium.org to run a CQ dry run

[Nate Chapin, 2017-05-05 20:34:55]

The CQ bit was unchecked by japhet@chromium.org

[Nate Chapin, 2017-05-05 20:34:58]

The CQ bit was unchecked by japhet@chromium.org

[Nate Chapin, 2017-05-08 17:17:53]

The CQ bit was checked by japhet@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-08 17:18:31]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-08 20:04:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-08 20:04:52]

Dry run: This issue passed the CQ dry run.

[Nate Chapin, 2017-05-16 23:30:42]

The CQ bit was checked by japhet@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-16 23:33:29]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Nate Chapin, 2017-05-17 18:18:58]

japhet@chromium.org changed reviewers:
+ dcheng@chromium.org

[Nate Chapin, 2017-05-17 18:18:59]

Followup to
https://chromium.googlesource.com/chromium/src/+/2e62c43c2024d820f36302576f89...

https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/Editor.cpp (left):

https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:1043:
GetFrame().GetInputMethodController().Clear();
This is also called during Document::Shutdown(), so it's redundant here.

https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/DocumentLoader.cpp (right):

https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/DocumentLoader.cpp:1056:
FrameLoaderStateMachine::kCommittedFirstRealLoad);
This feels like a slightly weird place for this, but I don't have a better
idea...

https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/FrameLoader.cpp (left):

https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/FrameLoader.cpp:368:
frame_->GetEditor().Clear();
Move this and the next line to LocalFrame::DocumentAttached(). I think this
means it'll get called for the initial empty document, but that should be
benign.

https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/FrameLoader.cpp:371:
frame_->View()->Clear();
We just got a new FrameView, no need to clear it.

https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/FrameLoader.cpp:374:
state_machine_.AdvanceTo(FrameLoaderStateMachine::kCommittedFirstRealLoad);
Moved to DocumentLoader.

https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/testing/Internals.cpp (left):

https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/testing/Internals.cpp:262:
page->DeprecatedLocalMainFrame()->View()->Clear();
I can't find any evidence tests need this?

[dcheng, 2017-05-22 21:23:26]

LGTM with a question

(Sorry for missing this; I was in Sydney. Feel free to ping earlier next time)

https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/Editor.cpp (left):

https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:1043:
GetFrame().GetInputMethodController().Clear();
On 2017/05/17 18:18:59, Nate Chapin wrote:
> This is also called during Document::Shutdown(), so it's redundant here.

Due to InputMethodController::ContextDestroyed()?

I'd say that it's a bit unclear how this whole class works together, but I think
this doesn't make it any worse.

https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/DocumentLoader.cpp (right):

https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/DocumentLoader.cpp:1056:
FrameLoaderStateMachine::kCommittedFirstRealLoad);
On 2017/05/17 18:18:59, Nate Chapin wrote:
> This feels like a slightly weird place for this, but I don't have a better
> idea...

Hmm... I think this is OK.

[Nate Chapin, 2017-05-22 21:43:47]

On 2017/05/22 21:23:26, dcheng wrote:
> LGTM with a question
> 
> (Sorry for missing this; I was in Sydney. Feel free to ping earlier next time)
> 
>
https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/editing/Editor.cpp (left):
> 
>
https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/editing/Editor.cpp:1043:
> GetFrame().GetInputMethodController().Clear();
> On 2017/05/17 18:18:59, Nate Chapin wrote:
> > This is also called during Document::Shutdown(), so it's redundant here.
> 
> Due to InputMethodController::ContextDestroyed()?

Right.

> 
> I'd say that it's a bit unclear how this whole class works together, but I
think
> this doesn't make it any worse.

Yeah... :/

> 
>
https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/loader/DocumentLoader.cpp (right):
> 
>
https://codereview.chromium.org/2862293002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/loader/DocumentLoader.cpp:1056:
> FrameLoaderStateMachine::kCommittedFirstRealLoad);
> On 2017/05/17 18:18:59, Nate Chapin wrote:
> > This feels like a slightly weird place for this, but I don't have a better
> > idea...
> 
> Hmm... I think this is OK.

[Nate Chapin, 2017-05-22 21:43:54]

The CQ bit was checked by japhet@chromium.org

[commit-bot: I haz the power, 2017-05-22 21:44:16]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-22 21:46:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-22 21:47:00]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Nate Chapin, 2017-05-23 23:35:52]

The CQ bit was checked by japhet@chromium.org

[Nate Chapin, 2017-05-23 23:35:53]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2862293002/#ps80001
(title: "Rebase")

[commit-bot: I haz the power, 2017-05-23 23:40:19]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-24 02:49:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-24 02:49:52]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[dcheng, 2017-05-24 03:12:39]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2017-05-24 03:13:30]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-24 08:00:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-24 08:01:00]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Nate Chapin, 2017-05-24 18:01:49]

The CQ bit was checked by japhet@chromium.org

[commit-bot: I haz the power, 2017-05-24 18:03:00]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-24 22:19:45]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1495648909646120,
"parent_rev": "1f90c7dc31ae659ff560baebcabfe24c3e036312", "commit_rev":
"0cca324ec82d5b59ba8b917f810d652e010b4e72"}

[commit-bot: I haz the power, 2017-05-24 22:19:53]

Description was changed from

==========
Finish removing FrameLoader::Clear()

BUG=
==========

to

==========
Finish removing FrameLoader::Clear()

BUG=

Review-Url: https://codereview.chromium.org/2862293002
Cr-Commit-Position: refs/heads/master@{#474444}
Committed:
https://chromium.googlesource.com/chromium/src/+/0cca324ec82d5b59ba8b917f810d...
==========

[commit-bot: I haz the power, 2017-05-24 22:19:54]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/0cca324ec82d5b59ba8b917f810d...