Bug 198395

Summary:

Web Inspector: remove RemoteObject.prototype.getPropertyDescriptorsAsObject

Product:

WebKit

Reporter:

Devin Rousso <hi>

Component:

Web Inspector

Assignee:

Devin Rousso <hi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, ews-watchlist, hi, inspector-bugzilla-changes, mattbaker, rniwa, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Archive of layout-test-results from ews102 for mac-highsierra	none
Archive of layout-test-results from ews105 for mac-highsierra-wk2	none
Archive of layout-test-results from ews112 for mac-highsierra	none
Patch	none

Devin Rousso

Reported 2019-05-30 15:55:40 PDT

When calling `remoteObject.getPropertyDescriptorsAsObject`, if one of the returned property descriptors is named `__proto__`, it will replace the `__proto__` of the plain object created for that function with the `WI.PropertyDescriptor`, altering the prototype chain. This is not a good thing. Since this function is only used in a few places, we should remove it entirely.

Attachments
Patch (7.14 KB, patch) 2019-05-30 19:40 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews102 for mac-highsierra (3.20 MB, application/zip) 2019-05-30 20:25 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews105 for mac-highsierra-wk2 (2.91 MB, application/zip) 2019-05-30 20:51 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews112 for mac-highsierra (2.88 MB, application/zip) 2019-05-30 21:26 PDT, EWS Watchlist	no flags	Details
Patch (7.15 KB, patch) 2019-05-30 21:46 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Devin Rousso

Comment 1 2019-05-30 19:40:28 PDT

Created attachment 371022 [details] Patch

EWS Watchlist

Comment 2 2019-05-30 20:25:52 PDT Comment hidden (obsolete)

Comment on attachment 371022 [details] Patch Attachment 371022 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/12335099 New failing tests: inspector/audit/data-domNodes.html inspector/audit/data-domAttributes.html inspector/audit/data-errors.html

EWS Watchlist

Comment 3 2019-05-30 20:25:53 PDT Comment hidden (obsolete)

Created attachment 371025 [details] Archive of layout-test-results from ews102 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-highsierra Platform: Mac OS X 10.13.6

EWS Watchlist

Comment 4 2019-05-30 20:51:55 PDT Comment hidden (obsolete)

Comment on attachment 371022 [details] Patch Attachment 371022 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/12335267 New failing tests: inspector/audit/data-domNodes.html inspector/audit/data-domAttributes.html inspector/audit/data-errors.html

EWS Watchlist

Comment 5 2019-05-30 20:51:57 PDT Comment hidden (obsolete)

Created attachment 371026 [details] Archive of layout-test-results from ews105 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6

EWS Watchlist

Comment 6 2019-05-30 21:26:40 PDT Comment hidden (obsolete)

Comment on attachment 371022 [details] Patch Attachment 371022 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12335345 New failing tests: inspector/audit/data-domNodes.html inspector/audit/data-domAttributes.html inspector/audit/data-errors.html

EWS Watchlist

Comment 7 2019-05-30 21:26:42 PDT Comment hidden (obsolete)

Created attachment 371030 [details] Archive of layout-test-results from ews112 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-highsierra Platform: Mac OS X 10.13.6

Devin Rousso

Comment 8 2019-05-30 21:46:30 PDT

Created attachment 371033 [details] Patch Integer vs String comparison -.-

Matt Baker

Comment 9 2019-06-03 13:18:49 PDT

Comment on attachment 371033 [details] Patch r=me, nice fix. Since ErrorObjectView.prototype.update is only called internally, and isn't an override, let's make it private or even inline it inside ErrorObjectView.prototype.expand, which is the only call site.

Devin Rousso

Comment 10 2019-06-03 13:29:02 PDT

(In reply to Matt Baker from comment #9) > Since ErrorObjectView.prototype.update is only called internally, and isn't an override, let's make it private or even inline it inside ErrorObjectView.prototype.expand, which is the only call site. I'd rather not make a change like this as it's frankly unnecessary for this patch, and could be more prone to issues than what I'd tested. Furthermore, I'm actually a fan of the idea of modularizing functionality, even if it's only used once, as it makes code easier to read and more future-capable.

WebKit Commit Bot

Comment 11 2019-06-03 13:58:50 PDT

Comment on attachment 371033 [details] Patch Clearing flags on attachment: 371033 Committed r246046: <https://trac.webkit.org/changeset/246046>

WebKit Commit Bot

Comment 12 2019-06-03 13:58:52 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 13 2019-06-03 13:59:20 PDT

<rdar://problem/51366626>

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