Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(195)

Issue 10837222: Enable EULA dialog to be shown from metro Chrome. (Closed)

Created:
8 years, 4 months ago by robertshield
Modified:
8 years, 2 months ago
CC:
chromium-reviews, grt+watch_chromium.org, chrome-win8-eng_google.com
Visibility:
Public.

Description

Enable EULA dialog to be shown from metro Chrome. BUG=131033 TEST=Run Chrome in Metro mode while the EULA dialog still needs to be accepted. Get kicked back to the desktop to accept the dialog. On accept, get kicked back into metro mode. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=158797

Patch Set 1 #

Patch Set 2 : Add relaunching chrome in metro to setup.exe. Some shell util refactoring. #

Patch Set 3 : git cl try #

Patch Set 4 : Cleanup #

Patch Set 5 : include fix #

Patch Set 6 : Fix merge conflicts. #

Patch Set 7 : Avoid calling ExitProcess from the show EULA code. It is ugly and makes metro deeply unhappy. #

Patch Set 8 : Create EULA beacon from setup.exe. #

Patch Set 9 : Bit o' spit n' polish. #

Total comments: 25

Patch Set 10 : Dear Greg #

Total comments: 2

Patch Set 11 : "De-dupe some code, one more Dear Greg." #

Total comments: 3

Patch Set 12 : Dear Greg, the third. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+213 lines, -104 lines) Patch
M chrome/browser/chrome_browser_main.cc View 1 2 3 4 5 6 7 1 chunk +8 lines, -2 lines 0 comments Download
M chrome/browser/first_run/first_run.h View 1 2 3 4 5 6 2 chunks +12 lines, -5 lines 0 comments Download
M chrome/browser/first_run/first_run_posix.cc View 1 2 3 4 5 6 3 chunks +8 lines, -7 lines 0 comments Download
M chrome/browser/first_run/first_run_win.cc View 1 2 3 4 5 6 7 8 9 10 10 chunks +69 lines, -50 lines 0 comments Download
chrome/browser/process_singleton_win.cc View 1 2 3 4 5 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/shell_integration_win.cc View 1 2 3 4 5 6 7 8 2 chunks +5 lines, -2 lines 0 comments Download
M chrome/common/chrome_result_codes.h View 1 2 3 4 5 6 7 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/installer/setup/setup_main.cc View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +43 lines, -2 lines 0 comments Download
M chrome/installer/util/google_update_settings.cc View 1 2 3 4 5 6 7 2 chunks +0 lines, -22 lines 0 comments Download
M chrome/installer/util/install_util.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +7 lines, -0 lines 0 comments Download
M chrome/installer/util/install_util.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +24 lines, -0 lines 0 comments Download
M chrome/installer/util/shell_util.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/installer/util/shell_util.cc View 1 2 5 chunks +16 lines, -9 lines 0 comments Download
chrome/installer/util/util_constants.h View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/installer/util/util_constants.cc View 1 2 3 4 5 6 7 8 2 chunks +4 lines, -0 lines 0 comments Download
M ui/base/win/shell.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M ui/base/win/shell.cc View 1 2 3 4 5 6 7 8 9 2 chunks +7 lines, -3 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
robertshield
8 years, 3 months ago (2012-09-20 15:11:49 UTC) #1
grt (UTC plus 2)
looks pretty good. http://codereview.chromium.org/10837222/diff/37001/chrome/browser/first_run/first_run_win.cc File chrome/browser/first_run/first_run_win.cc (right): http://codereview.chromium.org/10837222/diff/37001/chrome/browser/first_run/first_run_win.cc#newcode173 chrome/browser/first_run/first_run_win.cc:173: // here http://goo.gl/HBOe9. It causes the ...
8 years, 3 months ago (2012-09-20 15:58:31 UTC) #2
robertshield
Thanks Greg! PTAL https://chromiumcodereview.appspot.com/10837222/diff/37001/chrome/browser/first_run/first_run_win.cc File chrome/browser/first_run/first_run_win.cc (right): https://chromiumcodereview.appspot.com/10837222/diff/37001/chrome/browser/first_run/first_run_win.cc#newcode173 chrome/browser/first_run/first_run_win.cc:173: // here http://goo.gl/HBOe9. It causes the ...
8 years, 3 months ago (2012-09-21 01:24:43 UTC) #3
grt (UTC plus 2)
very nice! lgtm with merely one nit. https://chromiumcodereview.appspot.com/10837222/diff/37001/chrome/installer/setup/setup_main.cc File chrome/installer/setup/setup_main.cc (right): https://chromiumcodereview.appspot.com/10837222/diff/37001/chrome/installer/setup/setup_main.cc#newcode1039 chrome/installer/setup/setup_main.cc:1039: bool GetSentinelFilePath(const ...
8 years, 3 months ago (2012-09-21 01:35:41 UTC) #4
robertshield
Adressed nit, also deduped the beacon-finding code, since it really doesn't make sense to have ...
8 years, 3 months ago (2012-09-21 13:12:59 UTC) #5
grt (UTC plus 2)
this is nicer, thanks for the extra work. lgtm w/ a nit. https://chromiumcodereview.appspot.com/10837222/diff/35004/chrome/installer/setup/setup_main.cc File chrome/installer/setup/setup_main.cc ...
8 years, 3 months ago (2012-09-21 13:21:15 UTC) #6
robertshield
+Ben for OWNERS review, Ben PTAQL
8 years, 3 months ago (2012-09-21 14:25:10 UTC) #7
robertshield
Scott, just noticed that Ben was out sick, please could you give your OWNERS blessing? ...
8 years, 2 months ago (2012-09-26 01:35:53 UTC) #8
grt (UTC plus 2)
http://codereview.chromium.org/10837222/diff/35004/chrome/installer/setup/setup_main.cc File chrome/installer/setup/setup_main.cc (right): http://codereview.chromium.org/10837222/diff/35004/chrome/installer/setup/setup_main.cc#newcode1041 chrome/installer/setup/setup_main.cc:1041: return false; On 2012/09/26 01:35:53, robertshield wrote: > On ...
8 years, 2 months ago (2012-09-26 01:59:21 UTC) #9
sky
LGTM
8 years, 2 months ago (2012-09-26 04:00:41 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/robertshield@chromium.org/10837222/27008
8 years, 2 months ago (2012-09-26 12:41:02 UTC) #11
commit-bot: I haz the power
8 years, 2 months ago (2012-09-26 14:16:52 UTC) #12
Change committed as 158797

Powered by Google App Engine
This is Rietveld 408576698