Removed FrameHost::settings()

Removed FrameHost::settings()

Removed the FrameHost::settings() method, and redirected calls to
Page::settings() using FrameHost's Page reference, or directly on the
Page object if it is available.

This is part of the work to merge FrameHost back into Page.

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

[sashab, 2017-02-23 05:01:10]

sashab@chromium.org changed reviewers:
+ dcheng@chromium.org, slangley@chromium.org

[sashab, 2017-02-23 05:01:11]

slangley ptal; Question for dcheng

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

https://codereview.chromium.org/2709263003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/page/Page.h:177: const Settings& settings() const
{ return *m_settings; }
dcheng - I copied this method style from FrameHost, but not sure if this is what
we want. Want me to change it back to  Settings& settings() const?

[sashab, 2017-02-23 05:03:11]

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

[commit-bot: I haz the power, 2017-02-23 05:03:32]

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

[sashab, 2017-02-23 05:17:11]

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

[commit-bot: I haz the power, 2017-02-23 05:17:27]

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

[sashab, 2017-02-23 05:26:02]

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

[dcheng, 2017-02-23 05:26:10]

https://codereview.chromium.org/2709263003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/MediaValues.cpp (right):

https://codereview.chromium.org/2709263003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/MediaValues.cpp:92: return
frame->host()->page().settings().getDefaultFontSize();
Here and elsewhere, host()->page() should just be page() I think (I assume we're
getting rid of FrameHost)

[commit-bot: I haz the power, 2017-02-23 05:26:16]

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

[sashab, 2017-02-23 05:26:32]

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

[commit-bot: I haz the power, 2017-02-23 05:26:52]

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-02-23 06:57:48]

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

[commit-bot: I haz the power, 2017-02-23 06:57:48]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-02-23 09:02:22]

On 2017/02/23 05:26:10, dcheng wrote:
>
https://codereview.chromium.org/2709263003/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/css/MediaValues.cpp (right):
> 
>
https://codereview.chromium.org/2709263003/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/css/MediaValues.cpp:92: return
> frame->host()->page().settings().getDefaultFontSize();
> Here and elsewhere, host()->page() should just be page() I think (I assume
we're
> getting rid of FrameHost)

+1

[sashab, 2017-02-24 01:09:36]

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

[commit-bot: I haz the power, 2017-02-24 01:10:31]

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

[sashab, 2017-02-24 01:13:17]

Description was changed from

==========
Removed FrameHost::settings()

Removed the FrameHost::settings() method, and redirected calls to
Page::settings() using FrameHost's Page reference.

This is part of the work to merge FrameHost back into Page.

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

to

==========
Removed FrameHost::settings()

Removed the FrameHost::settings() method, and redirected calls to
Page::settings() using FrameHost's Page reference, or directly on the
Page object if it is available.

This is part of the work to merge FrameHost back into Page.

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

[sashab, 2017-02-24 01:14:47]

Thanks dcheng & haraken! Great idea. Updated the CL and I think I got
everywhere, slangley@ PTAL.

[slangley, 2017-02-24 01:53:18]

lgtm

[dcheng, 2017-02-24 02:01:02]

lgtm

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

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

[commit-bot: I haz the power, 2017-02-24 02:43:53]

Dry run: This issue passed the CQ dry run.

[sashab, 2017-02-24 03:02:16]

The CQ bit was checked by sashab@chromium.org

[commit-bot: I haz the power, 2017-02-24 03:02:50]

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-02-24 03:13:41]

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

[commit-bot: I haz the power, 2017-02-24 03:13:42]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[sashab, 2017-02-24 04:28:00]

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

[sashab, 2017-02-24 04:28:00]

+haraken who is modules/ OWNER

[haraken, 2017-02-24 04:37:18]

LGTM

https://codereview.chromium.org/2709263003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp (right):

https://codereview.chromium.org/2709263003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp:950: FrameHost* host =
frame()->host();

Can you remove this check since we're deprecating the FrameHost?

Page* page = frame()->page();
if (!page)
  return FloatSize();

[sashab, 2017-02-24 04:41:53]

https://codereview.chromium.org/2709263003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp (right):

https://codereview.chromium.org/2709263003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp:950: FrameHost* host =
frame()->host();
On 2017/02/24 at 04:37:18, haraken wrote:
> Can you remove this check since we're deprecating the FrameHost?
> 
> Page* page = frame()->page();
> if (!page)
>   return FloatSize();

I need this here because host->visualViewport is used below.

But sure, swapped the frame and page so the page is the one in the if-statement.

[sashab, 2017-02-24 04:41:57]

The CQ bit was checked by sashab@chromium.org

[sashab, 2017-02-24 04:41:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
slangley@chromium.org, haraken@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2709263003/#ps100001
(title: "Switched if statement to use page")

[commit-bot: I haz the power, 2017-02-24 04:42:17]

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-02-24 06:28:34]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1487911317631170,
"parent_rev": "5c10c52ba10b124c002ab47a7165d983d9869e51", "commit_rev":
"616d2f430dc084729f71b275408df8ae9555f629"}

[commit-bot: I haz the power, 2017-02-24 06:29:14]

Description was changed from

==========
Removed FrameHost::settings()

Removed the FrameHost::settings() method, and redirected calls to
Page::settings() using FrameHost's Page reference, or directly on the
Page object if it is available.

This is part of the work to merge FrameHost back into Page.

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

to

==========
Removed FrameHost::settings()

Removed the FrameHost::settings() method, and redirected calls to
Page::settings() using FrameHost's Page reference, or directly on the
Page object if it is available.

This is part of the work to merge FrameHost back into Page.

BUG=691794

Review-Url: https://codereview.chromium.org/2709263003
Cr-Commit-Position: refs/heads/master@{#452774}
Committed:
https://chromium.googlesource.com/chromium/src/+/616d2f430dc084729f71b275408d...
==========

[commit-bot: I haz the power, 2017-02-24 06:29:15]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/616d2f430dc084729f71b275408d...