Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(74)

Issue 9443007: Add Chrome To Mobile Service and Views Page Action. (Closed)

Created:
7 years, 6 months ago by msw
Modified:
7 years, 5 months ago
CC:
chromium-reviews, Dmitry Lomov (no reviews), jennb, Dmitry Titov, jianli, dcheng, Andrei
Visibility:
Public.

Description

Add Chrome To Mobile Service and Views Page Action. Implements the Chrome To Mobile extension in Chrome. List the user's mobile devices via the Cloud Print server. Add a page action icon when the service reports 1+ devices. Add a bubble to send the current page URL / MHTML snapshot. The bubble shows a radio group for multiple devices. (or it shows a single device as part of the title label) The bubble also shows a checkbox to send an offline copy. Send URLFetcher requests to GET/POST the URL/Snapshot. The bubble shows "Sending..."/"Sent"/ error request status. BUG=102709 TEST=New Chrome To Mobile bubble works as expected :) Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=126343

Patch Set 1 : First URL successfully sent to device. #

Patch Set 2 : Plain text/mhtml snapshots work; use ProfileKeyedService[Factory], etc. #

Patch Set 3 : Put shared CloudPrint consts/helpers in chrome/common/; use CloudPrintURL. #

Total comments: 31

Patch Set 4 : Address comments, generate snapshots in service with ScopedTempDir, etc. #

Patch Set 5 : Disable for OffTheRecord profiles; remove superfluous profile args. #

Patch Set 6 : Fix auth retries, snapshot path, PostBlockingPoolTask to init ScopedTempDir. #

Patch Set 7 : Nix animation change, init icon visible, fix mac id test, DCHECK token, etc. #

Total comments: 8

Patch Set 8 : Sync and merge. #

Patch Set 9 : Address comments and lints, revert unneeded cloud print changes. #

