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

Issue 10436015: Remove chrome.windows.* support for platform apps. (Closed)

Created:
8 years, 7 months ago by jeremya
Modified:
8 years, 6 months ago
CC:
chromium-reviews, Aaron Boodman, mihaip-chromium-reviews_chromium.org, Ben Goodger (Google), benwells, Tyler Breisacher (Chromium)
Visibility:
Public.

Description

Remove chrome.windows.* support for platform apps. Use chrome.appWindow.* instead. R=mihaip@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=140947

Patch Set 1 #

Total comments: 4

Patch Set 2 : more things #

Patch Set 3 : . #

Total comments: 2

Patch Set 4 : rebase #

Patch Set 5 : remove ShellWindowController #

Patch Set 6 : fix compile #

Total comments: 2

Patch Set 7 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+38 lines, -158 lines) Patch
M chrome/browser/extensions/extension_tabs_module.cc View 1 2 3 1 chunk +0 lines, -12 lines 0 comments Download
M chrome/browser/extensions/extension_tabs_module_constants.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/extensions/extension_tabs_module_constants.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/extensions/platform_app_browsertest.cc View 1 2 3 4 5 2 chunks +5 lines, -6 lines 0 comments Download
M chrome/browser/ui/extensions/shell_window.h View 1 2 3 4 2 chunks +0 lines, -4 lines 0 comments Download
M chrome/browser/ui/extensions/shell_window.cc View 1 2 3 4 5 chunks +2 lines, -65 lines 0 comments Download
M chrome/common/extensions/api/_permission_features.json View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/common/extensions/api/windows.json View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/common/extensions/extension_permission_set.cc View 1 2 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/test/data/extensions/platform_apps/context_menu/manifest.json View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/data/extensions/platform_apps/context_menu/test.js View 1 2 3 1 chunk +1 line, -4 lines 0 comments Download
M chrome/test/data/extensions/platform_apps/iframes/manifest.json View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/data/extensions/platform_apps/iframes/test.js View 1 2 1 chunk +1 line, -4 lines 0 comments Download
M chrome/test/data/extensions/platform_apps/minimal/manifest.json View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/data/extensions/platform_apps/minimal/test.js View 1 2 1 chunk +1 line, -4 lines 0 comments Download
M chrome/test/data/extensions/platform_apps/navigation/manifest.json View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/data/extensions/platform_apps/navigation/test.js View 1 1 chunk +1 line, -4 lines 0 comments Download
M chrome/test/data/extensions/platform_apps/windows_api/manifest.json View 1 chunk +1 line, -1 line 0 comments Download
A chrome/test/data/extensions/platform_apps/windows_api/test.html View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/test/data/extensions/platform_apps/windows_api/test.js View 1 2 3 4 5 6 1 chunk +18 lines, -43 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
jeremya
8 years, 7 months ago (2012-05-24 04:40:26 UTC) #1
Mihai Parparita -not on Chrome
There are a few other platform app browser tests that use chrome.windows.create: http://code.google.com/p/chromium/source/search?origq=chrome.windows.create+file%3Aplatform_app http://codereview.chromium.org/10436015/diff/1/chrome/test/data/extensions/platform_apps/windows_api/test.js File ...
8 years, 7 months ago (2012-05-24 21:16:03 UTC) #2
jeremya
Tests are passing now. https://chromiumcodereview.appspot.com/10436015/diff/1/chrome/test/data/extensions/platform_apps/windows_api/test.js File chrome/test/data/extensions/platform_apps/windows_api/test.js (right): https://chromiumcodereview.appspot.com/10436015/diff/1/chrome/test/data/extensions/platform_apps/windows_api/test.js#newcode11 chrome/test/data/extensions/platform_apps/windows_api/test.js:11: chrome.test.assertTrue(win.true); On 2012/05/24 21:16:04, Mihai ...
8 years, 6 months ago (2012-05-28 04:30:03 UTC) #3
Mihai Parparita -not on Chrome
LGTM If this lands after https://chromiumcodereview.appspot.com/10456003/, maybe all of the {}, function() {} dummy params ...
8 years, 6 months ago (2012-05-29 06:09:13 UTC) #4
jeremya
https://chromiumcodereview.appspot.com/10436015/diff/6009/chrome/browser/ui/extensions/shell_window.cc File chrome/browser/ui/extensions/shell_window.cc (right): https://chromiumcodereview.appspot.com/10436015/diff/6009/chrome/browser/ui/extensions/shell_window.cc#newcode79 chrome/browser/ui/extensions/shell_window.cc:79: // this should NOTREACHED(). On 2012/05/29 06:09:13, Mihai Parparita ...
8 years, 6 months ago (2012-05-30 01:10:47 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jeremya@chromium.org/10436015/10001
8 years, 6 months ago (2012-06-06 00:49:58 UTC) #6
commit-bot: I haz the power
Try job failure for 10436015-10001 (retry) on linux_rel for step "browser_tests". It's a second try, ...
8 years, 6 months ago (2012-06-06 01:36:14 UTC) #7
jeremya
Since shell windows are no longer accessible from the chrome.windows.* API, I've removed the ExtensionWindowController ...
8 years, 6 months ago (2012-06-06 05:01:33 UTC) #8
Mihai Parparita -not on Chrome
LGTM (though we'll probably want to bring this back if the new windowing API should ...
8 years, 6 months ago (2012-06-06 20:58:43 UTC) #9
jeremya
https://chromiumcodereview.appspot.com/10436015/diff/16002/chrome/browser/ui/extensions/shell_window.h File chrome/browser/ui/extensions/shell_window.h (right): https://chromiumcodereview.appspot.com/10436015/diff/16002/chrome/browser/ui/extensions/shell_window.h#newcode113 chrome/browser/ui/extensions/shell_window.h:113: virtual ExtensionWindowController* GetExtensionWindowController() const On 2012/06/06 20:58:43, Mihai Parparita ...
8 years, 6 months ago (2012-06-06 23:50:06 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jeremya@chromium.org/10436015/13004
8 years, 6 months ago (2012-06-07 00:50:55 UTC) #11
commit-bot: I haz the power
8 years, 6 months ago (2012-06-07 03:56:08 UTC) #12
Change committed as 140947

Powered by Google App Engine
This is Rietveld 408576698