Devirtualize the calls in nsSecureBrowserUIImpl::CheckForBlockedContent

RESOLVED FIXED in Firefox 66

Status

()

defect
P2
normal
RESOLVED FIXED
4 months ago
3 months ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

(Blocks 1 bug)

unspecified
mozilla66
Points:
---

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 months ago
See this profile: https://perfht.ml/2EZPaHi

Here is the code: <https://searchfox.org/mozilla-central/rev/3e7afaa5b57b3f8ed100cd1f13bb0a93b9b2cb99/security/manager/ssl/nsSecureBrowserUIImpl.cpp#114>

All of the nsIDocShell::GetFoo() calls here need to be devirtualized and ideally inlined for performance...
(Assignee)

Comment 1

4 months ago
Inlining these methods without including nsIDocument.h inside nsDocShell.h isn't possible, and that's not desirable, but since all of these methods merely forward the work to the docshell's document, CheckForBlockedContent() can be changed to do that directly.

Updated

4 months ago
Priority: -- → P2

Comment 3

3 months ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f6f97217b46c
Devirtualize and inline the calls in nsSecureBrowserUIImpl::CheckForBlockedContent(); r=baku

Comment 4

3 months ago
Backout by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/096e9d2bdcb1
Backed out changeset f6f97217b46c for build bustage at /security/manager/ssl/nsSecureBrowserUIImpl.cpp on a CLOSED TREE

Backed out changeset f6f97217b46c (bug 1517136) for build bustage at /security/manager/ssl/nsSecureBrowserUIImpl.cpp on a CLOSED TREE

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/096e9d2bdcb1d7d9ea029de2943f4b41c92cbd5b

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&revision=f6f97217b46cbcfecc46d844540ec94f862d9d34

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=220386761&repo=mozilla-inbound&lineNumber=37686

Log snippet:

