Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2512473004:
cros: Enable WebUILoginView reuse. (Closed)
|
Created:
4 years, 1 month ago by jdufault Modified:
3 years, 11 months ago CC:
chromium-reviews, alemate+watch_chromium.org, sadrul, achuith+watch_chromium.org, arv+watch_chromium.org, oshima+watch_chromium.org, kalyank, davemoore+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptioncros: Enable WebUILoginView reuse.
This CL also includes a few fixes for loading a WebUILoginView instance by itself without associated helper classes also being initialized.
There are three child CLs associated with this one:
- [1] contains fixes on the HTML side.
- [2] makes the lock screen show up correctly in the task manager.
- [3] preloads and reuses the WebUIScreenLocker instance (derived from WebUILoginView).
1: https://codereview.chromium.org/2555453003/
2: https://codereview.chromium.org/2550263002/
3: https://codereview.chromium.org/2553863002/
There is additional context at go/cros-lock-screen-optimization.
BUG=669638
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/3a6fa9bb8a91ec674c9a62d4fc8f4765b41b10b6
Cr-Commit-Position: refs/heads/master@{#437942}
Patch Set 1 : Initial upload #
Total comments: 30
Patch Set 2 : Address comments #
Total comments: 2
Patch Set 3 : Address comments #Dependent Patchsets: Messages
Total messages: 59 (49 generated)
Description was changed from ========== IGNORE/NOTREADY: Share lock webui across lock sessions. BUG= ========== to ========== IGNORE/NOTREADY: Share lock webui across lock sessions. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by jdufault@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by jdufault@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by jdufault@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by jdufault@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by jdufault@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by jdufault@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
Patchset #1 (id:60001) has been deleted
Description was changed from ========== IGNORE/NOTREADY: Share lock webui across lock sessions. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== IGNORE/NOTREADY: Share lock webui across lock sessions. BUG=669638 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== IGNORE/NOTREADY: Share lock webui across lock sessions. BUG=669638 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== cros: Enable WebUILoginView reuse. This CL also includes a few fixes for loading a WebUILoginView instance by itself without associated helper classes also being initialized. There are three child CLs associated with this one: - [1] contains fixes on the HTML side. - [2] makes the lock screen show up correctly in the task manager. - [3] preloads and reuses the WebUIScreenLocker instance (derived from WebUILoginView). 1: https://codereview.chromium.org/2555453003/ 2: https://codereview.chromium.org/2550263002/ 3: https://codereview.chromium.org/2553863002/ BUG=669638 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== cros: Enable WebUILoginView reuse. This CL also includes a few fixes for loading a WebUILoginView instance by itself without associated helper classes also being initialized. There are three child CLs associated with this one: - [1] contains fixes on the HTML side. - [2] makes the lock screen show up correctly in the task manager. - [3] preloads and reuses the WebUIScreenLocker instance (derived from WebUILoginView). 1: https://codereview.chromium.org/2555453003/ 2: https://codereview.chromium.org/2550263002/ 3: https://codereview.chromium.org/2553863002/ BUG=669638 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== cros: Enable WebUILoginView reuse. This CL also includes a few fixes for loading a WebUILoginView instance by itself without associated helper classes also being initialized. There are three child CLs associated with this one: - [1] contains fixes on the HTML side. - [2] makes the lock screen show up correctly in the task manager. - [3] preloads and reuses the WebUIScreenLocker instance (derived from WebUILoginView). 1: https://codereview.chromium.org/2555453003/ 2: https://codereview.chromium.org/2550263002/ 3: https://codereview.chromium.org/2553863002/ BUG=669638 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by jdufault@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #1 (id:80001) has been deleted
jdufault@chromium.org changed reviewers: + achuith@chromium.org, alemate@chromium.org
achuith@, alemate@, PTAL. This is the root CL that you should look at first.
jdufault@chromium.org changed reviewers: + xiyuan@chromium.org
Description was changed from ========== cros: Enable WebUILoginView reuse. This CL also includes a few fixes for loading a WebUILoginView instance by itself without associated helper classes also being initialized. There are three child CLs associated with this one: - [1] contains fixes on the HTML side. - [2] makes the lock screen show up correctly in the task manager. - [3] preloads and reuses the WebUIScreenLocker instance (derived from WebUILoginView). 1: https://codereview.chromium.org/2555453003/ 2: https://codereview.chromium.org/2550263002/ 3: https://codereview.chromium.org/2553863002/ BUG=669638 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== cros: Enable WebUILoginView reuse. This CL also includes a few fixes for loading a WebUILoginView instance by itself without associated helper classes also being initialized. There are three child CLs associated with this one: - [1] contains fixes on the HTML side. - [2] makes the lock screen show up correctly in the task manager. - [3] preloads and reuses the WebUIScreenLocker instance (derived from WebUILoginView). 1: https://codereview.chromium.org/2555453003/ 2: https://codereview.chromium.org/2550263002/ 3: https://codereview.chromium.org/2553863002/ There is additional context at go/cros-lock-screen-optimization. BUG=669638 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
xiyuan@ PTAL since achuith@ is OOO.
https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... File chrome/browser/chromeos/login/ui/shared_web_view.cc (right): https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.cc:35: web_view_.reset(); nit: Unnecessary since Shutdown should always be called before dtor for KeyedService. https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.cc:69: handle_count_ += 1; nit: ++handle_count_; https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.cc:74: handle_count_ -= 1; nti: --handle_count_; https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... File chrome/browser/chromeos/login/ui/shared_web_view.h (right): https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.h:25: class SharedWebView : public KeyedService, nit: Document the class. https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.h:43: // Mark the webview as no-longer in use. nit: insert an empty line before https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.h:58: Profile* profile_; nit: Profile* const since |profile_| is not going to change during the lifetime of the class. https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.h:79: class SharedWebViewUsageHandle { This looks like to be used as a ref count for |web_view_| in SharedWebView. I wonder if we could leverage existing infra to achieve this. e.g. Make SharedWebViewUsageHandle a RefCounted and own the web_view_. SharedWebView can hold a ref to keep it alive. And we can use RefCounted::HasOneRef to tell whether there is only one ref on it. class SharedWebViewUsageHandle : RefCounted<SharedWebViewUsageHandle> { ... private: std::unique_ptr<views::WebView> web_view_; ... }; class SharedWebView : ... { public: ... bool Get(const GURL& url, scoped_refptr<SharedWebViewUsageHandle>* out_handle); private: ... scoped_refptr<SharedWebViewUsageHandle> web_view_handle_; ... }; https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... File chrome/browser/chromeos/login/ui/shared_web_view_factory.cc (right): https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view_factory.cc:37: return chrome::GetBrowserContextRedirectedInIncognito(context); |context| should be the incognito sign-in profile and we should use that as-is. Otherwise, we would create a WebView use the original profile which is backed by disk and we don't have to leave traces for login. And we might consider check |context| is a sign-in profile and only use that. i.e. // No SharedWebView if |context| is not sign-in profile. if (Profile::FromBrowserContext(context) != ProfileHelper::GetSigninProfile()) return nullptr; return context; https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view_factory.cc:43: // is considered incognito. After we override GetBrowserContextToUse above, this should be called for incognito profile. So the comment is not exactly correct. Update or remove. https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... File chrome/browser/chromeos/login/ui/webui_login_view.cc (right): https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/webui_login_view.cc:256: if (is_reusing_webview_) We would skip Oobe.teardown if |webui_login_| is created regardless whether it is created from SharedWebView or from WebUILoginView ? Should we always call Oobe.teardown if |webui_login_| is from SharedWebView since we are going to re-use it later ? https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/webui_login_view.cc:312: web_contents->SetDelegate(this); Clear these two delegate in dtor just in case? https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/webui_login_view.cc:406: return nullptr; Out of curiosity, this is needed now because we reuse the webview? https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... File chrome/browser/chromeos/login/ui/webui_login_view.h (right): https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/webui_login_view.h:49: GURL preloaded_url; nit: #include "url/gurl.h" and get rid of forward decl since we declare it as member. https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/webui_login_view.h:114: static void InitializeWebView(views::WebView* web_view); nit: no need to expose this in header, put it in the anonymous namespace in cc file https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/webui_login_view.h:160: WebViewSettings settings_; nit: const WebViewSettings if we don't need to change |settings_|.
The CQ bit was checked by jdufault@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... File chrome/browser/chromeos/login/ui/shared_web_view.cc (right): https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.cc:35: web_view_.reset(); On 2016/12/09 22:37:29, xiyuan wrote: > nit: Unnecessary since Shutdown should always be called before dtor for > KeyedService. Done. https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.cc:69: handle_count_ += 1; On 2016/12/09 22:37:29, xiyuan wrote: > nit: ++handle_count_; Removed https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.cc:74: handle_count_ -= 1; On 2016/12/09 22:37:29, xiyuan wrote: > nti: --handle_count_; Removed https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... File chrome/browser/chromeos/login/ui/shared_web_view.h (right): https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.h:25: class SharedWebView : public KeyedService, On 2016/12/09 22:37:30, xiyuan wrote: > nit: Document the class. Done. https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.h:43: // Mark the webview as no-longer in use. On 2016/12/09 22:37:30, xiyuan wrote: > nit: insert an empty line before Removed https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.h:58: Profile* profile_; On 2016/12/09 22:37:30, xiyuan wrote: > nit: Profile* const since |profile_| is not going to change during the lifetime > of the class. Done. https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.h:79: class SharedWebViewUsageHandle { On 2016/12/09 22:37:30, xiyuan wrote: > This looks like to be used as a ref count for |web_view_| in SharedWebView. I > wonder if we could leverage existing infra to achieve this. > > e.g. > > Make SharedWebViewUsageHandle a RefCounted and own the web_view_. SharedWebView > can hold a ref to keep it alive. And we can use RefCounted::HasOneRef to tell > whether there is only one ref on it. > > class SharedWebViewUsageHandle : RefCounted<SharedWebViewUsageHandle> { > ... > private: > std::unique_ptr<views::WebView> web_view_; > ... > }; > > class SharedWebView : ... { > public: > ... > bool Get(const GURL& url, > scoped_refptr<SharedWebViewUsageHandle>* out_handle); > private: > ... > scoped_refptr<SharedWebViewUsageHandle> web_view_handle_; > ... > }; Done. https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... File chrome/browser/chromeos/login/ui/shared_web_view_factory.cc (right): https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view_factory.cc:37: return chrome::GetBrowserContextRedirectedInIncognito(context); On 2016/12/09 22:37:30, xiyuan wrote: > |context| should be the incognito sign-in profile and we should use that as-is. > Otherwise, we would create a WebView use the original profile which is backed by > disk and we don't have to leave traces for login. > > And we might consider check |context| is a sign-in profile and only use that. > > i.e. > > // No SharedWebView if |context| is not sign-in profile. > if (Profile::FromBrowserContext(context) != ProfileHelper::GetSigninProfile()) > return nullptr; > > return context; Done. https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view_factory.cc:43: // is considered incognito. On 2016/12/09 22:37:30, xiyuan wrote: > After we override GetBrowserContextToUse above, this should be called for > incognito profile. So the comment is not exactly correct. Update or remove. Done. https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... File chrome/browser/chromeos/login/ui/webui_login_view.cc (right): https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/webui_login_view.cc:256: if (is_reusing_webview_) On 2016/12/09 22:37:30, xiyuan wrote: > We would skip Oobe.teardown if |webui_login_| is created regardless whether it > is created from SharedWebView or from WebUILoginView ? > > Should we always call Oobe.teardown if |webui_login_| is from SharedWebView > since we are going to re-use it later ? Good catch, thanks. https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/webui_login_view.cc:312: web_contents->SetDelegate(this); On 2016/12/09 22:37:30, xiyuan wrote: > Clear these two delegate in dtor just in case? Done. https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/webui_login_view.cc:406: return nullptr; On 2016/12/09 22:37:30, xiyuan wrote: > Out of curiosity, this is needed now because we reuse the webview? Yep. https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... File chrome/browser/chromeos/login/ui/webui_login_view.h (right): https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/webui_login_view.h:49: GURL preloaded_url; On 2016/12/09 22:37:30, xiyuan wrote: > nit: #include "url/gurl.h" and get rid of forward decl since we declare it as > member. Done. https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/webui_login_view.h:114: static void InitializeWebView(views::WebView* web_view); On 2016/12/09 22:37:30, xiyuan wrote: > nit: no need to expose this in header, put it in the anonymous namespace in cc > file This is needed for preloading the webview (done in a child CL). https://codereview.chromium.org/2512473004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/webui_login_view.h:160: WebViewSettings settings_; On 2016/12/09 22:37:30, xiyuan wrote: > nit: const WebViewSettings if we don't need to change |settings_|. Done.
LGTM with one nit to update the comment https://codereview.chromium.org/2512473004/diff/120001/chrome/browser/chromeo... File chrome/browser/chromeos/login/ui/shared_web_view.h (right): https://codereview.chromium.org/2512473004/diff/120001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.h:25: // profile. This allows for a WebView to be reused over time or preloaded. Use "the current user's profile" -> the sign-in profile since SharedWebView is used to share the webview for login/lock screen.
The CQ bit was checked by jdufault@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from alemate@chromium.org, xiyuan@chromium.org Link to the patchset: https://codereview.chromium.org/2512473004/#ps140001 (title: "Address comments")
https://codereview.chromium.org/2512473004/diff/120001/chrome/browser/chromeo... File chrome/browser/chromeos/login/ui/shared_web_view.h (right): https://codereview.chromium.org/2512473004/diff/120001/chrome/browser/chromeo... chrome/browser/chromeos/login/ui/shared_web_view.h:25: // profile. This allows for a WebView to be reused over time or preloaded. Use On 2016/12/12 18:51:11, xiyuan wrote: > "the current user's profile" -> the sign-in profile since SharedWebView is used > to share the webview for login/lock screen. Done.
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 140001, "attempt_start_ts": 1481572222324440,
"parent_rev": "ac4bf7a33cb97cf798c6b414095e8985254629ca", "commit_rev":
"a4a1d0360e13f7a25357b5e6426d26ca9544e5d6"}
Message was sent while issue was closed.
Description was changed from ========== cros: Enable WebUILoginView reuse. This CL also includes a few fixes for loading a WebUILoginView instance by itself without associated helper classes also being initialized. There are three child CLs associated with this one: - [1] contains fixes on the HTML side. - [2] makes the lock screen show up correctly in the task manager. - [3] preloads and reuses the WebUIScreenLocker instance (derived from WebUILoginView). 1: https://codereview.chromium.org/2555453003/ 2: https://codereview.chromium.org/2550263002/ 3: https://codereview.chromium.org/2553863002/ There is additional context at go/cros-lock-screen-optimization. BUG=669638 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== cros: Enable WebUILoginView reuse. This CL also includes a few fixes for loading a WebUILoginView instance by itself without associated helper classes also being initialized. There are three child CLs associated with this one: - [1] contains fixes on the HTML side. - [2] makes the lock screen show up correctly in the task manager. - [3] preloads and reuses the WebUIScreenLocker instance (derived from WebUILoginView). 1: https://codereview.chromium.org/2555453003/ 2: https://codereview.chromium.org/2550263002/ 3: https://codereview.chromium.org/2553863002/ There is additional context at go/cros-lock-screen-optimization. BUG=669638 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2512473004 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== cros: Enable WebUILoginView reuse. This CL also includes a few fixes for loading a WebUILoginView instance by itself without associated helper classes also being initialized. There are three child CLs associated with this one: - [1] contains fixes on the HTML side. - [2] makes the lock screen show up correctly in the task manager. - [3] preloads and reuses the WebUIScreenLocker instance (derived from WebUILoginView). 1: https://codereview.chromium.org/2555453003/ 2: https://codereview.chromium.org/2550263002/ 3: https://codereview.chromium.org/2553863002/ There is additional context at go/cros-lock-screen-optimization. BUG=669638 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2512473004 ========== to ========== cros: Enable WebUILoginView reuse. This CL also includes a few fixes for loading a WebUILoginView instance by itself without associated helper classes also being initialized. There are three child CLs associated with this one: - [1] contains fixes on the HTML side. - [2] makes the lock screen show up correctly in the task manager. - [3] preloads and reuses the WebUIScreenLocker instance (derived from WebUILoginView). 1: https://codereview.chromium.org/2555453003/ 2: https://codereview.chromium.org/2550263002/ 3: https://codereview.chromium.org/2553863002/ There is additional context at go/cros-lock-screen-optimization. BUG=669638 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/3a6fa9bb8a91ec674c9a62d4fc8f4765b41b10b6 Cr-Commit-Position: refs/heads/master@{#437942} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/3a6fa9bb8a91ec674c9a62d4fc8f4765b41b10b6 Cr-Commit-Position: refs/heads/master@{#437942} |
