GTK: Do not depend on the notification observer order...

chrome/browser/ui/toolbar/wrench_menu_model.h

Index: chrome/browser/ui/toolbar/wrench_menu_model.h
===================================================================
--- chrome/browser/ui/toolbar/wrench_menu_model.h	(revision 153513)
+++ chrome/browser/ui/toolbar/wrench_menu_model.h	(working copy)
@@ -122,6 +122,13 @@
   // Calculates |zoom_label_| in response to a zoom change.
   void UpdateZoomControls();
 
+#if defined(TOOLKIT_GTK)

[sail, 2012/08/28 16:46:55]

I don't think you need this #ifdef.

[Lei Zhang, 2012/08/28 19:52:54]

No other platform needs this functionality right now though.

[sail, 2012/08/28 20:42:52]

I think that's ok. Even if this is unused the test will be cross platform and
that's enough.

+  // Rebuilds the model, rather than requiring the object to be recreated. Thus
+  // avoids having to re-add notification observers, which causes the ordering
+  // for notifications to be non-deterministic - http://crbug.com/118823.

[sail, 2012/08/28 16:46:55]

This is kind of vague. Why does the GTK code depend on notifications to be in a
certain order? Can't we fix that?

[Lei Zhang, 2012/08/28 19:52:54]

All platform depends on the zoom notification observers to process the zoom
notification in the right order. The WrenchMenuModel needs to observe the
notification first so it can update its internal zoom value. Then whoever draws
the menu gets the notifications and fetches the value from WrenchMenuModel.

Because we are destroying the WrenchMenuModel and recreating it on GTK, we end
up recreating the observer, which then only gets the zoom notification after the
redraw.

[sail, 2012/08/28 20:42:52]

I think I understand but that still seems like a bug. It seems like what we want
is a the menu model to have its own way to update the later observers.

+  void Rebuild();
+#endif
+
  private:
   // Testing constructor used for mocking.
   friend class ::MockWrenchMenuModel;
@@ -160,7 +167,7 @@
   ui::AcceleratorProvider* provider_;  // weak
 
   Browser* browser_;  // weak
-  TabStripModel* tab_strip_model_; // weak
+  TabStripModel* tab_strip_model_;  // weak
 
   content::NotificationRegistrar registrar_;