[task 2019-01-07T19:33:45.963Z] 19:33:45 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/media/webrtc/trunk/gtest'
[task 2019-01-07T19:33:45.964Z] 19:33:45 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/media/webrtc/trunk/gtest'
[task 2019-01-07T19:33:45.965Z] 19:33:45 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/media/webrtc/trunk/gtest'
[task 2019-01-07T19:33:45.987Z] 19:33:45 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/security/manager/ssl'
[task 2019-01-07T19:33:46.005Z] 19:33:46 INFO - /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/clang++ --target=i686-linux-gnu -o Unified_cpp_security_manager_ssl2.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DDEBUG=1 -DSSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES=True -DNSS_ENABLE_ECC=True -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/security/manager/ssl -I/builds/worker/workspace/build/src/obj-firefox/security/manager/ssl -I/builds/worker/workspace/build/src/dom/base -I/builds/worker/workspace/build/src/dom/crypto -I/builds/worker/workspace/build/src/security/certverifier -I/builds/worker/workspace/build/src/obj-firefox/dist/public/nss -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -march=pentium-m -msse -msse2 -mfpmath=sse -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Os -fno-omit-frame-pointer -funwind-tables -Werror -I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include -Wextra -Wno-missing-field-initializers -Wno-unused-parameter -MD -MP -MF .deps/Unified_cpp_security_manager_ssl2.o.pp /builds/worker/workspace/build/src/obj-firefox/security/manager/ssl/Unified_cpp_security_manager_ssl2.cpp
[task 2019-01-07T19:33:46.006Z] 19:33:46 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/security/manager/ssl/Unified_cpp_security_manager_ssl2.cpp:137:
[task 2019-01-07T19:33:46.006Z] 19:33:46 ERROR - /builds/worker/workspace/build/src/security/manager/ssl/nsSecureBrowserUIImpl.cpp:110:12: error: use of undeclared identifier 'nsIDocument'
[task 2019-01-07T19:33:46.006Z] 19:33:46 INFO - nsCOMPtr<nsIDocument> doc = docShell->GetDocument();
[task 2019-01-07T19:33:46.006Z] 19:33:46 INFO - ^
[task 2019-01-07T19:33:46.006Z] 19:33:46 ERROR - /builds/worker/workspace/build/src/security/manager/ssl/nsSecureBrowserUIImpl.cpp:111:8: error: use of undeclared identifier 'doc'
[task 2019-01-07T19:33:46.006Z] 19:33:46 INFO - if (!doc) {
[task 2019-01-07T19:33:46.006Z] 19:33:46 INFO - ^
[task 2019-01-07T19:33:46.006Z] 19:33:46 ERROR - /builds/worker/workspace/build/src/security/manager/ssl/nsSecureBrowserUIImpl.cpp:121:9: error: use of undeclared identifier 'doc'; did you mean 'dom'?
[task 2019-01-07T19:33:46.006Z] 19:33:46 INFO - if (doc->GetHasMixedActiveContentLoaded()) {
[task 2019-01-07T19:33:46.007Z] 19:33:46 INFO - ^~~
[task 2019-01-07T19:33:46.007Z] 19:33:46 INFO - dom
[task 2019-01-07T19:33:46.007Z] 19:33:46 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/nsNetUtil.h:56:11: note: 'dom' declared here
[task 2019-01-07T19:33:46.007Z] 19:33:46 INFO - namespace dom {
[task 2019-01-07T19:33:46.007Z] 19:33:46 INFO - ^
[task 2019-01-07T19:33:46.007Z] 19:33:46 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/security/manager/ssl/Unified_cpp_security_manager_ssl2.cpp:137:
[task 2019-01-07T19:33:46.007Z] 19:33:46 ERROR - /builds/worker/workspace/build/src/security/manager/ssl/nsSecureBrowserUIImpl.cpp:121:9: error: unexpected namespace name 'dom': expected expression
[task 2019-01-07T19:33:46.008Z] 19:33:46 INFO - if (doc->GetHasMixedActiveContentLoaded()) {
[task 2019-01-07T19:33:46.008Z] 19:33:46 INFO - ^
[task 2019-01-07T19:33:46.008Z] 19:33:46 ERROR - /builds/worker/workspace/build/src/security/manager/ssl/nsSecureBrowserUIImpl.cpp:126:9: error: use of undeclared identifier 'doc'; did you mean 'dom'?
[task 2019-01-07T19:33:46.009Z] 19:33:46 INFO - if (doc->GetHasMixedDisplayContentLoaded()) {
[task 2019-01-07T19:33:46.010Z] 19:33:46 INFO - ^~~
[task 2019-01-07T19:33:46.012Z] 19:33:46 INFO - dom
[task 2019-01-07T19:33:46.012Z] 19:33:46 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/nsNetUtil.h:56:11: note: 'dom' declared here
[task 2019-01-07T19:33:46.012Z] 19:33:46 INFO - namespace dom {
[task 2019-01-07T19:33:46.013Z] 19:33:46 INFO - ^
[task 2019-01-07T19:33:46.013Z] 19:33:46 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/security/manager/ssl/Unified_cpp_security_manager_ssl2.cpp:137:
[task 2019-01-07T19:33:46.013Z] 19:33:46 ERROR - /builds/worker/workspace/build/src/security/manager/ssl/nsSecureBrowserUIImpl.cpp:126:9: error: unexpected namespace name 'dom': expected expression
[task 2019-01-07T19:33:46.013Z] 19:33:46 INFO - if (doc->GetHasMixedDisplayContentLoaded()) {

Flags: needinfo?(ehsan)
(Assignee)

Updated

3 months ago
Flags: needinfo?(ehsan)

Comment 6

3 months ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d801dfa24d7d
Devirtualize and inline the calls in nsSecureBrowserUIImpl::CheckForBlockedContent(); r=baku

Backed out changeset d801dfa24d7d (bug 1517136) for build bustages at /security/manager/ssl/nsSecureBrowserUIImpl.cpp on a CLOSED TREE

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/cb27ce235427c56a3bf3bfbcee25249d5fae5939

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&revision=d801dfa24d7d11e7dca3934ba52a3900c07af660

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=220413663&repo=mozilla-inbound&lineNumber=35777

Log snippet:

[task 2019-01-07T21:29:39.682Z] 21:29:39 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/gfx/angle/targets/translator'
[task 2019-01-07T21:29:39.682Z] 21:29:39 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/security/manager/ssl'
[task 2019-01-07T21:29:39.686Z] 21:29:39 INFO - /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/clang++ --target=i686-linux-gnu -o Unified_cpp_security_manager_ssl2.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DDEBUG=1 -DSSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES=True -DNSS_ENABLE_ECC=True -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/security/manager/ssl -I/builds/worker/workspace/build/src/obj-firefox/security/manager/ssl -I/builds/worker/workspace/build/src/dom/base -I/builds/worker/workspace/build/src/dom/crypto -I/builds/worker/workspace/build/src/security/certverifier -I/builds/worker/workspace/build/src/obj-firefox/dist/public/nss -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -march=pentium-m -msse -msse2 -mfpmath=sse -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Os -fno-omit-frame-pointer -funwind-tables -Werror -I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include -Wextra -Wno-missing-field-initializers -Wno-unused-parameter -MD -MP -MF .deps/Unified_cpp_security_manager_ssl2.o.pp /builds/worker/workspace/build/src/obj-firefox/security/manager/ssl/Unified_cpp_security_manager_ssl2.cpp
[task 2019-01-07T21:29:39.686Z] 21:29:39 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/security/manager/ssl/Unified_cpp_security_manager_ssl2.cpp:137:
[task 2019-01-07T21:29:39.686Z] 21:29:39 ERROR - /builds/worker/workspace/build/src/security/manager/ssl/nsSecureBrowserUIImpl.cpp:110:10: error: use of undeclared identifier 'Document'; did you mean 'nsGkAtoms::document'?
[task 2019-01-07T21:29:39.686Z] 21:29:39 INFO - RefPtr<Document> doc = docShell->GetDocument();
[task 2019-01-07T21:29:39.686Z] 21:29:39 INFO - ^~~~~~~~
[task 2019-01-07T21:29:39.686Z] 21:29:39 INFO - nsGkAtoms::document
[task 2019-01-07T21:29:39.686Z] 21:29:39 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/nsGkAtomList.h:357:9: note: 'nsGkAtoms::document' declared here
[task 2019-01-07T21:29:39.686Z] 21:29:39 INFO - GK_ATOM(document, "document", 0x80144fb8, true, nsStaticAtom, Atom)
[task 2019-01-07T21:29:39.686Z] 21:29:39 INFO - ^
[task 2019-01-07T21:29:39.686Z] 21:29:39 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/security/manager/ssl/Unified_cpp_security_manager_ssl2.cpp:137:
[task 2019-01-07T21:29:39.686Z] 21:29:39 ERROR - /builds/worker/workspace/build/src/security/manager/ssl/nsSecureBrowserUIImpl.cpp:110:10: error: template argument for template type parameter must be a type
[task 2019-01-07T21:29:39.686Z] 21:29:39 INFO - RefPtr<Document> doc = docShell->GetDocument();
[task 2019-01-07T21:29:39.686Z] 21:29:39 INFO - ^~~~~~~~
[task 2019-01-07T21:29:39.686Z] 21:29:39 INFO - /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_message_utils.h:24:20: note: template parameter is declared here
[task 2019-01-07T21:29:39.686Z] 21:29:39 INFO - template <typename T>
[task 2019-01-07T21:29:39.686Z] 21:29:39 INFO - ^
[task 2019-01-07T21:29:39.687Z] 21:29:39 INFO - 2 errors generated.
[task 2019-01-07T21:29:39.687Z] 21:29:39 INFO - /builds/worker/workspace/build/src/config/rules.mk:1119: recipe for target 'Unified_cpp_security_manager_ssl2.o' failed
[task 2019-01-07T21:29:39.687Z] 21:29:39 ERROR - make[4]: *** [Unified_cpp_security_manager_ssl2.o] Error 1
[task 2019-01-07T21:29:39.687Z] 21:29:39 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/security/manager/ssl'
[task 2019-01-07T21:29:39.687Z] 21:29:39 INFO - /builds/worker/workspace/build/src/config/recurse.mk:74: recipe for target 'security/manager/ssl/target' failed
[task 2019-01-07T21:29:39.687Z] 21:29:39 ERROR - make[3]: *** [security/manager/ssl/target] Error 2
[task 2019-01-07T21:29:39.687Z] 21:29:39 INFO - make[3]: *** Waiting for unfinished jobs....
[task 2019-01-07T21:29:39.687Z] 21:29:39 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/gfx/angle/targets/translator'

Flags: needinfo?(ehsan)

Comment 8

3 months ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b153c2a6f40a
Devirtualize and inline the calls in nsSecureBrowserUIImpl::CheckForBlockedContent(); r=baku
(Assignee)

Updated

3 months ago
Flags: needinfo?(ehsan)

Comment 9

3 months ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Assignee: nobody → ehsan
https://hg.mozilla.org/projects/cedar/rev/b153c2a6f40af821f291c0b84bb5ae4a29c00967
Bug 1517136 - Devirtualize and inline the calls in nsSecureBrowserUIImpl::CheckForBlockedContent(); r=baku
You need to log in before you can comment on or make changes to this bug.