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

Issue 374933003: Remove DispositionChangePhase. (Closed)

Created:
6 years, 5 months ago by Ben Goodger (Google)
Modified:
6 years, 5 months ago
Reviewers:
sky
CC:
chromium-reviews, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), ben+mojo_chromium.org
Project:
chromium
Visibility:
Public.

Description

Remove DispositionChangePhase. The problem with having a single observer method with a phase param is that it's cumbersome and error prone to use for most clients that just want to know when something *has* changed. I think it's appropriate then to create separate methods for each phase and have the client override the one (or both) that is relevant to them. TBR=sky@chromium.org BUG=none Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=281874

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+135 lines, -212 lines) Patch
M mojo/aura/window_tree_host_mojo.h View 1 chunk +2 lines, -3 lines 0 comments Download
M mojo/aura/window_tree_host_mojo.cc View 1 chunk +2 lines, -3 lines 0 comments Download
M mojo/examples/embedded_app/embedded_app.cc View 1 chunk +4 lines, -10 lines 0 comments Download
M mojo/examples/nesting_app/nesting_app.cc View 1 chunk +1 line, -5 lines 0 comments Download
M mojo/services/public/cpp/view_manager/lib/node.cc View 6 chunks +49 lines, -67 lines 0 comments Download
M mojo/services/public/cpp/view_manager/lib/node_observer.cc View 1 chunk +1 line, -2 lines 0 comments Download
M mojo/services/public/cpp/view_manager/lib/view.cc View 1 chunk +6 lines, -8 lines 0 comments Download
M mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.cc View 1 chunk +1 line, -4 lines 0 comments Download
M mojo/services/public/cpp/view_manager/node_observer.h View 1 chunk +33 lines, -21 lines 0 comments Download
M mojo/services/public/cpp/view_manager/tests/node_unittest.cc View 20 chunks +21 lines, -52 lines 0 comments Download
M mojo/services/public/cpp/view_manager/tests/view_manager_unittest.cc View 7 chunks +11 lines, -31 lines 0 comments Download
M mojo/services/public/cpp/view_manager/view_observer.h View 1 chunk +4 lines, -6 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Ben Goodger (Google)
The CQ bit was checked by ben@chromium.org
6 years, 5 months ago (2014-07-08 18:20:14 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ben@chromium.org/374933003/1
6 years, 5 months ago (2014-07-08 18:22:33 UTC) #2
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_dbg_triggered_tests on tryserver.chromium ...
6 years, 5 months ago (2014-07-08 19:57:24 UTC) #3
commit-bot: I haz the power
6 years, 5 months ago (2014-07-08 23:21:41 UTC) #4
Message was sent while issue was closed.
Change committed as 281874

Powered by Google App Engine
This is Rietveld 408576698