EV indicator of the previous page persists on error page of an inexistent about: page (about:cofnig / about:lol)

RESOLVED FIXED in Firefox 64

Status

()

defect
P1
normal
RESOLVED FIXED
9 months ago
8 months ago

People

(Reporter: darkspirit, Assigned: keeler)

Tracking

({nightly-community, regression})

Trunk
mozilla64
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(geckoview62 unaffected, firefox-esr60 unaffected, firefox62 unaffected, firefox63 unaffected, firefox64 fixed)

Details

(Whiteboard: [psm-assigned])

Attachments

(1 attachment)

Debian Testing, KDE, Xorg

1. Open https://www.mozilla.org/de/firefox/nightly/firstrun/
2. Now visit about:lol (or about:cofnig) in the same tab
3. There is still an EV indicator: "Mozilla Corporation (US)"

Same regression range as in bug 1490982 comment 4:
mozregression --repo autoland --launch 7056aff16fb8124f1d6043538b9947017c4623e4 -a https://www.mozilla.org/de/firefox/nightly/firstrun/
last good

mozregression --repo autoland --launch 2f4adf14e6231a1668558dd78ecbe56a421591b6 -a https://www.mozilla.org/de/firefox/nightly/firstrun/
first bad
Flags: needinfo?(dkeeler)
Thanks for filing this. It looks like the failedChannel in docShell is null when nsSecureBrowserUIImpl gets called for this location change event (and the failed channel is what the docShell passes nsSecureBrowserUIImpl). Given that, we don't (can't) notify anything of the security change because that's how we would get our nsISecurityEventSink. From what I can tell, the original implementation would re-use the nsISecurityEventSink from the previous OnLocationChange notification, which seems like a bug. That said, I did a quick test with querying the original docShell to an nsISecurityEventSink and calling OnSecurityChange on that, and it seems to work. Maybe we can just use the original docShell in all cases?
Assignee: nobody → dkeeler
Flags: needinfo?(dkeeler)
Priority: -- → P1
Whiteboard: [psm-assigned]
When navigating to an about: page that doesn't exist (e.g.
"about:somethingthatdoesnotexist"), the docShell will call
nsSecureBrowserUIImpl::OnLocationChange with a request that is null.
Consequently, we can't use that to QueryInterface to a nsISecurityEventSink to
call OnSecurityChange. The previous implementation would use the prior
request's nsISecurityEventSink, which was a bug but luckily this produced the
correct behavior. Since the original docShell the nsSecureBrowserUIImpl was
initialized with is what needs to be notified, we can just QueryInterface that
to an nsISecurityEventSink and call OnSecurityChange directly instead.

Comment 3

9 months ago
Comment on attachment 9012276 [details]
bug 1493427 - use the docShell to call OnSecurityChange in nsSecureBrowserUIImpl::OnLocationChange r?Gijs

:Gijs (out Thu 27 - Sun 30 / 9;  he/him) has approved the revision.
Attachment #9012276 - Flags: review+

Comment 5

9 months ago
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/39762ef5d56e
use the docShell to call OnSecurityChange in nsSecureBrowserUIImpl::OnLocationChange r=Gijs

Comment 6

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/39762ef5d56e
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
https://hg.mozilla.org/mozilla-central/rev/7fa93a38b8bbf23952ee4598b40eedde8b51ffa9