Patch Set 10 : Bail on empty GetOAuth2LoginRefreshToken(). #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1474 lines, -187 lines) Patch
M chrome/app/chrome_command_ids.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M chrome/app/generated_resources.grd View 2 chunks +44 lines, -0 lines 0 comments Download
A chrome/browser/chrome_to_mobile_service.h View 1 2 3 4 5 6 7 8 1 chunk +133 lines, -0 lines 0 comments Download
A chrome/browser/chrome_to_mobile_service.cc View 1 2 3 4 5 6 7 8 9 1 chunk +336 lines, -0 lines 0 comments Download
A chrome/browser/chrome_to_mobile_service_factory.h View 1 1 chunk +36 lines, -0 lines 0 comments Download
A chrome/browser/chrome_to_mobile_service_factory.cc View 1 1 chunk +32 lines, -0 lines 0 comments Download
M chrome/browser/ui/browser.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/browser.cc View 1 2 3 4 5 6 7 3 chunks +9 lines, -1 line 0 comments Download
M chrome/browser/ui/browser_window.h View 1 3 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/browser_window_cocoa.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/browser_window_cocoa.mm View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/view_id_util_browsertest.mm View 1 2 3 4 5 6 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/gtk/browser_window_gtk.h View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/gtk/browser_window_gtk.cc View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/gtk/view_id_util_browsertest.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/panels/panel.h View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/panels/panel.cc View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/view_ids.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/views/browser_dialogs.h View 1 1 chunk +5 lines, -0 lines 0 comments Download
A chrome/browser/ui/views/chrome_to_mobile_bubble_view.h View 1 2 3 1 chunk +104 lines, -0 lines 0 comments Download
A chrome/browser/ui/views/chrome_to_mobile_bubble_view.cc View 1 2 3 4 1 chunk +315 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/frame/browser_view.h View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/frame/browser_view.cc View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
A chrome/browser/ui/views/location_bar/chrome_to_mobile_view.h View 1 2 3 1 chunk +48 lines, -0 lines 0 comments Download
A chrome/browser/ui/views/location_bar/chrome_to_mobile_view.cc View 1 2 1 chunk +76 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/location_bar/location_bar_view.h View 1 3 chunks +7 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/location_bar/location_bar_view.cc View 1 2 3 4 5 6 7 8 11 chunks +49 lines, -5 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 6 7 5 chunks +12 lines, -0 lines 0 comments Download
M chrome/chrome_common.gypi View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
A chrome/common/cloud_print/cloud_print_helpers.h View 1 2 3 4 5 6 7 8 1 chunk +56 lines, -0 lines 0 comments Download
A chrome/common/cloud_print/cloud_print_helpers.cc View 1 2 3 4 5 6 7 8 1 chunk +94 lines, -0 lines 0 comments Download
M chrome/service/cloud_print/cloud_print_connector.cc View 1 2 3 4 5 6 6 chunks +24 lines, -33 lines 0 comments Download
M chrome/service/cloud_print/cloud_print_consts.h View 1 2 3 4 5 6 7 8 4 chunks +1 line, -5 lines 0 comments Download
M chrome/service/cloud_print/cloud_print_consts.cc View 1 2 3 4 5 6 7 8 4 chunks +1 line, -8 lines 0 comments Download
M chrome/service/cloud_print/cloud_print_helpers.h View 1 2 3 4 5 6 7 8 3 chunks +3 lines, -18 lines 0 comments Download
M chrome/service/cloud_print/cloud_print_helpers.cc View 1 2 3 4 5 6 7 8 12 chunks +31 lines, -84 lines 0 comments Download
M chrome/service/cloud_print/cloud_print_url_fetcher.cc View 1 2 3 4 5 6 7 3 chunks +3 lines, -2 lines 0 comments Download
M chrome/service/cloud_print/printer_job_handler.cc View 1 2 4 chunks +20 lines, -27 lines 0 comments Download
M chrome/test/base/test_browser_window.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 11 (0 generated)
msw
PTAL, consider my TODOs, and suggest reviewers; thanks! Lots may be wrong, advice for major ...
7 years, 5 months ago (2012-03-10 00:40:33 UTC) #1
sky
https://chromiumcodereview.appspot.com/9443007/diff/40001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://chromiumcodereview.appspot.com/9443007/diff/40001/chrome/app/generated_resources.grd#newcode6953 chrome/app/generated_resources.grd:6953: + Also send copy for offline viewing (<ph name="PAGE_SIZE">$1<ex>111kB</ex></ph>) ...
7 years, 5 months ago (2012-03-11 21:41:22 UTC) #2
msw
I'm looking into the tryjob failures, but don't have any leads. https://chromiumcodereview.appspot.com/9443007/diff/40001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): ...
7 years, 5 months ago (2012-03-12 10:17:18 UTC) #3
sky
https://chromiumcodereview.appspot.com/9443007/diff/40001/chrome/browser/ui/browser.h File chrome/browser/ui/browser.h (right): https://chromiumcodereview.appspot.com/9443007/diff/40001/chrome/browser/ui/browser.h#newcode579 chrome/browser/ui/browser.h:579: void ChromeToMobile(); On 2012/03/12 10:17:18, msw wrote: > On ...
7 years, 5 months ago (2012-03-12 15:28:12 UTC) #4
msw
Please take a look asap. I'm fixing trybot failures, and I'll gladly incorporate any feedback ...
7 years, 5 months ago (2012-03-12 22:11:46 UTC) #5
sky
LGTM
7 years, 5 months ago (2012-03-12 23:06:27 UTC) #6
Scott Byer
LGTM with a couple of include order / lint issues. http://codereview.chromium.org/9443007/diff/55002/chrome/browser/chrome_to_mobile_service.cc File chrome/browser/chrome_to_mobile_service.cc (right): http://codereview.chromium.org/9443007/diff/55002/chrome/browser/chrome_to_mobile_service.cc#newcode13 ...
7 years, 5 months ago (2012-03-12 23:26:23 UTC) #7
nyquist
http://codereview.chromium.org/9443007/diff/55002/chrome/browser/chrome_to_mobile_service.cc File chrome/browser/chrome_to_mobile_service.cc (right): http://codereview.chromium.org/9443007/diff/55002/chrome/browser/chrome_to_mobile_service.cc#newcode201 chrome/browser/chrome_to_mobile_service.cc:201: content::URLFetcher* submit_url = CreateRequest(data); Nit: This naming seems odd ...
7 years, 5 months ago (2012-03-13 00:52:23 UTC) #8
msw
Addressed comments and additional lints. I plan to land with green trybots, but will gladly ...
7 years, 5 months ago (2012-03-13 02:39:14 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/9443007/54057
7 years, 5 months ago (2012-03-13 04:51:13 UTC) #10
commit-bot: I haz the power
7 years, 5 months ago (2012-03-13 06:08:49 UTC) #11
Change committed as 126343

Powered by Google App Engine
This is Rietveld 408576698