Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2191173003:
MD History: Get sign in state from backend when refreshing the page (Closed)
|
Created:
4 years, 4 months ago by lshang Modified:
4 years, 4 months ago CC:
chromium-reviews, Patrick Dubroy, michaelpg+watch-md-ui_chromium.org, dbeam+watch-history_chromium.org, pam+watch_chromium.org, arv+watch_chromium.org, chrome-apps-syd-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMD History: Get sign in state from backend when refreshing the page
In synced tabs page, the sign in promo will still show up when user signs
in via the sign in promo and refresh the page.
The reason of this bug is that internal sign in state of the element got
updated, but refreshing the page, which gets the sign in state default
value from loadTimeData, will still get the outdated state.
Fix of the bug is to get sign in state from HistoryLoginHandler every time
the page is reloaded, this will get the updated current sign in state.
BUG=625109
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/93473de1895935289af8159ac8e0def340659fcb
Cr-Commit-Position: refs/heads/master@{#409120}
Patch Set 1 #
Total comments: 6
Patch Set 2 : simplify #Messages
Total messages: 24 (16 generated)
Description was changed from ========== MD History: Get sign in state from backend when refreshing the page Merge branch 'master' into MDH_hide_sign_in_promo_in_devtools add some log BUG= ========== to ========== MD History: Get sign in state from backend when refreshing the page Merge branch 'master' into MDH_hide_sign_in_promo_in_devtools add some log BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by lshang@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...
Patchset #1 (id:1) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== MD History: Get sign in state from backend when refreshing the page Merge branch 'master' into MDH_hide_sign_in_promo_in_devtools add some log BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== MD History: Get sign in state from backend when refreshing the page In synced tabs page, the sign in promo will still show up when user signs in via the sign in promo and refresh the page. The reason of this bug is that internal sign in state of the element got updated, but refreshing the page, which gets the sign in state default value from loadTimeData, will still get the outdated state. Fix of the bug is to get sign in state from HistoryLoginHandler every time the page is reloaded, this will get the updated current sign in state. BUG=625109 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== MD History: Get sign in state from backend when refreshing the page In synced tabs page, the sign in promo will still show up when user signs in via the sign in promo and refresh the page. The reason of this bug is that internal sign in state of the element got updated, but refreshing the page, which gets the sign in state default value from loadTimeData, will still get the outdated state. Fix of the bug is to get sign in state from HistoryLoginHandler every time the page is reloaded, this will get the updated current sign in state. BUG=625109 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== MD History: Get sign in state from backend when refreshing the page In synced tabs page, the sign in promo will still show up when user signs in via the sign in promo and refresh the page. The reason of this bug is that internal sign in state of the element got updated, but refreshing the page, which gets the sign in state default value from loadTimeData, will still get the outdated state. Fix of the bug is to get sign in state from HistoryLoginHandler every time the page is reloaded, this will get the updated current sign in state. BUG=625109 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
lshang@chromium.org changed reviewers: + calamity@chromium.org, tsergeant@chromium.org
PTAL thanks!
https://codereview.chromium.org/2191173003/diff/20001/chrome/browser/resource... File chrome/browser/resources/md_history/synced_device_manager.js (right): https://codereview.chromium.org/2191173003/diff/20001/chrome/browser/resource... chrome/browser/resources/md_history/synced_device_manager.js:44: value: true, You might want to leave this in... I think this will be janky with non-signed in profiles if you go directly to chrome://history/syncedTabs. https://codereview.chromium.org/2191173003/diff/20001/chrome/browser/resource... chrome/browser/resources/md_history/synced_device_manager.js:66: this.getSignInState_(); No need for a method here, just chrome.send('otherDevicesInitialized'). The comment // Update the sign in state. is probably sufficient. The current comment is a bit too specific. https://codereview.chromium.org/2191173003/diff/20001/chrome/browser/ui/webui... File chrome/browser/ui/webui/history_login_handler.cc (right): https://codereview.chromium.org/2191173003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/history_login_handler.cc:25: base::Unretained(this))); Can we just reuse this message? It seems to be exactly the thing that solves this problem in old history.
https://codereview.chromium.org/2191173003/diff/20001/chrome/browser/resource... File chrome/browser/resources/md_history/synced_device_manager.js (right): https://codereview.chromium.org/2191173003/diff/20001/chrome/browser/resource... chrome/browser/resources/md_history/synced_device_manager.js:44: value: true, On 2016/07/29 05:14:59, calamity wrote: > You might want to leave this in... I think this will be janky with non-signed in > profiles if you go directly to chrome://history/syncedTabs. Done. https://codereview.chromium.org/2191173003/diff/20001/chrome/browser/resource... chrome/browser/resources/md_history/synced_device_manager.js:66: this.getSignInState_(); On 2016/07/29 05:14:59, calamity wrote: > No need for a method here, just chrome.send('otherDevicesInitialized'). > > The comment // Update the sign in state. is probably sufficient. The current > comment is a bit too specific. Done. https://codereview.chromium.org/2191173003/diff/20001/chrome/browser/ui/webui... File chrome/browser/ui/webui/history_login_handler.cc (right): https://codereview.chromium.org/2191173003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/history_login_handler.cc:25: base::Unretained(this))); On 2016/07/29 05:14:59, calamity wrote: > Can we just reuse this message? It seems to be exactly the thing that solves > this problem in old history. Done. I thought it would be a little confusing to chrome.send('otherDevicesInitialized') when the element is attached, but since this is in synced tabs, it might make sense. I'm quite neutral on this, also reuse this message will make this CL much simpler.
this version lgtm
The CQ bit was checked by lshang@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: This issue passed the CQ dry run.
The CQ bit was checked by lshang@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== MD History: Get sign in state from backend when refreshing the page In synced tabs page, the sign in promo will still show up when user signs in via the sign in promo and refresh the page. The reason of this bug is that internal sign in state of the element got updated, but refreshing the page, which gets the sign in state default value from loadTimeData, will still get the outdated state. Fix of the bug is to get sign in state from HistoryLoginHandler every time the page is reloaded, this will get the updated current sign in state. BUG=625109 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== MD History: Get sign in state from backend when refreshing the page In synced tabs page, the sign in promo will still show up when user signs in via the sign in promo and refresh the page. The reason of this bug is that internal sign in state of the element got updated, but refreshing the page, which gets the sign in state default value from loadTimeData, will still get the outdated state. Fix of the bug is to get sign in state from HistoryLoginHandler every time the page is reloaded, this will get the updated current sign in state. BUG=625109 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== MD History: Get sign in state from backend when refreshing the page In synced tabs page, the sign in promo will still show up when user signs in via the sign in promo and refresh the page. The reason of this bug is that internal sign in state of the element got updated, but refreshing the page, which gets the sign in state default value from loadTimeData, will still get the outdated state. Fix of the bug is to get sign in state from HistoryLoginHandler every time the page is reloaded, this will get the updated current sign in state. BUG=625109 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== MD History: Get sign in state from backend when refreshing the page In synced tabs page, the sign in promo will still show up when user signs in via the sign in promo and refresh the page. The reason of this bug is that internal sign in state of the element got updated, but refreshing the page, which gets the sign in state default value from loadTimeData, will still get the outdated state. Fix of the bug is to get sign in state from HistoryLoginHandler every time the page is reloaded, this will get the updated current sign in state. BUG=625109 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/93473de1895935289af8159ac8e0def340659fcb Cr-Commit-Position: refs/heads/master@{#409120} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/93473de1895935289af8159ac8e0def340659fcb Cr-Commit-Position: refs/heads/master@{#409120} |
