Password Manager logs on IOS (implementation) |
|||||
Issue descriptionThere is chrome://password-manager-internals Password Manager logs page on all non-iOS platforms. Missing it on iOS makes it harder to reproduce bugs. This is tracking bug for implementing chrome://password-manager-internals in Chrome on iOS.
,
Oct 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/15fb1d7e0f94d7f4cfa3be036f615354000a51d6 commit 15fb1d7e0f94d7f4cfa3be036f615354000a51d6 Author: Vadym Doroshenko <dvadym@chromium.org> Date: Fri Oct 20 15:57:06 2017 Moving password-manager-internals resources to components/. This is the first CL for implementing password-manager-internals in Bling. In order to reuse resources on IOS they should be moved from chrome/browser/ to components/. Bug: 775894 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I4b9cc6096bba1ce215821be13f55ed8b0659fad9 Reviewed-on: https://chromium-review.googlesource.com/725709 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Vaclav Brozek <vabr@chromium.org> Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> Cr-Commit-Position: refs/heads/master@{#510451} [modify] https://crrev.com/15fb1d7e0f94d7f4cfa3be036f615354000a51d6/chrome/BUILD.gn [modify] https://crrev.com/15fb1d7e0f94d7f4cfa3be036f615354000a51d6/chrome/browser/resources/BUILD.gn [delete] https://crrev.com/4b9ab834cbdc1e6d4ec03a1f855dbba53f0a46f0/chrome/browser/resources/password_manager_internals/OWNERS [delete] https://crrev.com/4b9ab834cbdc1e6d4ec03a1f855dbba53f0a46f0/chrome/browser/resources/password_manager_internals_resources.grd [modify] https://crrev.com/15fb1d7e0f94d7f4cfa3be036f615354000a51d6/chrome/browser/ui/webui/password_manager_internals/password_manager_internals_ui.cc [modify] https://crrev.com/15fb1d7e0f94d7f4cfa3be036f615354000a51d6/chrome/chrome_paks.gni [add] https://crrev.com/15fb1d7e0f94d7f4cfa3be036f615354000a51d6/components/password_manager/core/browser/resources/OWNERS [rename] https://crrev.com/15fb1d7e0f94d7f4cfa3be036f615354000a51d6/components/password_manager/core/browser/resources/password_manager_internals.css [rename] https://crrev.com/15fb1d7e0f94d7f4cfa3be036f615354000a51d6/components/password_manager/core/browser/resources/password_manager_internals.html [rename] https://crrev.com/15fb1d7e0f94d7f4cfa3be036f615354000a51d6/components/password_manager/core/browser/resources/password_manager_internals.js [modify] https://crrev.com/15fb1d7e0f94d7f4cfa3be036f615354000a51d6/components/resources/components_resources.grd [add] https://crrev.com/15fb1d7e0f94d7f4cfa3be036f615354000a51d6/components/resources/password_manager_internals_resources.grdp [modify] https://crrev.com/15fb1d7e0f94d7f4cfa3be036f615354000a51d6/tools/gritsettings/resource_ids
,
Nov 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6378d1e2c278d121ecb822384ecede93c44b9050 commit 6378d1e2c278d121ecb822384ecede93c44b9050 Author: Vadym Doroshenko <dvadym@chromium.org> Date: Thu Nov 23 17:54:46 2017 [IOS Password Manager] password-manager-internals implementation. chrome://password-manager-internals is Password Manager logs, that's very convenient for debugging. This CL implements IOS WebUI for showing the chrome://password-manager-internals on IOS as on other platforms. Logs from browser part of PasswordManager are already showed. Logs from ios/chrome/browser part of PasswordManager will be implemented in the following CLs. Bug: 775894 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: If8c9bb20a8e1f4de678a4d628fe6de34b12c8770 Reviewed-on: https://chromium-review.googlesource.com/735479 Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> Reviewed-by: Vaclav Brozek <vabr@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Cr-Commit-Position: refs/heads/master@{#518978} [modify] https://crrev.com/6378d1e2c278d121ecb822384ecede93c44b9050/ios/chrome/browser/chrome_url_constants.cc [modify] https://crrev.com/6378d1e2c278d121ecb822384ecede93c44b9050/ios/chrome/browser/chrome_url_constants.h [modify] https://crrev.com/6378d1e2c278d121ecb822384ecede93c44b9050/ios/chrome/browser/passwords/BUILD.gn [modify] https://crrev.com/6378d1e2c278d121ecb822384ecede93c44b9050/ios/chrome/browser/passwords/ios_chrome_password_manager_client.h [modify] https://crrev.com/6378d1e2c278d121ecb822384ecede93c44b9050/ios/chrome/browser/passwords/ios_chrome_password_manager_client.mm [add] https://crrev.com/6378d1e2c278d121ecb822384ecede93c44b9050/ios/chrome/browser/passwords/password_manager_internals_service_factory.cc [add] https://crrev.com/6378d1e2c278d121ecb822384ecede93c44b9050/ios/chrome/browser/passwords/password_manager_internals_service_factory.h [modify] https://crrev.com/6378d1e2c278d121ecb822384ecede93c44b9050/ios/chrome/browser/ui/webui/BUILD.gn [modify] https://crrev.com/6378d1e2c278d121ecb822384ecede93c44b9050/ios/chrome/browser/ui/webui/chrome_web_ui_ios_controller_factory.mm [add] https://crrev.com/6378d1e2c278d121ecb822384ecede93c44b9050/ios/chrome/browser/ui/webui/password_manager_internals_ui_ios.h [add] https://crrev.com/6378d1e2c278d121ecb822384ecede93c44b9050/ios/chrome/browser/ui/webui/password_manager_internals_ui_ios.mm
,
Nov 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d6f2633dfd4703120be124c706d751dcc2559dc2 commit d6f2633dfd4703120be124c706d751dcc2559dc2 Author: Elodie Banel <lod@chromium.org> Date: Fri Nov 24 14:01:16 2017 Revert "[IOS Password Manager] password-manager-internals implementation." This reverts commit 6378d1e2c278d121ecb822384ecede93c44b9050. Reason for revert: testChromeURLsLoadWithoutError is broken following this change. See https://bugs.chromium.org/p/chromium/issues/detail?id=788303 Original change's description: > [IOS Password Manager] password-manager-internals implementation. > > chrome://password-manager-internals is Password Manager logs, that's > very convenient for debugging. This CL implements IOS WebUI for showing > the chrome://password-manager-internals on IOS as on other platforms. > Logs from browser part of PasswordManager are already showed. Logs from > ios/chrome/browser part of PasswordManager will be implemented in the > following CLs. > > Bug: 775894 > Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs > Change-Id: If8c9bb20a8e1f4de678a4d628fe6de34b12c8770 > Reviewed-on: https://chromium-review.googlesource.com/735479 > Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> > Reviewed-by: Vaclav Brozek <vabr@chromium.org> > Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> > Cr-Commit-Position: refs/heads/master@{#518978} TBR=vabr@chromium.org,sdefresne@chromium.org,dvadym@chromium.org Change-Id: I3c34419fc6e8337a250bab5886a212c0766e75d4 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 775894 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Reviewed-on: https://chromium-review.googlesource.com/789151 Reviewed-by: Elodie Banel <lod@chromium.org> Commit-Queue: Elodie Banel <lod@chromium.org> Cr-Commit-Position: refs/heads/master@{#519100} [modify] https://crrev.com/d6f2633dfd4703120be124c706d751dcc2559dc2/ios/chrome/browser/chrome_url_constants.cc [modify] https://crrev.com/d6f2633dfd4703120be124c706d751dcc2559dc2/ios/chrome/browser/chrome_url_constants.h [modify] https://crrev.com/d6f2633dfd4703120be124c706d751dcc2559dc2/ios/chrome/browser/passwords/BUILD.gn [modify] https://crrev.com/d6f2633dfd4703120be124c706d751dcc2559dc2/ios/chrome/browser/passwords/ios_chrome_password_manager_client.h [modify] https://crrev.com/d6f2633dfd4703120be124c706d751dcc2559dc2/ios/chrome/browser/passwords/ios_chrome_password_manager_client.mm [delete] https://crrev.com/a2a21f5b5cd39cb4686249b3859fa897d756ceba/ios/chrome/browser/passwords/password_manager_internals_service_factory.cc [delete] https://crrev.com/a2a21f5b5cd39cb4686249b3859fa897d756ceba/ios/chrome/browser/passwords/password_manager_internals_service_factory.h [modify] https://crrev.com/d6f2633dfd4703120be124c706d751dcc2559dc2/ios/chrome/browser/ui/webui/BUILD.gn [modify] https://crrev.com/d6f2633dfd4703120be124c706d751dcc2559dc2/ios/chrome/browser/ui/webui/chrome_web_ui_ios_controller_factory.mm [delete] https://crrev.com/a2a21f5b5cd39cb4686249b3859fa897d756ceba/ios/chrome/browser/ui/webui/password_manager_internals_ui_ios.h [delete] https://crrev.com/a2a21f5b5cd39cb4686249b3859fa897d756ceba/ios/chrome/browser/ui/webui/password_manager_internals_ui_ios.mm
,
Nov 24 2017
The failure which lead to the revert above is that sometimes when Chrome tries to load the internals page, it crashes on seeing wrong data in one of the associated URL request response. The failure is interesting. I can reproduce it most of the time on iPhone 7 (simulator) in portrait mode. Trying the same on iPhone 8 Plus or in landscape mode did not result in a crash. Looking into URLFetcherStringWriter::Write, I keep seeing the data starting with the same non-sense-looking sequence of bytes like \213 \377, and with the same length of 627 bytes. I could not find some corruption in the //net logic around the buffers, and it agrees with this failure being exclusively related to the internals page. On the other hand, the internals page seems just a bunch of boilerplate, so not sure yet where the problem is.
,
Nov 24 2017
Still puzzled with the issue described in #5. I was looking at ios/chrome/browser/ui/webui/omaha_ui.* as a reference implementation and could not find substantial differences against https://crrev.com/c/735479. Also the recent addition of chrome://suggestions in https://crrev.com/c/574592 did not make me realise what was wrong with passwords internals.
,
Nov 27 2017
Seems that the response data always starts with \231\377 for other internals pages (e.g., about:omaha), but when that data arrives to ios/web/webui/url_fetcher_block_adapter.mm, the net::URLFetcher instance converts it to reasonable data in all cases except for about:password-manager-internals. Still not sure why, I'm looking further.
,
Nov 27 2017
The only difference wrt. about:omaha was the use of GZip. When I enabled GZip for password manager internals, the issue went away. The non-sensical strings would hint at some issues with forgotten GZip decoding, but I'm far from being sure what has been happening. I will try to get the internals page enabled with the GZip fix, because the sooner we have it, the better. There might still be going something fishy with the way sources are processed if the WebUI controller does not request the usage of GZip. I have currently not enough time to investigate that and don't expect having any this year, unless some expert wants to join in.
,
Nov 27 2017
Oops, I did not realise this bug tracks the whole internals page, not just the fix. Hence moving Vadym back to Owner :).
,
Nov 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/449b181eb8c52ac69969bfe0d285f28535e09b61 commit 449b181eb8c52ac69969bfe0d285f28535e09b61 Author: Vaclav Brozek <vabr@chromium.org> Date: Mon Nov 27 20:03:33 2017 Reland [IOS Password Manager] password-manager-internals implementation. This relands https://crrev.com/518978 and fixes some issues. Patch set 1 here is the original CL to be relanded. Following patch sets contain the following fixes: (1) Introduces source->UseGzip(); in PasswordManagerInternalsUIIOS source set up. Reasons are described in https://crbug.com/775894#c8. (2) Ensures that PasswordManagerInternalsUIIOS always removes itself from WebState observers. Previously, this might have been skipped if other errors with the logging service occurred. While a separate issue, it could have been masked by the other one, so it is not safe to land them separately. (3) Minor nits in build file. Original description follows: chrome://password-manager-internals is Password Manager logs, that's very convenient for debugging. This CL implements IOS WebUI for showing the chrome://password-manager-internals on IOS as on other platforms. Logs from browser part of PasswordManager are already showed. Logs from ios/chrome/browser part of PasswordManager will be implemented in the following CLs. Bug: 775894 Change-Id: Ic36a644a7240812d29d3aa599cde1fd3226d1ab0 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Reviewed-on: https://chromium-review.googlesource.com/735479 Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> Reviewed-by: Vaclav Brozek <vabr@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#518978} Reviewed-on: https://chromium-review.googlesource.com/790516 Reviewed-by: Eugene But <eugenebut@chromium.org> Reviewed-by: Vadym Doroshenko <dvadym@chromium.org> Cr-Commit-Position: refs/heads/master@{#519374} [modify] https://crrev.com/449b181eb8c52ac69969bfe0d285f28535e09b61/ios/chrome/browser/chrome_url_constants.cc [modify] https://crrev.com/449b181eb8c52ac69969bfe0d285f28535e09b61/ios/chrome/browser/chrome_url_constants.h [modify] https://crrev.com/449b181eb8c52ac69969bfe0d285f28535e09b61/ios/chrome/browser/passwords/BUILD.gn [modify] https://crrev.com/449b181eb8c52ac69969bfe0d285f28535e09b61/ios/chrome/browser/passwords/ios_chrome_password_manager_client.h [modify] https://crrev.com/449b181eb8c52ac69969bfe0d285f28535e09b61/ios/chrome/browser/passwords/ios_chrome_password_manager_client.mm [add] https://crrev.com/449b181eb8c52ac69969bfe0d285f28535e09b61/ios/chrome/browser/passwords/password_manager_internals_service_factory.cc [add] https://crrev.com/449b181eb8c52ac69969bfe0d285f28535e09b61/ios/chrome/browser/passwords/password_manager_internals_service_factory.h [modify] https://crrev.com/449b181eb8c52ac69969bfe0d285f28535e09b61/ios/chrome/browser/ui/webui/BUILD.gn [modify] https://crrev.com/449b181eb8c52ac69969bfe0d285f28535e09b61/ios/chrome/browser/ui/webui/chrome_web_ui_ios_controller_factory.mm [add] https://crrev.com/449b181eb8c52ac69969bfe0d285f28535e09b61/ios/chrome/browser/ui/webui/password_manager_internals_ui_ios.h [add] https://crrev.com/449b181eb8c52ac69969bfe0d285f28535e09b61/ios/chrome/browser/ui/webui/password_manager_internals_ui_ios.mm
,
Nov 29
vabr going hobby only -> reducing involvement. Please contact me directly in urgent matters. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by vabr@chromium.org
, Oct 19 2017