Bug 190341

Summary:

Have DOMWindowProperty get is frame from its associated DOMWindow

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

WebCore Misc.

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, commit-queue, darin, ggaren, rniwa, sam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

Chris Dumez

Reported 2018-10-07 15:10:53 PDT

Have DOMWindowProperty get is frame from its associated DOMWindow, instead of having its own m_frame that can potentially get out-of-sync.

Attachments
Patch (107.13 KB, patch) 2018-10-07 15:12 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (109.18 KB, patch) 2018-10-07 15:53 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Chris Dumez

Comment 1 2018-10-07 15:12:32 PDT

Created attachment 351745 [details] Patch

Chris Dumez

Comment 2 2018-10-07 15:53:33 PDT

Created attachment 351747 [details] Patch

Alex Christensen

Comment 3 2018-10-08 09:40:13 PDT

Comment on attachment 351747 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351747&action=review > Source/WebCore/page/DOMWindow.cpp:686 > -Screen* DOMWindow::screen() const > +Screen* DOMWindow::screen() Does this change really introduce mutating the DOMWindow when we access its screen? Same for other const removals. I see no good reason for them.

Chris Dumez

Comment 4 2018-10-08 09:41:27 PDT

(In reply to Alex Christensen from comment #3) > Comment on attachment 351747 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=351747&action=review > > > Source/WebCore/page/DOMWindow.cpp:686 > > -Screen* DOMWindow::screen() const > > +Screen* DOMWindow::screen() > > Does this change really introduce mutating the DOMWindow when we access its > screen? Same for other const removals. I see no good reason for them. I needed to drop the const for correctness because the DOMWindow passes itself to the DOMWindowProperties when constructing them as a DOMWindow&.

Chris Dumez

Comment 5 2018-10-08 09:55:31 PDT

(In reply to Chris Dumez from comment #4) > (In reply to Alex Christensen from comment #3) > > Comment on attachment 351747 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=351747&action=review > > > > > Source/WebCore/page/DOMWindow.cpp:686 > > > -Screen* DOMWindow::screen() const > > > +Screen* DOMWindow::screen() > > > > Does this change really introduce mutating the DOMWindow when we access its > > screen? Same for other const removals. I see no good reason for them. > > I needed to drop the const for correctness because the DOMWindow passes > itself to the DOMWindowProperties when constructing them as a DOMWindow&. The only reason they were const before is because the DOMWindow was passing its frame to the DOMWindowProperties when constructing them and the frame() getter is marked const.

Chris Dumez

Comment 6 2018-10-08 10:12:45 PDT

Comment on attachment 351747 [details] Patch Clearing flags on attachment: 351747 Committed r236917: <https://trac.webkit.org/changeset/236917>

Chris Dumez

Comment 7 2018-10-08 10:12:47 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 8 2018-10-08 10:14:10 PDT

<rdar://problem/45091778>

Note You need to log in before you can comment on or make changes to this bug.