Log:
https://treeherder.mozilla.org/logviewer.html#?job_id=202119350&repo=autoland

 INFO -  TEST-INFO | started process 7240 (C:\Users\task_1538102091\build\application\firefox\firefox -wait-for-browser -no-deelevate -profile c:\users\task_1538102091\appdata\local\temp\tmpe6wabr\profile http://localhost:50064/startup_test/tresize/addon/content/tresize-test.html)
04:19:15     INFO -  TEST-INFO | 7240: exit 1
04:19:22     INFO -  rmtree() failed for "('c:\\users\\task_1538102091\\appdata\\local\\temp\\tmpe6wabr',)". Reason: The process cannot access the file because it is being used by another process (13). Retrying...
04:19:22     INFO -  rmtree() failed for "('c:\\users\\task_1538102091\\appdata\\local\\temp\\tmpe6wabr',)". Reason: The process cannot access the file because it is being used by another process (13). Retrying...
04:19:22     INFO -  rmtree() failed for "('c:\\users\\task_1538102091\\appdata\\local\\temp\\tmpe6wabr',)". Reason: The process cannot access the file because it is being used by another process (13). Retrying...
04:19:22     INFO -  rmtree() failed for "('c:\\users\\task_1538102091\\appdata\\local\\temp\\tmpe6wabr',)". Reason: The process cannot access the file because it is being used by another process (13). Retrying...
04:19:22     INFO -  rmtree() failed for "('c:\\users\\task_1538102091\\appdata\\local\\temp\\tmpe6wabr',)". Reason: The process cannot access the file because it is being used by another process (13). Retrying...
04:19:22     INFO -  Exception while removing profile directory: c:\users\task_1538102091\appdata\local\temp\tmpe6wabr
04:19:22     INFO -  [Error 32] The process cannot access the file because it is being used by another process: 'c:\\users\\task_1538102091\\appdata\\local\\temp\\tmpe6wabr\\profile\\cert9.db'
04:19:22     INFO -  TEST-UNEXPECTED-ERROR | tresize | Could not find report in browser output: [('tsformat', ('__start_report', '__end_report')), ('tpformat', ('__start_tp_report', '__end_tp_report'))]
04:19:22    ERROR -  Traceback (most recent call last):
04:19:22     INFO -    File "C:\Users\task_1538102091\build\tests\talos\talos\run_tests.py", line 303, in run_tests
04:19:22     INFO -      talos_results.add(mytest.runTest(browser_config, test))
04:19:22     INFO -    File "C:\Users\task_1538102091\build\tests\talos\talos\ttest.py", line 63, in runTest
04:19:22     INFO -      return self._runTest(browser_config, test_config, setup)
04:19:22     INFO -    File "C:\Users\task_1538102091\build\tests\talos\talos\ttest.py", line 277, in _runTest
04:19:22     INFO -      else None)
04:19:22     INFO -    File "C:\Users\task_1538102091\build\tests\talos\talos\results.py", line 95, in add
04:19:22     INFO -      global_counters=self.global_counters
04:19:22     INFO -    File "C:\Users\task_1538102091\build\tests\talos\talos\results.py", line 326, in __init__
04:19:22     INFO -      self.parse()
04:19:22     INFO -    File "C:\Users\task_1538102091\build\tests\talos\talos\results.py", line 353, in parse
04:19:22     INFO -      % self.report_tokens)
04:19:22     INFO -    File "C:\Users\task_1538102091\build\tests\talos\talos\results.py", line 337, in error
04:19:22     INFO -      raise utils.TalosError(message)
04:19:22     INFO -  TalosError: Could not find report in browser output: [('tsformat', ('__start_report', '__end_report')), ('tpformat', ('__start_tp_report', '__end_tp_report'))]
04:19:22     INFO -  TEST-INFO took 105690ms
04:19:22     INFO -  SUITE-END | took 218s
04:22:20     INFO -  WARNING | IO Completion Port failed to signal process shutdown
04:22:20     INFO -  Parent process 1456 exited with children alive:
04:22:20     INFO -  PIDS: 8400, 2664, 9040, 4624, 2776, 6364
04:22:20     INFO -  Attempting to kill them, but no guarantee of success
04:22:20    ERROR - Return code: 2
04:22:20  WARNING - setting return code to 2
04:22:20    ERROR - # TBPL FAILURE #
04:22:20     INFO - Running post-action listener: _package_coverage_data
04:22:20     INFO - Running post-action listener: _resource_record_post_action
04:22:20     INFO - Running post-action listener: process_java_coverage_data
04:22:20     INFO - [mozharness: 2018-09-28 04:22:20.496000Z] Finished run-tests step (success)
04:22:20     INFO - Running post-run listener: _resource_record_post_run
04:22:20     INFO - Total resource usage - Wall time: 398s; CPU: 5.0%; Read bytes: 45225472; Write bytes: 267981824; Read time: 1; Write time: 4
04:22:20     INFO - TinderboxPrint: CPU usage<br/>4.9%
04:22:20     INFO - TinderboxPrint: I/O read bytes / time<br/>45,225,472 / 1
04:22:20     INFO - TinderboxPrint: I/O write bytes / time<br/>267,981,824 / 4
04:22:20     INFO - TinderboxPrint: CPU idle<br/>3,025.5 (95.0%)
04:22:20     INFO - TinderboxPrint: CPU system<br/>39.2 (1.2%)
04:22:20     INFO - TinderboxPrint: CPU user<br/>117.8 (3.7%)
04:22:20     INFO - install - Wall time: 2s; CPU: 13.0%; Read bytes: 4096; Write bytes: 9643520; Read time: 0; Write time: 0
04:22:20     INFO - setup-mitmproxy - Wall time: 0s; CPU: Can't collect data; Read bytes: 0; Write bytes: 0; Read time: 0; Write time: 0
04:22:20     INFO - run-tests - Wall time: 397s; CPU: 5.0%; Read bytes: 41814528; Write bytes: 256793088; Read time: 1; Write time: 4
04:22:20     INFO - Running post-run listener: copy_logs_to_upload_dir
04:22:20     INFO - Copying logs to upload dir...
04:22:20     INFO - mkdir: C:\Users\task_1538102091\build\upload\logs
04:22:20     INFO - Copying logs to upload dir...
04:22:20     INFO - Using _rmtree_windows ...
04:22:20     INFO - Running command: del /F /Q "C:\Users\task_1538102091\build\upload\logs\localconfig.json"
04:22:20     INFO - Return code: 0

The increase in frequency was introduced with push https://hg.mozilla.org/integration/autoland/rev/39762ef5d56e
Status: RESOLVED → REOPENED
Flags: needinfo?(dkeeler)
Resolution: FIXED → ---
Target Milestone: mozilla64 → ---
Depends on: 1494573
Flags: needinfo?(dkeeler)

Comment 9

9 months ago
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/84b4b9b7586a
use the docShell to call OnSecurityChange in nsSecureBrowserUIImpl::OnLocationChange r=Gijs

Comment 10

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/84b4b9b7586a
Status: REOPENED → RESOLVED
Closed: 9 months ago9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64

Updated

8 months ago
Depends on: 1497555
You need to log in before you can comment on or make changes to this bug.