Move FrameHost::m_subframeCount to Page

Move FrameHost::m_subframeCount to Page

Move the field FrameHost::m_subframeCount to Page, and update
FrameHost methods to directly call the equivalent Page method.

BUG=691794
Review-Url: https://codereview.chromium.org/2739033003
Cr-Commit-Position: refs/heads/master@{#456321}
Committed: https://chromium.googlesource.com/chromium/src/+/eb7ffe6561358b82498f885544ffdf50a5e46646

[sashab, 2017-03-09 04:25:46]

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

[commit-bot: I haz the power, 2017-03-09 04:25:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sashab, 2017-03-09 04:26:51]

sashab@chromium.org changed reviewers:
+ joelhockey@chromium.org

[commit-bot: I haz the power, 2017-03-09 06:16:08]

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

[commit-bot: I haz the power, 2017-03-09 06:16:14]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-03-09 08:13:01]

LGTM

[joelhockey, 2017-03-10 00:25:43]

lgtm

https://codereview.chromium.org/2739033003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/page/Page.h (right):

https://codereview.chromium.org/2739033003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/Page.h:241: DCHECK(m_subframeCount);
This is a question of chromium coding style rather than a comment.  Is this
better expressed as:

DCHECK_GT(m_subframeCount, 0);

Are either acceptable, or is there a specific preference for one over the other?

[joelhockey, 2017-03-10 00:25:45]

lgtm

[sashab, 2017-03-10 00:56:21]

https://codereview.chromium.org/2739033003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/page/Page.h (right):

https://codereview.chromium.org/2739033003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/Page.h:241: DCHECK(m_subframeCount);
On 2017/03/10 00:25:43, joelhockey wrote:
> This is a question of chromium coding style rather than a comment.  Is this
> better expressed as:
> 
> DCHECK_GT(m_subframeCount, 0);
> 
> Are either acceptable, or is there a specific preference for one over the
other?

Great catch! I'd definitely say DCHECK_GT is better; changed it, thanks.

[sashab, 2017-03-10 00:56:25]

The CQ bit was checked by sashab@chromium.org

[sashab, 2017-03-10 00:56:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, joelhockey@chromium.org
Link to the patchset: https://codereview.chromium.org/2739033003/#ps20001
(title: "Review feedback")

[commit-bot: I haz the power, 2017-03-10 00:56:53]

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-03-10 04:14:50]

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

[commit-bot: I haz the power, 2017-03-10 04:14:51]

Failed to apply patch for third_party/WebKit/Source/core/frame/FrameHost.cpp:
While running git apply --index -p1;
  error: patch failed: third_party/WebKit/Source/core/frame/FrameHost.cpp:53
  error: third_party/WebKit/Source/core/frame/FrameHost.cpp: patch does not
apply

Patch:       third_party/WebKit/Source/core/frame/FrameHost.cpp
Index: third_party/WebKit/Source/core/frame/FrameHost.cpp
diff --git a/third_party/WebKit/Source/core/frame/FrameHost.cpp
b/third_party/WebKit/Source/core/frame/FrameHost.cpp
index
ce9e70d2ad73f93b8c3a3f1146cca890ecf52cf7..b44daf931d2bfc47d0bda48f3b85c73f189567a2
100644
--- a/third_party/WebKit/Source/core/frame/FrameHost.cpp
+++ b/third_party/WebKit/Source/core/frame/FrameHost.cpp
@@ -53,8 +53,7 @@ FrameHost::FrameHost(Page& page)
                                        m_page->chromeClient())),
       m_consoleMessageStorage(new ConsoleMessageStorage()),
       m_globalRootScrollerController(
-          TopDocumentRootScrollerController::create(*this)),
-      m_subframeCount(0) {}
+          TopDocumentRootScrollerController::create(*this)) {}
 
 // Explicitly in the .cpp to avoid default constructor in .h
 FrameHost::~FrameHost() {}
@@ -120,23 +119,16 @@ DEFINE_TRACE(FrameHost) {
   visitor->trace(m_globalRootScrollerController);
 }
 
-#if DCHECK_IS_ON()
-void checkFrameCountConsistency(int expectedFrameCount, Frame* frame) {
-  ASSERT(expectedFrameCount >= 0);
-
-  int actualFrameCount = 0;
-  for (; frame; frame = frame->tree().traverseNext())
-    ++actualFrameCount;
+void FrameHost::incrementSubframeCount() {
+  page().incrementSubframeCount();
+}
 
-  ASSERT(expectedFrameCount == actualFrameCount);
+void FrameHost::decrementSubframeCount() {
+  page().decrementSubframeCount();
 }
-#endif
 
 int FrameHost::subframeCount() const {
-#if DCHECK_IS_ON()
-  checkFrameCountConsistency(m_subframeCount + 1, m_page->mainFrame());
-#endif
-  return m_subframeCount;
+  return page().subframeCount();
 }
 
 }  // namespace blink

[sashab, 2017-03-13 03:49:24]

The CQ bit was checked by sashab@chromium.org

[sashab, 2017-03-13 03:49:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, joelhockey@chromium.org
Link to the patchset: https://codereview.chromium.org/2739033003/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2017-03-13 03:49:39]

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-03-13 05:21:35]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489376964381190,
"parent_rev": "b5e82dce09c880f7aa92a7a2203dc4b0e64c0e84", "commit_rev":
"eb7ffe6561358b82498f885544ffdf50a5e46646"}

[commit-bot: I haz the power, 2017-03-13 05:22:53]

Description was changed from

==========
Move FrameHost::m_subframeCount to Page

Move the field FrameHost::m_subframeCount to Page, and update
FrameHost methods to directly call the equivalent Page method.

BUG=691794
==========

to

==========
Move FrameHost::m_subframeCount to Page

Move the field FrameHost::m_subframeCount to Page, and update
FrameHost methods to directly call the equivalent Page method.

BUG=691794

Review-Url: https://codereview.chromium.org/2739033003
Cr-Commit-Position: refs/heads/master@{#456321}
Committed:
https://chromium.googlesource.com/chromium/src/+/eb7ffe6561358b82498f885544ff...
==========

[commit-bot: I haz the power, 2017-03-13 05:22:54]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/eb7ffe6561358b82498f885544ff...

[yoichio, 2017-03-13 08:03:09]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2746873002/ by yoichio@chromium.org.

The reason for reverting is: Builders this step failed on:
WebKit Linux Trusty Leak [2 since first detection]
2 tests failed:
external/wpt/fullscreen/api/document-exit-fullscreen-nested-in-iframe-manual.html
( Code Search )
fast/forms/select/input-select-after-resize.html ( Code Search )

https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Linu....